From db21aa0a26528ca58f7d69f962036a7a2cfcba62 Mon Sep 17 00:00:00 2001 From: Claudio Maggioni Date: Tue, 21 Apr 2020 17:36:17 +0200 Subject: [PATCH] Code review --- .../smarthut/controller/SwitchController.java | 17 +++++----- .../smarthut/models/Automation.java | 12 +++---- .../smarthut/models/BooleanTrigger.java | 2 +- .../sanmarinoes/smarthut/models/Device.java | 5 +++ .../smarthut/models/InputDevice.java | 7 ---- .../smarthut/models/RangeTrigger.java | 33 +++++++++++++++---- .../sanmarinoes/smarthut/models/Scene.java | 10 +----- .../sanmarinoes/smarthut/models/Trigger.java | 7 ++-- .../smarthut/service/DeviceService.java | 14 ++++++-- 9 files changed, 60 insertions(+), 47 deletions(-) diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SwitchController.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SwitchController.java index 3a87b18..3bf41b8 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SwitchController.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/SwitchController.java @@ -6,10 +6,10 @@ import ch.usi.inf.sa4.sanmarinoes.smarthut.dto.GenericDeviceSaveReguest; import ch.usi.inf.sa4.sanmarinoes.smarthut.dto.SwitchOperationRequest; 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.DeviceService; import java.security.Principal; import java.util.*; import java.util.List; -import java.util.stream.Collectors; import javax.validation.Valid; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.*; @@ -21,7 +21,7 @@ import org.springframework.web.bind.annotation.*; public class SwitchController extends InputDeviceConnectionController { private SwitchRepository switchRepository; - private SwitchableRepository switchableRepository; + private DeviceService deviceService; /** * Contstructs the controller by requiring essential object for the controller implementation @@ -31,10 +31,12 @@ public class SwitchController extends InputDeviceConnectionController outputRepository) { + SwitchRepository inputRepository, + SwitchableRepository outputRepository, + DeviceService deviceService) { super(inputRepository, outputRepository, Switchable.SWITCH_SWITCHABLE_CONNECTOR); this.switchRepository = inputRepository; - this.switchableRepository = outputRepository; + this.deviceService = deviceService; } @GetMapping @@ -57,7 +59,7 @@ public class SwitchController extends InputDeviceConnectionController operate( + public List operate( @Valid @RequestBody final SwitchOperationRequest sr, final Principal principal) throws NotFoundException { final Switch s = @@ -77,9 +79,8 @@ public class SwitchController extends InputDeviceConnectionController> triggers = new HashSet<>(); - @OneToMany(mappedBy = "scenes", cascade = CascadeType.REMOVE) + @OneToMany(mappedBy = "automation", cascade = CascadeType.REMOVE) @GsonExclude private Set scenes = new HashSet<>(); diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/BooleanTrigger.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/BooleanTrigger.java index db83c94..f52e5ca 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/BooleanTrigger.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/BooleanTrigger.java @@ -25,7 +25,7 @@ public class BooleanTrigger extends Trigg } @Override - public boolean check() { + public boolean triggered() { return getDevice().readTriggerState() == isOn(); } } diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Device.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Device.java index 0114ac3..1787a67 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Device.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Device.java @@ -34,6 +34,11 @@ public abstract class Device { @GsonExclude private Room room; + @OneToMany(mappedBy = "device", orphanRemoval = true) + @GsonExclude + // @SocketGsonExclude + private Set> triggers = new HashSet<>(); + /** * The room this device belongs in, as a foreign key id. To use when updating and inserting from * a REST call. diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/InputDevice.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/InputDevice.java index 0623872..2acf0c2 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/InputDevice.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/InputDevice.java @@ -1,12 +1,9 @@ package ch.usi.inf.sa4.sanmarinoes.smarthut.models; -import ch.usi.inf.sa4.sanmarinoes.smarthut.config.GsonExclude; -import java.util.HashSet; import java.util.Set; import javax.persistence.Entity; import javax.persistence.Inheritance; import javax.persistence.InheritanceType; -import javax.persistence.OneToMany; /** * A generic abstraction for an input device, i.e. something that captures input either from the @@ -15,10 +12,6 @@ import javax.persistence.OneToMany; @Entity @Inheritance(strategy = InheritanceType.JOINED) public abstract class InputDevice extends Device { - @OneToMany(mappedBy = "scene", orphanRemoval = true) - @GsonExclude - private Set> triggers = new HashSet<>(); - public InputDevice(String kind) { super(kind, FlowType.INPUT); } diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/RangeTrigger.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/RangeTrigger.java index 6c34f80..73c86eb 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/RangeTrigger.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/RangeTrigger.java @@ -1,5 +1,6 @@ package ch.usi.inf.sa4.sanmarinoes.smarthut.models; +import com.google.gson.annotations.SerializedName; import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.Inheritance; @@ -8,28 +9,46 @@ import javax.validation.constraints.NotNull; @Entity @Inheritance(strategy = InheritanceType.JOINED) -public class RangeTrigger extends Trigger { +public class RangeTrigger extends Trigger { @Override - public boolean check(T d) { - return false; + public boolean triggered() { + double value = getDevice().readTriggerState(); + switch (operator) { + case EQUAL: + return value == range; + case LESS: + return value < range; + case GREATER: + return value > range; + case GREATER_EQUAL: + return value >= range; + case LESS_EQUAL: + return value <= range; + } + throw new IllegalStateException(); } enum Operator { + @SerializedName("EQUAL") EQUAL, + @SerializedName("LESS") LESS, + @SerializedName("GREATER") GREATER, + @SerializedName("LESS_EQUAL") LESS_EQUAL, - GREATER_EQUAL; + @SerializedName("GREATER_EQUAL") + GREATER_EQUAL } @NotNull @Column(nullable = false) - Operator operator; + private Operator operator; @NotNull @Column(nullable = false) - private Float range; + private double range; public Operator getOperator() { return operator; @@ -39,7 +58,7 @@ public class RangeTrigger extends Trigger { this.operator = operator; } - public Float getRange() { + public double getRange() { return range; } diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Scene.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Scene.java index 439acdb..c4b6213 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Scene.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Scene.java @@ -30,7 +30,7 @@ public class Scene { @GsonExclude private Long userId; - @OneToMany(mappedBy = "states", orphanRemoval = true) + @OneToMany(mappedBy = "scene", orphanRemoval = true) @GsonExclude private Set> states = new HashSet<>(); @@ -39,10 +39,6 @@ public class Scene { @Column(nullable = false) private String name; - @ManyToMany(mappedBy = "automations", cascade = CascadeType.DETACH) - @GsonExclude - private Set automations = new HashSet<>(); - public String getName() { return name; } @@ -82,8 +78,4 @@ public class Scene { public void setUserId(Long userId) { this.userId = userId; } - - public Set getAutomations() { - return automations; - } } diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Trigger.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Trigger.java index f42658d..1ef68f0 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Trigger.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/Trigger.java @@ -12,15 +12,14 @@ import javax.persistence.InheritanceType; import javax.persistence.JoinColumn; import javax.persistence.ManyToOne; import javax.persistence.PreRemove; -import javax.persistence.Table; -import javax.persistence.UniqueConstraint; import javax.validation.constraints.NotNull; @Entity -@Table(uniqueConstraints = {@UniqueConstraint(columnNames = {"device_id", "scene_id"})}) @Inheritance(strategy = InheritanceType.SINGLE_TABLE) public abstract class Trigger { + public abstract boolean triggered(); + @Id @GeneratedValue(strategy = GenerationType.AUTO) @Column(name = "id", updatable = false, nullable = false, unique = true) @@ -89,8 +88,6 @@ public abstract class Trigger { this.automationId = automationId; } - public abstract boolean check(T d); - @PreRemove public void removeDeviceAndScene() { this.setDevice(null); 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 d8bbb0d..04512e5 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 @@ -4,7 +4,9 @@ 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.Trigger; import ch.usi.inf.sa4.sanmarinoes.smarthut.models.TriggerRepository; +import java.util.Collection; import java.util.List; +import java.util.stream.Collectors; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -26,17 +28,25 @@ public class DeviceService { @Autowired DeviceRepository deviceRepository; @Autowired TriggerRepository> triggerRepository; + public List saveAll(Collection devices) { + return devices.stream().map(this::save).collect(Collectors.toList()); + } + public T save(T device) { final long deviceId = device.getId(); - List> triggers = - triggerRepository.findAllByDeviceId(deviceId); + List> triggers = triggerRepository.findAllByDeviceId(deviceId); + + List> triggeredTriggers = + triggers.stream().filter(Trigger::triggered).collect(Collectors.toList()); // map activated -> automations // remove duplicates // sort automations per priority // sort scenes inside automations per priority // apply scenes (SceneService.apply()) + + return deviceRepository.save(device); } }