From c4f295d7d9b1931fd12b2aa4848e266a92bc8167 Mon Sep 17 00:00:00 2001 From: tommi27 Date: Tue, 21 Apr 2020 17:09:11 +0200 Subject: [PATCH] refactored controllers code --- .../controller/RegularLightController.java | 65 ++++++++------- .../smarthut/controller/RoomController.java | 80 +++++++------------ 2 files changed, 63 insertions(+), 82 deletions(-) diff --git a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/RegularLightController.java b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/RegularLightController.java index 7016030..06fccfb 100644 --- a/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/RegularLightController.java +++ b/src/main/java/ch/usi/inf/sa4/sanmarinoes/smarthut/controller/RegularLightController.java @@ -6,9 +6,9 @@ import ch.usi.inf.sa4.sanmarinoes.smarthut.dto.SwitchableSaveRequest; import ch.usi.inf.sa4.sanmarinoes.smarthut.error.DuplicateStateException; 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.List; -import java.util.Optional; import javax.validation.Valid; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; @@ -28,10 +28,30 @@ import org.springframework.web.bind.annotation.RestController; public class RegularLightController { @Autowired private UserRepository userRepository; - @Autowired private RoomRepository roomRepository; @Autowired private RegularLightRepository regularLightService; @Autowired private SceneRepository sceneRepository; @Autowired private StateRepository> stateRepository; + @Autowired private DeviceService deviceService; + + private RegularLight fetchIfOwnerOrGuest(final Principal principal, Long id, Long hostId) + throws NotFoundException { + if (hostId == null) { + return regularLightService.findById(id).orElseThrow(NotFoundException::new); + } else { + RegularLight rl = + regularLightService + .findByIdAndUserId(id, hostId) + .orElseThrow(NotFoundException::new); + User host = userRepository.findById(hostId).orElseThrow(IllegalStateException::new); + User guest = userRepository.findByUsername(principal.getName()); + rl.setFromHost(true); + if (!host.getGuests().contains(guest)) { + throw new NotFoundException(); + } else { + return rl; + } + } + } @GetMapping public List findAll() { @@ -43,46 +63,25 @@ public class RegularLightController { return regularLightService.findById(id).orElseThrow(NotFoundException::new); } - private RegularLight save(RegularLight newRL, SwitchableSaveRequest rl) { - newRL.setName(rl.getName()); - newRL.setRoomId(rl.getRoomId()); - newRL.setOn(rl.isOn()); + private RegularLight save(RegularLight initial, SwitchableSaveRequest rl, String username) { + initial.setName(rl.getName()); + initial.setRoomId(rl.getRoomId()); + initial.setOn(rl.isOn()); - return regularLightService.save(newRL); + return deviceService.saveAsOwner(initial, username); } @PostMapping - public RegularLight create(@Valid @RequestBody SwitchableSaveRequest rl) { - return save(new RegularLight(), rl); + public RegularLight create( + @Valid @RequestBody SwitchableSaveRequest rl, final Principal principal) { + return save(new RegularLight(), rl, principal.getName()); } @PutMapping public RegularLight update( - @Valid @RequestBody SwitchableSaveRequest rl, - final Principal principal, - Optional guestId) + @Valid @RequestBody SwitchableSaveRequest rl, final Principal principal, Long hostId) throws NotFoundException { - - /** Extremely verbose check for guest/user authorization */ - if (guestId.isPresent() - && userRepository - .findById( - roomRepository - .findById(rl.getRoomId()) - .get() - .getUserId() - .longValue()) - .get() - .getGuests() - .contains(userRepository.findById(guestId.get().longValue()))) { - return save( - regularLightService - .findByIdAndUsername(rl.getId(), principal.getName()) - .orElseThrow(NotFoundException::new), - rl); - } else { - throw new Error("401: Unauthorized user. Not a guest."); - } + return save(fetchIfOwnerOrGuest(principal, rl.getId(), hostId), rl, principal.getName()); } @DeleteMapping("/{id}") 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 734ecce..03a115f 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 @@ -32,39 +32,39 @@ public class RoomController { @Autowired private ThermostatService thermostatService; - @GetMapping - public List findAll(Optional guestId) { - - List rooms = toList(roomRepository.findAll()); - - if (guestId.isPresent() - && !rooms.isEmpty() - && userRepository - .findById(rooms.get(0).getUserId()) - .get() - .getGuests() - .contains(userRepository.findById(guestId.get().longValue()))) { - return rooms; + private List fetchOwnerOrGuest( + final List list, Long hostId, final Principal principal) throws NotFoundException { + if (hostId == null) { + return list; } else { - throw new Error("401: Unauthorized user. Not a guest."); + User host = userRepository.findById(hostId).orElseThrow(NotFoundException::new); + User guest = userRepository.findByUsername(principal.getName()); + if (!host.getGuests().contains(guest)) { + throw new NotFoundException(); + } else { + return list; + } } } + @GetMapping + public List findAll(Long hostId, final Principal principal) throws NotFoundException { + + List rooms = toList(roomRepository.findAll()); + return fetchOwnerOrGuest(rooms, hostId, principal); + } + @GetMapping("/{id}") - public @ResponseBody Room findById(@PathVariable("id") long id, Optional guestId) + public @ResponseBody Room findById( + @PathVariable("id") long id, final Principal principal, Long hostId) throws NotFoundException { Room room = roomRepository.findById(id).orElseThrow(NotFoundException::new); - - if (guestId.isPresent() - && userRepository - .findById(room.getUserId().longValue()) - .get() - .getGuests() - .contains(userRepository.findById(guestId.get()))) { - return room; - } else { - throw new Error("401: Unauthorized user. Not a guest."); - } + /* Very ugly way of avoiding code duplication. If this method call throws no exception, + * we can return the room safely. I pass null as I do not return a list in this case. + * Refer to fetchOwnerOrGuest for further information. + */ + fetchOwnerOrGuest(null, hostId, principal); + return room; } @PostMapping @@ -126,34 +126,16 @@ public class RoomController { * id). */ @GetMapping(path = "/{roomId}/devices") - public List getDevices(@PathVariable("roomId") long roomid, Optional guestId) { + public List getDevices( + @PathVariable("roomId") long roomid, final Principal principal, Long hostId) + throws NotFoundException { Iterable devices = deviceRepository.findByRoomId(roomid); for (Device d : devices) { if (d instanceof Thermostat) { thermostatService.populateMeasuredTemperature((Thermostat) d); } } - List dl = toList(devices); - - /** - * Extremely verbose method calls to find the current user and check if the optional user is - * one of their guests - */ - if (guestId.isPresent() - && !dl.isEmpty() - && userRepository - .findById( - roomRepository - .findById(dl.get(0).getRoomId().longValue()) - .get() - .getUserId() - .longValue()) - .get() - .getGuests() - .contains(userRepository.findById(guestId.get().longValue()))) { - return dl; - } else { - throw new Error("401: Unauthorized user. Not a guest."); - } + List deviceList = toList(devices); + return fetchOwnerOrGuest(deviceList, hostId, principal); } }