From 3894fa14a9b9f04ebe79a6bce44d5d92681079a0 Mon Sep 17 00:00:00 2001 From: "Claudio Maggioni (maggicl)" Date: Wed, 22 Apr 2020 22:54:34 +0200 Subject: [PATCH] Code review --- .../smarthut/config/SpringFoxConfig.java | 1 + .../smarthut/controller/DeviceController.java | 6 +- .../smarthut/controller/GuestController.java | 26 ++++---- .../smarthut/controller/RoomController.java | 25 ++++---- .../sa4/sanmarinoes/smarthut/models/User.java | 18 +++--- .../smarthut/service/DeviceService.java | 60 +++++++++++++++++-- 6 files changed, 95 insertions(+), 41 deletions(-) diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/config/SpringFoxConfig.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/config/SpringFoxConfig.java index 2f45d22..d09adfb 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/config/SpringFoxConfig.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/config/SpringFoxConfig.java @@ -81,6 +81,7 @@ public class SpringFoxConfig { .or(PathSelectors.regex("/switch.*")::apply) .or(PathSelectors.regex("/motionSensor.*")::apply) .or(PathSelectors.regex("/curtains.*")::apply) + .or(PathSelectors.regex("/user.*")::apply) .or(PathSelectors.regex("/auth/profile.*")::apply); } diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/DeviceController.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/DeviceController.java index ac1acd4..b53b0af 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/DeviceController.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/DeviceController.java @@ -24,8 +24,10 @@ public class DeviceController { @Autowired private RoomRepository roomRepository; @GetMapping - public List getAll(final Principal user) { - return deviceRepository.findAllByUsername(user.getName()); + public List getAll( + @RequestParam(value = "hostId", required = false) Long hostId, final Principal user) + throws NotFoundException { + return deviceService.findAll(hostId, user.getName()); } @PutMapping diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/GuestController.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/GuestController.java index e0fc3e9..b0994a1 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/GuestController.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/GuestController.java @@ -10,8 +10,12 @@ import java.security.Principal; import java.util.List; import javax.validation.Valid; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.web.bind.annotation.*; +@RestController +@EnableAutoConfiguration +@RequestMapping("/user") public class GuestController { @Autowired private UserRepository userRepository; @@ -21,31 +25,29 @@ public class GuestController { return toList(userRepository.findAll()); } - @PostMapping - public User addUserAsGuest(long id, final Principal principal) throws NotFoundException { + @PostMapping("/guest") + public User addUserAsGuest(@RequestParam("userId") long id, final Principal principal) + throws NotFoundException { User guest = userRepository.findById(id).orElseThrow(NotFoundException::new); User host = userRepository.findByUsername(principal.getName()); host.addGuest(guest); guest.addHost(host); userRepository.save(guest); - /* Not sure if this is useful. userRepository.save(guest); */ return userRepository.save(host); } - public User save(GuestPermissionsRequest g, User currentUser) { + @PutMapping("/permissions") + public User updatePermissions( + @Valid @RequestBody GuestPermissionsRequest g, final Principal principal) { + final User currentUser = userRepository.findByUsername(principal.getName()); currentUser.setCameraEnabled(g.isCameraEnabled()); return userRepository.save(currentUser); } - @PutMapping - public User updatePermissions( - @Valid @RequestBody GuestPermissionsRequest g, final Principal principal) { - return this.save(g, userRepository.findByUsername(principal.getName())); - } - - @DeleteMapping - public void removeUserAsGuest(long id, final Principal principal) throws NotFoundException { + @DeleteMapping("/guest") + public void removeUserAsGuest(@RequestParam("userId") long id, final Principal principal) + throws NotFoundException { User guest = userRepository.findById(id).orElseThrow(NotFoundException::new); User host = userRepository.findByUsername(principal.getName()); diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/RoomController.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/RoomController.java index 7de58db..136446a 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/RoomController.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/RoomController.java @@ -5,6 +5,7 @@ import static ch.usi.inf.sa4.sanmarinoes.smarthut.utils.Utils.toList; import ch.usi.inf.sa4.sanmarinoes.smarthut.dto.RoomSaveRequest; 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 ch.usi.inf.sa4.sanmarinoes.smarthut.service.ThermostatService; import ch.usi.inf.sa4.sanmarinoes.smarthut.utils.Utils; import java.security.Principal; @@ -23,7 +24,7 @@ public class RoomController { @Autowired private UserRepository userRepository; - @Autowired private DeviceRepository deviceRepository; + @Autowired private DeviceService deviceService; @Autowired private SwitchRepository switchRepository; @@ -43,7 +44,10 @@ public class RoomController { } @GetMapping - public List findAll(Long hostId, final Principal principal) throws NotFoundException { + public List findAll( + @RequestParam(value = "hostId", required = false) Long hostId, + final Principal principal) + throws NotFoundException { List rooms = toList(roomRepository.findAll()); return fetchOwnerOrGuest(rooms, hostId, principal); @@ -51,7 +55,9 @@ public class RoomController { @GetMapping("/{id}") public @ResponseBody Room findById( - @PathVariable("id") long id, final Principal principal, Long hostId) + @PathVariable("id") long id, + final Principal principal, + @RequestParam(value = "hostId", required = false) Long hostId) throws NotFoundException { Room room = roomRepository.findById(id).orElseThrow(NotFoundException::new); /* Very ugly way of avoiding code duplication. If this method call throws no exception, @@ -122,15 +128,10 @@ public class RoomController { */ @GetMapping(path = "/{roomId}/devices") public List getDevices( - @PathVariable("roomId") long roomid, final Principal principal, Long hostId) + @PathVariable("roomId") long roomId, + final Principal principal, + @RequestParam(value = "hostId", required = false) Long hostId) throws NotFoundException { - Iterable devices = deviceRepository.findByRoomId(roomid); - for (Device d : devices) { - if (d instanceof Thermostat) { - thermostatService.populateMeasuredTemperature((Thermostat) d); - } - } - List deviceList = toList(devices); - return fetchOwnerOrGuest(deviceList, hostId, principal); + return deviceService.findAll(roomId, hostId, principal.getName()); } } diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/User.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/User.java index 5880ec6..b58e383 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/User.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/models/User.java @@ -38,11 +38,7 @@ public class User { private String email; /** Guests invited by this user */ - @ManyToMany(cascade = CascadeType.DETACH) - @JoinTable( - name = "invited", - joinColumns = @JoinColumn(name = "host_id"), - inverseJoinColumns = @JoinColumn(name = "guest_id")) + @ManyToMany(mappedBy = "hosts", cascade = CascadeType.DETACH) @GsonExclude private Set guests = new HashSet<>(); @@ -55,7 +51,8 @@ public class User { private Set hosts = new HashSet<>(); /** Determines whether a guest can access security cameras */ - @Column private boolean cameraEnabled; + @Column(nullable = false) + private boolean cameraEnabled; @Column(nullable = false) @GsonExclude @@ -130,9 +127,7 @@ public class User { } public void removeGuest(User guest) { - if (this.guests.contains(guest)) { - this.guests.remove(guest); - } + this.guests.remove(guest); } public void setCameraEnabled(boolean cameraEnabled) { @@ -141,8 +136,7 @@ public class User { @Override public String toString() { - return "User{" - + "id=" + return "User{id=" + id + ", name='" + name @@ -156,6 +150,8 @@ public class User { + ", email='" + email + '\'' + + ", cameraEnabled=" + + cameraEnabled + ", isEnabled=" + isEnabled + '}'; 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 1ed0ea4..f95cb21 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,11 +1,11 @@ package ch.usi.inf.sa4.sanmarinoes.smarthut.service; +import static ch.usi.inf.sa4.sanmarinoes.smarthut.utils.Utils.toList; + import ch.usi.inf.sa4.sanmarinoes.smarthut.error.NotFoundException; -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.User; -import ch.usi.inf.sa4.sanmarinoes.smarthut.models.UserRepository; +import ch.usi.inf.sa4.sanmarinoes.smarthut.models.*; import ch.usi.inf.sa4.sanmarinoes.smarthut.socket.SensorSocketEndpoint; +import java.util.List; import java.util.Set; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -15,8 +15,60 @@ public class DeviceService { // FIXME: TO BE MERGED WITH USER STORY 5 (MATTEO'S STUFF) @Autowired private DeviceRepository deviceRepository; + @Autowired private RoomRepository roomRepository; @Autowired private UserRepository userRepository; @Autowired private SensorSocketEndpoint endpoint; + @Autowired private ThermostatService thermostatService; + + public List findAll(Long hostId, String username) throws NotFoundException { + return findAll(null, hostId, username); + } + + public List findAll(Long roomId, Long hostId, String username) + throws NotFoundException { + try { + Iterable devices; + if (hostId == null) { + if (roomId != null) { + roomRepository + .findByIdAndUsername(roomId, username) + .orElseThrow(NotFoundException::new); + devices = deviceRepository.findByRoomId(roomId); + } else { + devices = deviceRepository.findAllByUsername(username); + } + } else { + final User guest = userRepository.findByUsername(username); + final User host = + userRepository.findById(hostId).orElseThrow(NotFoundException::new); + + if (!guest.getHosts().contains(host)) { + throw new NotFoundException(); + } + + if (roomId != null) { + Room r = roomRepository.findById(roomId).orElseThrow(NotFoundException::new); + if (!r.getUserId().equals(hostId)) { + throw new NotFoundException(); + } + devices = deviceRepository.findByRoomId(roomId); + } else { + devices = deviceRepository.findAllByUsername(host.getUsername()); + } + } + + for (Device d : devices) { + if (d instanceof Thermostat) { + thermostatService.populateMeasuredTemperature((Thermostat) d); + } + } + + return toList(devices); + } catch (NotFoundException e) { + e.printStackTrace(); + throw e; + } + } public T saveAsGuest(T device, String guestUsername, Long hostId) throws NotFoundException {