refactored controllers code

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<State<?>> 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<RegularLight> 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<Long> 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}")

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<Room> findAll(Optional<Long> guestId) {
-
-        List<Room> 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 <T> List<T> fetchOwnerOrGuest(
+            final List<T> 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<Room> findAll(Long hostId, final Principal principal) throws NotFoundException {
+
+        List<Room> rooms = toList(roomRepository.findAll());
+        return fetchOwnerOrGuest(rooms, hostId, principal);
+    }
+
     @GetMapping("/{id}")
-    public @ResponseBody Room findById(@PathVariable("id") long id, Optional<Long> 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<Device> getDevices(@PathVariable("roomId") long roomid, Optional<Long> guestId) {
+    public List<Device> getDevices(
+            @PathVariable("roomId") long roomid, final Principal principal, Long hostId)
+            throws NotFoundException {
         Iterable<Device> devices = deviceRepository.findByRoomId(roomid);
         for (Device d : devices) {
             if (d instanceof Thermostat) {
                 thermostatService.populateMeasuredTemperature((Thermostat) d);
             }
         }
-        List<Device> 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<Device> deviceList = toList(devices);
+        return fetchOwnerOrGuest(deviceList, hostId, principal);
     }
 }