From 79df13086835903301a6ba196d4f0dd7be23d90d Mon Sep 17 00:00:00 2001 From: "Claudio Maggioni (maggicl)" Date: Sat, 23 May 2020 23:23:24 +0200 Subject: [PATCH] Code smells fix --- .../smarthut/models/ConfirmationToken.java | 2 ++ .../sanmarinoes/smarthut/models/Switchable.java | 1 + .../smarthut/service/AutomationService.java | 14 +++++++++----- .../smarthut/service/DeviceService.java | 7 ++----- .../controller/ButtonDimmerControllerTests.java | 3 ++- .../controller/CurtainsControllerTests.java | 6 +++--- .../smarthut/controller/GuestControllerTests.java | 6 +++--- .../controller/KnobDimmerControllerTests.java | 3 ++- .../controller/SecurityCameraControllerTests.java | 6 +++--- .../smarthut/controller/SwitchControllerTests.java | 3 ++- .../smarthut/service/DeviceServiceTests.java | 4 +--- 11 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/ConfirmationToken.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/ConfirmationToken.java index 5f61bc9..ab858b7 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/ConfirmationToken.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/ConfirmationToken.java @@ -4,6 +4,7 @@ import java.util.Date; import java.util.UUID; import javax.persistence.*; import lombok.Data; +import lombok.NonNull; @Data @Entity @@ -26,6 +27,7 @@ public class ConfirmationToken { } @Temporal(TemporalType.TIMESTAMP) + @NonNull private Date createdDate; @OneToOne(targetEntity = User.class, fetch = FetchType.EAGER) diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Switchable.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Switchable.java index d976758..2f0f64e 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Switchable.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Switchable.java @@ -9,6 +9,7 @@ import lombok.EqualsAndHashCode; /** A device that can be turned either on or off */ @Entity +@EqualsAndHashCode(callSuper = true) @Inheritance(strategy = InheritanceType.JOINED) public abstract class Switchable extends OutputDevice { diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/AutomationService.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/AutomationService.java index 08c0bf6..3887b02 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/AutomationService.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/AutomationService.java @@ -1,9 +1,6 @@ package ch.usi.inf.sa4.sanmarinoes.smarthut.service; -import ch.usi.inf.sa4.sanmarinoes.smarthut.models.Automation; -import ch.usi.inf.sa4.sanmarinoes.smarthut.models.AutomationRepository; -import ch.usi.inf.sa4.sanmarinoes.smarthut.models.Trigger; -import ch.usi.inf.sa4.sanmarinoes.smarthut.models.TriggerRepository; +import ch.usi.inf.sa4.sanmarinoes.smarthut.models.*; import java.util.List; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -12,13 +9,16 @@ import org.springframework.stereotype.Component; public class AutomationService { private final AutomationRepository automationRepository; private final TriggerRepository> triggerRepository; + private final ConditionRepository> conditionRepository; @Autowired public AutomationService( AutomationRepository automationRepository, - TriggerRepository> triggerRepository) { + TriggerRepository> triggerRepository, + ConditionRepository> conditionRepository) { this.automationRepository = automationRepository; this.triggerRepository = triggerRepository; + this.conditionRepository = conditionRepository; } public List> findTriggersByDeviceId(Long deviceId) { @@ -28,4 +28,8 @@ public class AutomationService { public Automation findByVerifiedId(Long automationId) { return automationRepository.findById(automationId).orElseThrow(); } + + public List> findAllConditionsByAutomationId(Long automationId) { + return conditionRepository.findAllByAutomationId(automationId); + } } diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/DeviceService.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/DeviceService.java index 07adb5a..43607d0 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/DeviceService.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/DeviceService.java @@ -22,7 +22,6 @@ public class DeviceService { private final EagerUserRepository userRepository; private final DevicePopulationService devicePopulationService; private final DevicePropagationService devicePropagationService; - private final ConditionRepository> conditionRepository; @Autowired public DeviceService( @@ -32,8 +31,7 @@ public class DeviceService { AutomationService automationService, EagerUserRepository userRepository, DevicePopulationService devicePopulationService, - DevicePropagationService devicePropagationService, - ConditionRepository> conditionRepository) { + DevicePropagationService devicePropagationService) { this.deviceRepository = deviceRepository; this.sceneService = sceneService; this.roomRepository = roomRepository; @@ -41,7 +39,6 @@ public class DeviceService { this.userRepository = userRepository; this.devicePopulationService = devicePopulationService; this.devicePropagationService = devicePropagationService; - this.conditionRepository = conditionRepository; } public void throwIfRoomNotOwned(Long roomId, String username) throws NotFoundException { @@ -65,7 +62,7 @@ public class DeviceService { .filter( a -> { final List> conditions = - conditionRepository.findAllByAutomationId(a.getId()); + automationService.findAllConditionsByAutomationId(a.getId()); if (conditions.isEmpty()) return true; return conditions.stream().allMatch(Condition::triggered); }) diff --git a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/ButtonDimmerControllerTests.java b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/ButtonDimmerControllerTests.java index 39f5b16..25cbb91 100644 --- a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/ButtonDimmerControllerTests.java +++ b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/ButtonDimmerControllerTests.java @@ -15,6 +15,7 @@ import java.security.Principal; import java.util.Optional; import java.util.Set; import lombok.SneakyThrows; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -73,7 +74,7 @@ public class ButtonDimmerControllerTests { doNothing().when(service).deleteByIdAsOwner(eq(42L), eq("user")); MockHttpServletRequest request = new MockHttpServletRequest(); RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request)); - controller.delete(42L, principal); + Assertions.assertDoesNotThrow(() -> controller.delete(42L, principal)); } @Test diff --git a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/CurtainsControllerTests.java b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/CurtainsControllerTests.java index eebab51..448a06e 100644 --- a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/CurtainsControllerTests.java +++ b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/CurtainsControllerTests.java @@ -15,6 +15,7 @@ import ch.usi.inf.sa4.sanmarinoes.smarthut.service.DeviceService; import java.security.Principal; import java.util.Optional; import lombok.SneakyThrows; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -91,11 +92,10 @@ public class CurtainsControllerTests { doNothing().when(deviceService).deleteByIdAsOwner(eq(42L), eq("user")); MockHttpServletRequest request = new MockHttpServletRequest(); RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request)); - controller.delete(42L, principal); + Assertions.assertDoesNotThrow(() -> controller.delete(42L, principal)); } @Test - @SneakyThrows({NotFoundException.class, DuplicateStateException.class}) public void testSceneBinding() { when(principal.getName()).thenReturn("user"); Curtains curtains = new Curtains(); @@ -107,7 +107,7 @@ public class CurtainsControllerTests { State s = curtains.cloneState(); when(sceneRepository.findById(1L)).thenReturn(Optional.of(scene)); when(stateRepository.countByDeviceIdAndSceneId(24L, 1L)).thenReturn(0); - controller.sceneBinding(24L, 1L, principal); + Assertions.assertDoesNotThrow(() -> controller.sceneBinding(24L, 1L, principal)); } @Test diff --git a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/GuestControllerTests.java b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/GuestControllerTests.java index 6170571..96034ac 100644 --- a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/GuestControllerTests.java +++ b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/GuestControllerTests.java @@ -43,7 +43,7 @@ public class GuestControllerTests { @Test public void findAllEmptyTest() { List l = guestController.findAll(); - assertThat(l.isEmpty()); + assertThat(l).isEmpty(); } @DisplayName( @@ -85,7 +85,7 @@ public class GuestControllerTests { public void findHostsEmptyTest() { when(mockPrincipal.getName()).thenReturn("user"); when(userRepository.findByUsername("user")).thenReturn(user); - assertThat(guestController.findHosts(mockPrincipal).isEmpty()); + assertThat(guestController.findHosts(mockPrincipal)).isEmpty(); } @DisplayName("Check that the guest list is empty") @@ -93,7 +93,7 @@ public class GuestControllerTests { public void findGuestsEmptyTest() { when(mockPrincipal.getName()).thenReturn("user"); when(userRepository.findByUsername("user")).thenReturn(user); - assertThat(guestController.findGuests(mockPrincipal).isEmpty()); + assertThat(guestController.findGuests(mockPrincipal)).isEmpty(); } @Test diff --git a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/KnobDimmerControllerTests.java b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/KnobDimmerControllerTests.java index 3e17d48..f796312 100644 --- a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/KnobDimmerControllerTests.java +++ b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/KnobDimmerControllerTests.java @@ -15,6 +15,7 @@ import java.security.Principal; import java.util.Optional; import java.util.Set; import lombok.SneakyThrows; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -73,7 +74,7 @@ public class KnobDimmerControllerTests { doNothing().when(deviceService).deleteByIdAsOwner(eq(42L), eq("user")); MockHttpServletRequest request = new MockHttpServletRequest(); RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request)); - controller.delete(42L, principal); + Assertions.assertDoesNotThrow(() -> controller.delete(42L, principal)); } @Test diff --git a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SecurityCameraControllerTests.java b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SecurityCameraControllerTests.java index 5e2d429..1c7760b 100644 --- a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SecurityCameraControllerTests.java +++ b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SecurityCameraControllerTests.java @@ -16,6 +16,7 @@ import ch.usi.inf.sa4.sanmarinoes.smarthut.service.DeviceService; import java.security.Principal; import java.util.Optional; import lombok.SneakyThrows; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -96,11 +97,10 @@ public class SecurityCameraControllerTests { doNothing().when(deviceService).deleteByIdAsOwner(eq(42L), eq("user")); MockHttpServletRequest request = new MockHttpServletRequest(); RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request)); - controller.delete(42L, principal); + Assertions.assertDoesNotThrow(() -> controller.delete(42L, principal)); } @Test - @SneakyThrows({NotFoundException.class, DuplicateStateException.class}) public void testSceneBinding() { when(principal.getName()).thenReturn("user"); SecurityCamera camera = new SecurityCamera(); @@ -114,7 +114,7 @@ public class SecurityCameraControllerTests { when(sceneRepository.findById(1L)).thenReturn(Optional.of(scene)); when(stateRepository.countByDeviceIdAndSceneId(24L, 1L)).thenReturn(0); // when(stateRepository.save(eq(state))).thenReturn(state); - controller.sceneBinding(24L, 1L, principal); + Assertions.assertDoesNotThrow(() -> controller.sceneBinding(24L, 1L, principal)); } @Test diff --git a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SwitchControllerTests.java b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SwitchControllerTests.java index f0c985f..6d3787d 100644 --- a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SwitchControllerTests.java +++ b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SwitchControllerTests.java @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Optional; import lombok.SneakyThrows; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -77,7 +78,7 @@ public class SwitchControllerTests { doNothing().when(deviceService).deleteByIdAsOwner(eq(42L), eq("user")); MockHttpServletRequest request = new MockHttpServletRequest(); RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request)); - controller.deleteById(42L, principal); + Assertions.assertDoesNotThrow(() -> controller.deleteById(42L, principal)); } @Test diff --git a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/DeviceServiceTests.java b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/DeviceServiceTests.java index cc61142..01000ea 100644 --- a/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/DeviceServiceTests.java +++ b/src/test/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/DeviceServiceTests.java @@ -33,8 +33,6 @@ public class DeviceServiceTests { @Mock private DevicePropagationService devicePropagationService; - @Mock private ConditionRepository> conditionRepository; - @InjectMocks private DeviceService deviceService; @Test @@ -101,7 +99,7 @@ public class DeviceServiceTests { a.getScenes().add(sp); when(automationService.findTriggersByDeviceId(1L)).thenReturn(List.of(b)); - when(conditionRepository.findAllByAutomationId(5L)).thenReturn(List.of(c)); + when(automationService.findAllConditionsByAutomationId(5L)).thenReturn(List.of(c)); when(automationService.findByVerifiedId(5L)).thenReturn(a); when(sceneService.findByValidatedId(4L)).thenReturn(s);