From 37ea9591cfe025849b897005466243d16881a233 Mon Sep 17 00:00:00 2001 From: "Claudio Maggioni (maggicl)" Date: Wed, 22 Apr 2020 23:04:43 +0200 Subject: [PATCH] Code review --- .../smarthut/controller/SceneController.java | 35 ++++++++----------- .../smarthut/service/DeviceService.java | 30 ++++++++-------- .../smarthut/service/SceneService.java | 28 +++++++++++++++ 3 files changed, 56 insertions(+), 37 deletions(-) create mode 100644 src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/SceneService.java diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SceneController.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SceneController.java index 2b4c9c1..e9174af 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SceneController.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SceneController.java @@ -5,8 +5,8 @@ import static ch.usi.inf.sa4.sanmarinoes.smarthut.utils.Utils.toList; import ch.usi.inf.sa4.sanmarinoes.smarthut.dto.SceneSaveRequest; import ch.usi.inf.sa4.sanmarinoes.smarthut.error.NotFoundException; import ch.usi.inf.sa4.sanmarinoes.smarthut.models.*; +import ch.usi.inf.sa4.sanmarinoes.smarthut.service.SceneService; import java.security.Principal; -import java.util.ArrayList; import java.util.List; import javax.validation.Valid; import org.springframework.beans.factory.annotation.Autowired; @@ -26,20 +26,21 @@ import org.springframework.web.bind.annotation.RestController; @RequestMapping("/scene") public class SceneController { - @Autowired SceneRepository sceneService; - @Autowired UserRepository userService; - @Autowired StateRepository> stateService; - @Autowired DeviceRepository deviceRepository; + @Autowired private SceneRepository sceneRepository; + @Autowired private SceneService sceneService; + @Autowired private UserRepository userService; + @Autowired private StateRepository> stateService; + @Autowired private DeviceRepository deviceRepository; @GetMapping public List findAll(Principal principal) { - return toList(sceneService.findByUsername(principal.getName())); + return toList(sceneRepository.findByUsername(principal.getName())); } @GetMapping("/{id}") public @ResponseBody Scene findById(@PathVariable("id") long id, Principal principal) throws NotFoundException { - return sceneService + return sceneRepository .findByIdAndUsername(id, principal.getName()) .orElseThrow(NotFoundException::new); } @@ -56,26 +57,18 @@ public class SceneController { newScene.setUserId(userId); newScene.setName(s.getName()); - return sceneService.save(newScene); + return sceneRepository.save(newScene); } @PostMapping("/{id}/apply") public @ResponseBody List apply(@PathVariable("id") long id, final Principal principal) throws NotFoundException { final Scene newScene = - sceneService + sceneRepository .findByIdAndUsername(id, principal.getName()) .orElseThrow(NotFoundException::new); - final List updated = new ArrayList<>(); - - for (final State s : newScene.getStates()) { - s.apply(); - updated.add(s.getDevice()); - } - deviceRepository.saveAll(updated); - - return updated; + return sceneService.apply(newScene); } @PutMapping("/{id}") @@ -83,7 +76,7 @@ public class SceneController { @PathVariable("id") long id, @RequestBody SceneSaveRequest s, final Principal principal) throws NotFoundException { final Scene newScene = - sceneService + sceneRepository .findByIdAndUsername(id, principal.getName()) .orElseThrow(NotFoundException::new); @@ -91,13 +84,13 @@ public class SceneController { newScene.setName(s.getName()); } - return sceneService.save(newScene); + return sceneRepository.save(newScene); } @DeleteMapping("/{id}") public void deleteById(@PathVariable("id") long id) { stateService.deleteAllBySceneId(id); - sceneService.deleteById(id); + sceneRepository.deleteById(id); } /** 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 273fe09..137d0e3 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 @@ -1,12 +1,6 @@ package ch.usi.inf.sa4.sanmarinoes.smarthut.service; -import ch.usi.inf.sa4.sanmarinoes.smarthut.models.AutomationRepository; -import ch.usi.inf.sa4.sanmarinoes.smarthut.models.Device; -import ch.usi.inf.sa4.sanmarinoes.smarthut.models.DeviceRepository; -import ch.usi.inf.sa4.sanmarinoes.smarthut.models.ScenePriority; -import ch.usi.inf.sa4.sanmarinoes.smarthut.models.SceneRepository; -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.Collection; import java.util.Comparator; import java.util.List; @@ -17,11 +11,11 @@ import org.springframework.stereotype.Component; @Component public class DeviceService { - @Autowired DeviceRepository deviceRepository; - @Autowired AutomationRepository automationRepository; - @Autowired SceneRepository sceneRepository; - // @Autowired SceneService sceneService; - @Autowired TriggerRepository> triggerRepository; + @Autowired private DeviceRepository deviceRepository; + @Autowired private AutomationRepository automationRepository; + @Autowired private SceneRepository sceneRepository; + @Autowired private SceneService sceneService; + @Autowired private TriggerRepository> triggerRepository; public List saveAll(Collection devices) { return devices.stream().map(this::save).collect(Collectors.toList()); @@ -36,14 +30,18 @@ public class DeviceService { triggers.stream() .filter(Trigger::triggered) .map(Trigger::getAutomationId) - .map(t -> automationRepository.findById(t)) + .map(t -> automationRepository.findById(t).orElseThrow(IllegalStateException::new)) .distinct() - .map(t -> t.get().getScenes()) + .map(Automation::getScenes) .flatMap(Collection::stream) .distinct() .sorted(Comparator.comparing(ScenePriority::getPriority)) - .map(t -> sceneRepository.findById(t.getSceneId())) - .forEach(SceneService::apply); + .map( + t -> + sceneRepository + .findById(t.getSceneId()) + .orElseThrow(IllegalStateException::new)) + .forEach(sceneService::apply); // map activated -> automations // remove duplicates diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/SceneService.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/SceneService.java new file mode 100644 index 0000000..826d896 --- /dev/null +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/service/SceneService.java @@ -0,0 +1,28 @@ +package ch.usi.inf.sa4.sanmarinoes.smarthut.service; + +import ch.usi.inf.sa4.sanmarinoes.smarthut.models.Device; +import ch.usi.inf.sa4.sanmarinoes.smarthut.models.DeviceRepository; +import ch.usi.inf.sa4.sanmarinoes.smarthut.models.Scene; +import ch.usi.inf.sa4.sanmarinoes.smarthut.models.State; +import java.util.ArrayList; +import java.util.List; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +@Component +public class SceneService { + + @Autowired private DeviceRepository deviceRepository; + + public List apply(Scene newScene) { + final List updated = new ArrayList<>(); + + for (final State s : newScene.getStates()) { + s.apply(); + updated.add(s.getDevice()); + } + deviceRepository.saveAll(updated); + + return updated; + } +}