Merge branch 'api-cleanup' of https://github.com/RIT-SWEN-261-02/team-project-2245-swen-261-02-2b into api-cleanup

8 files changed, 80 insertions, 61 deletions

diff --git a/ufund-api/src/main/java/com/ufund/api/ufundapi/controller/AuthController.java b/ufund-api/src/main/java/com/ufund/api/ufundapi/controller/AuthController.java
index b46d4ee..6ba6160 100644
--- a/ufund-api/src/main/java/com/ufund/api/ufundapi/controller/AuthController.java
+++ b/ufund-api/src/main/java/com/ufund/api/ufundapi/controller/AuthController.java
@@ -2,6 +2,8 @@ package com.ufund.api.ufundapi.controller;
 
 import java.io.IOException;
 import java.util.Map;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
@@ -17,6 +19,7 @@ import com.ufund.api.ufundapi.service.AuthService;
 @RestController
 @RequestMapping("auth")
 public class AuthController {
+    private static final Logger LOG = Logger.getLogger(AuthController.class.getName());
     private final AuthService authService;
 
     public AuthController(AuthService authService) {
@@ -32,14 +35,17 @@ public class AuthController {
      */
     @PostMapping("")
     public ResponseEntity<String> login(@RequestBody Map<String, String> params) {
+        LOG.log(Level.INFO, "POST /auth body: {0}", params);
         String username = params.get("username");
         String password = params.get("password");
         try {
             String key = authService.login(username, password);
             return new ResponseEntity<>(key, HttpStatus.OK);
-        } catch (IllegalAccessException e) {
+        } catch (IllegalAccessException ex) {
+            LOG.log(Level.WARNING, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.UNAUTHORIZED);
         } catch (IOException ex) {
+            LOG.log(Level.SEVERE, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
         }
     }
@@ -52,10 +58,12 @@ public class AuthController {
      */
     @DeleteMapping("")
     public ResponseEntity<Object> logout(@RequestHeader("jelly-api-key") String key) {
+        LOG.log(Level.INFO, "DELETE /auth key: {0}", key);
         try {
             authService.logout(key);
             return new ResponseEntity<>(HttpStatus.OK);
-        } catch (IOException e) {
+        } catch (IOException ex) {
+            LOG.log(Level.WARNING, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
         }
     }
diff --git a/ufund-api/src/main/java/com/ufund/api/ufundapi/controller/CupboardController.java b/ufund-api/src/main/java/com/ufund/api/ufundapi/controller/CupboardController.java
index 664b53b..8db8901 100644
--- a/ufund-api/src/main/java/com/ufund/api/ufundapi/controller/CupboardController.java
+++ b/ufund-api/src/main/java/com/ufund/api/ufundapi/controller/CupboardController.java
@@ -23,6 +23,8 @@ import com.ufund.api.ufundapi.model.Need;
 import com.ufund.api.ufundapi.model.Need.GoalType;
 import com.ufund.api.ufundapi.service.CupboardService;
 
+import static java.util.List.of;
+
 @RestController
 @RequestMapping("cupboard")
 public class CupboardController {
@@ -49,7 +51,8 @@ public class CupboardController {
      */
     @PostMapping("")
     public ResponseEntity<Need> createNeed(@RequestBody Map<String, Object> params) {
-        System.out.println(params);
+        LOG.log(Level.INFO, "POST /cupboard body: {0}", params);
+
         String name = (String) params.get("name");
         double maxGoal = (double) params.get("maxGoal");
         Need.GoalType goalType = GoalType.valueOf((String) params.get("type"));
@@ -58,10 +61,13 @@ public class CupboardController {
             Need need = cupboardService.createNeed(name, maxGoal, goalType);
             return new ResponseEntity<>(need, HttpStatus.OK);
         } catch (DuplicateKeyException ex) {
+            LOG.log(Level.WARNING, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.CONFLICT);
         } catch (IllegalArgumentException ex) {
+            LOG.log(Level.WARNING, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
         } catch (IOException ex) {
+            LOG.log(Level.SEVERE, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
         }
     }
@@ -76,7 +82,7 @@ public class CupboardController {
      */
     @GetMapping("")
     public ResponseEntity<Need[]> getNeeds() {
-        LOG.info("GET /needs");
+        LOG.info("GET /cupboard");
 
         try {
             Need[] needs = cupboardService.getNeeds();
@@ -88,19 +94,21 @@ public class CupboardController {
     }
 
      /**
-     * Responds to the GET request for all {@linkplain Need need} whose name contains
-     * the text in name
-     * 
-     * @param name The name parameter which contains the text used to find the {@link Need need}
-     * 
-     * @return ResponseEntity with array of {@link Need need} objects (may be empty) and
-     * HTTP status of OK<br>
-     * ResponseEntity with HTTP status of INTERNAL_SERVER_ERROR otherwise
-     * <p>
-     */
+      * Responds to the GET request for all {@linkplain Need need} whose name contains
+      * the text in name
+      *
+      * @param name The name parameter which contains the text used to find the {@link Need need}
+      *
+      * @deprecated Searching should now be done client side in the future
+      *
+      * @return ResponseEntity with array of {@link Need need} objects (may be empty) and
+      * HTTP status of OK<br>
+      * ResponseEntity with HTTP status of INTERNAL_SERVER_ERROR otherwise
+      * <p>
+      */
     @GetMapping("/")
     public ResponseEntity<Need[]> searchNeeds(@RequestParam String name) {
-        LOG.info("GET /need/?name="+name);
+        LOG.info("GET /cupboard/?name="+name);
 
         try {
             Need[] needs = cupboardService.searchNeeds(name);
@@ -121,7 +129,7 @@ public class CupboardController {
      */
     @GetMapping("/{id}")
     public ResponseEntity<Need> getNeed(@PathVariable int id) {
-        LOG.log(Level.INFO, "GET /need/{0}", id);
+        LOG.log(Level.INFO, "GET /cupboard/{0}", id);
 
         try {
             Need need = cupboardService.getNeed(id);
@@ -145,7 +153,7 @@ public class CupboardController {
      */
     @PutMapping("/{id}")
     public ResponseEntity<Need> updateNeed(@RequestBody Need need, @PathVariable int id) {
-        LOG.log(Level.INFO, "Updating need: " + need);
+        LOG.log(Level.INFO, "PUT /cupboard/{0} body: {1}", of(id, need));
         try {
             Need updatedNeed = cupboardService.updateNeed(need, id);
             if (updatedNeed != null) {
@@ -154,10 +162,10 @@ public class CupboardController {
                 return new ResponseEntity<>(HttpStatus.NOT_FOUND);
             }
         } catch (IllegalArgumentException ex) {
-            ex.printStackTrace();
+            LOG.log(Level.WARNING, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
         } catch (IOException ex) {
-            ex.printStackTrace();
+            LOG.log(Level.SEVERE, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
         }
     }
@@ -168,24 +176,23 @@ public class CupboardController {
      * @param data JSON object with paramters needID and amount
      * @param key  Key used to authenticate user
      * @return OK if successful, other statuses if failure
-     * @throws IllegalAccessException
      */
     @PutMapping("/checkout")
-    public ResponseEntity<Object> checkoutNeeds(@RequestBody Map<String, Integer> data, @RequestHeader("jelly-api-key") String key) throws IllegalAccessException {
+    public ResponseEntity<Object> checkoutNeeds(@RequestBody Map<String, Integer> data, @RequestHeader("jelly-api-key") String key) {
         int needID = data.get("needID");
         int checkoutAmount = data.get("amount");
-        LOG.log(Level.INFO, "Checking out need with ID: " + needID + " by " + checkoutAmount);
+        LOG.log(Level.INFO, "PUT /need/checkout body: {0}", data);
         try {
             cupboardService.checkoutNeed(needID, checkoutAmount, key);
             return new ResponseEntity<>(HttpStatus.OK);
         } catch (IllegalArgumentException ex) {
-            ex.printStackTrace();
+            LOG.log(Level.WARNING, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
         } catch (IllegalAccessException ex) {
-            ex.printStackTrace();
+            LOG.log(Level.WARNING, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.UNAUTHORIZED);
         } catch (IOException ex) {
-            ex.printStackTrace();
+            LOG.log(Level.SEVERE, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
         }
     }
@@ -198,6 +205,7 @@ public class CupboardController {
     */
     @DeleteMapping("/{id}")
     public ResponseEntity<Need> deleteNeed(@PathVariable int id) {
+        LOG.log(Level.INFO, "DELETE /cupboard/{0}", id);
         try {
             Need need = cupboardService.getNeed(id);
             if (cupboardService.deleteNeed(id)) {
@@ -205,7 +213,8 @@ public class CupboardController {
             } else {
                 return new ResponseEntity<>(HttpStatus.NOT_FOUND);
             } 
-        } catch (IOException e) {
+        } catch (IOException ex) {
+            LOG.log(Level.SEVERE, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
         }
     }
diff --git a/ufund-api/src/main/java/com/ufund/api/ufundapi/controller/UserController.java b/ufund-api/src/main/java/com/ufund/api/ufundapi/controller/UserController.java
index b0dbd1d..cd340ef 100644
--- a/ufund-api/src/main/java/com/ufund/api/ufundapi/controller/UserController.java
+++ b/ufund-api/src/main/java/com/ufund/api/ufundapi/controller/UserController.java
@@ -23,6 +23,8 @@ import com.ufund.api.ufundapi.model.User;
 import com.ufund.api.ufundapi.service.AuthService;
 import com.ufund.api.ufundapi.service.UserService;
 
+import static java.util.List.of;
+
 @RestController
 @RequestMapping("users")
 public class UserController {
@@ -43,6 +45,7 @@ public class UserController {
      */
     @PostMapping("")
     public ResponseEntity<User> createUser(@RequestBody Map<String, String> params) {
+        LOG.log(Level.INFO, "POST /users body: {0}", params);
         String username = params.get("username");
         String password = params.get("password");
 
@@ -54,8 +57,10 @@ public class UserController {
                 return new ResponseEntity<>(HttpStatus.CONFLICT);
             }
         } catch (DuplicateKeyException ex) {
+            LOG.log(Level.WARNING, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.CONFLICT);
         } catch (IOException ex) {
+            LOG.log(Level.SEVERE, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
         }
     }
@@ -72,7 +77,7 @@ public class UserController {
      */
     @GetMapping("/{username}")
     public ResponseEntity<User> getUser(@PathVariable String username, @RequestHeader("jelly-api-key") String key) {
-        LOG.log(Level.INFO, "GET /user/{0}", username);
+        LOG.log(Level.INFO, "GET /user/{0} key: {1}", of(username, key));
 
         try {
             authService.authenticate(username, key);
@@ -83,9 +88,10 @@ public class UserController {
                 return new ResponseEntity<>(HttpStatus.NOT_FOUND);
             }
         } catch (IllegalAccessException ex) {
+            LOG.log(Level.WARNING, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.UNAUTHORIZED);
-        } catch (IOException e) {
-            LOG.log(Level.SEVERE, e.getLocalizedMessage());
+        } catch (IOException ex) {
+            LOG.log(Level.SEVERE, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
         }
 
@@ -99,11 +105,10 @@ public class UserController {
      * @param key      The authentication key of the user
      * @return OK response and the user if it was successful, or
      *         INTERNAL_SERVER_ERROR if there was an issue
-     * @throws IllegalAccessException 
      */
     @PutMapping("/{username}")
-    public ResponseEntity<User> updateUser(@RequestHeader("jelly-api-key") String key, @RequestBody User user, @PathVariable String username) throws IllegalAccessException {
-        LOG.log(Level.INFO,"PUT: " + user + " " + username + " " + key);
+    public ResponseEntity<User> updateUser(@RequestBody User user, @PathVariable String username, @RequestHeader("jelly-api-key") String key) {
+        LOG.log(Level.INFO,"PUT /users/{0} body: {1} key: {2}", of(user, username, key));
         try {
             authService.authenticate(username, key);
             user = userService.updateUser(user, username);
@@ -113,13 +118,15 @@ public class UserController {
                 return new ResponseEntity<>(HttpStatus.NOT_FOUND);
             }
         } catch (InvalidParameterException ex) {
+            LOG.log(Level.WARNING, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
-        } catch (IOException e) {
+        } catch (IllegalAccessException ex) {
+            LOG.log(Level.WARNING, ex.getLocalizedMessage());
+            return new ResponseEntity<>(HttpStatus.UNAUTHORIZED);
+        } catch (IOException ex) {
+            LOG.log(Level.SEVERE, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
-        } 
-        // catch (IllegalAccessException e) {
-        //     return new ResponseEntity<>(HttpStatus.UNAUTHORIZED);
-        // }
+        }
     }
 
     /**
@@ -132,6 +139,7 @@ public class UserController {
      */
     @DeleteMapping("/{username}")
     public ResponseEntity<Boolean> deleteUser(@PathVariable String username, @RequestHeader("jelly-api-key") String key) {
+        LOG.log(Level.INFO, "DELETE /users/{0} id: {1}", of(username, key));
 
         try {
             authService.authenticate(username, key);
@@ -140,10 +148,12 @@ public class UserController {
             } else {
                 return new ResponseEntity<>(HttpStatus.NOT_FOUND);
             }
-        } catch (IOException e) {
-            return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
-        } catch (IllegalAccessException e) {
+        } catch (IllegalAccessException ex) {
+            LOG.log(Level.WARNING, ex.getLocalizedMessage());
             return new ResponseEntity<>(HttpStatus.UNAUTHORIZED);
+        } catch (IOException ex) {
+            LOG.log(Level.SEVERE, ex.getLocalizedMessage());
+            return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
         }
     }
 
diff --git a/ufund-api/src/main/java/com/ufund/api/ufundapi/model/Need.java b/ufund-api/src/main/java/com/ufund/api/ufundapi/model/Need.java
index 22e86e3..786b104 100644
--- a/ufund-api/src/main/java/com/ufund/api/ufundapi/model/Need.java
+++ b/ufund-api/src/main/java/com/ufund/api/ufundapi/model/Need.java
@@ -38,7 +38,7 @@ public class Need {
      * @param maxGoal The maximum goal for this need
      * @param type    The type of need (monetary, physical)
      */
-    public Need(String name, GoalType type, double maxGoal) {
+    public Need(String name, GoalType type, double maxGoal) { // TODO why is this needed
         this.name = name;
         this.type = type;
         this.maxGoal = maxGoal;
diff --git a/ufund-api/src/main/java/com/ufund/api/ufundapi/persistence/CupboardFileDAO.java b/ufund-api/src/main/java/com/ufund/api/ufundapi/persistence/CupboardFileDAO.java
index 4d11554..3115204 100644
--- a/ufund-api/src/main/java/com/ufund/api/ufundapi/persistence/CupboardFileDAO.java
+++ b/ufund-api/src/main/java/com/ufund/api/ufundapi/persistence/CupboardFileDAO.java
@@ -53,22 +53,13 @@ public class CupboardFileDAO implements CupboardDAO {
     }
 
     /**
-     * Return an array of the needs
-     *
-     * @return An array of all the needs
-     */
-    private Need[] getNeedsArray() {
-        return needs.values().toArray(Need[]::new);
-    }
-
-    /**
      * Saves the needs to json
      *
      * @return True if the save was successful, false otherwise
      * @throws IOException If there was an IO issue saving the file
      */
     private boolean save() throws IOException {
-        Need[] needArray = getNeedsArray();
+        Need[] needArray = needs.values().toArray(Need[]::new);
 
         objectMapper.writeValue(new File(filename), needArray);
         return true;
@@ -77,7 +68,7 @@ public class CupboardFileDAO implements CupboardDAO {
     @Override
     public Need[] getNeeds() {
         synchronized (needs) {
-            return getNeedsArray();
+            return needs.values().toArray(Need[]::new);
         }
     }
 
diff --git a/ufund-api/src/main/java/com/ufund/api/ufundapi/persistence/UserFileDAO.java b/ufund-api/src/main/java/com/ufund/api/ufundapi/persistence/UserFileDAO.java
index 16560e7..63d864a 100644
--- a/ufund-api/src/main/java/com/ufund/api/ufundapi/persistence/UserFileDAO.java
+++ b/ufund-api/src/main/java/com/ufund/api/ufundapi/persistence/UserFileDAO.java
@@ -5,6 +5,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
@@ -86,12 +87,12 @@ public class UserFileDAO implements UserDAO {
                 // user.copyPassword(old);
                 if (user.getBasket() == null || user.getType() == null) {
                     User oldData = users.get(user.getUsername());
-                    User crutch = new User(oldData.getUsername(), 0, new ArrayList<Integer>(), oldData.getType());
+                    User crutch = new User(oldData.getUsername(), 0, new ArrayList<>(), oldData.getType());
                     crutch.copyPassword(oldData);
                     users.put(user.getUsername(), crutch);
                 } else {
                     var old = users.put(user.getUsername(), user);
-                    user.copyPassword(old);
+                    user.copyPassword(Objects.requireNonNull(old));
                 }
                 save();
                 return user;
diff --git a/ufund-api/src/main/java/com/ufund/api/ufundapi/service/AuthService.java b/ufund-api/src/main/java/com/ufund/api/ufundapi/service/AuthService.java
index 71b8f41..4e5ebce 100644
--- a/ufund-api/src/main/java/com/ufund/api/ufundapi/service/AuthService.java
+++ b/ufund-api/src/main/java/com/ufund/api/ufundapi/service/AuthService.java
@@ -29,20 +29,20 @@ public class AuthService {
     public void authenticate(String targetUsername, String key) throws IllegalAccessException, IOException {
         var userAuth = userAuthDAO.getUserAuth(key);
         if (userAuth == null) {
-            throw new IllegalAccessException("Unauthenticated");
+            throw new IllegalAccessException("Invalid authentication key");
         }
 
        var username = userAuth.getUsername();
        var userType = userService.getUser(username).getType();
        if (!username.equals(targetUsername) && userType != User.UserType.MANAGER) {
-           throw new IllegalAccessException("Unauthorized");
+           throw new IllegalAccessException("Provided key does not grant access to perform the requested operation");
        }
     }
 
     public void authenticate(String key) throws IOException, IllegalAccessException {
         var userAuth = userAuthDAO.getUserAuth(key);
         if (userAuth == null) {
-            throw new IllegalAccessException("Unauthenticated");
+            throw new IllegalAccessException("Invalid authentication key");
         }
     }
 
@@ -58,7 +58,7 @@ public class AuthService {
     public String login(String username, String password) throws IllegalAccessException, IOException {
         var usr = userService.getUser(username);
         if (usr == null || !usr.verifyPassword(password)) {
-            throw new IllegalAccessException("Unauthorized");
+            throw new IllegalAccessException("Incorrect username or password");
         }
         var userAuth = UserAuth.generate(username);
         userAuthDAO.addUserAuth(userAuth);
diff --git a/ufund-api/src/main/java/com/ufund/api/ufundapi/service/CupboardService.java b/ufund-api/src/main/java/com/ufund/api/ufundapi/service/CupboardService.java
index 8713882..91e3ba5 100644
--- a/ufund-api/src/main/java/com/ufund/api/ufundapi/service/CupboardService.java
+++ b/ufund-api/src/main/java/com/ufund/api/ufundapi/service/CupboardService.java
@@ -104,12 +104,12 @@ public class CupboardService {
      * 
      * @param id The ID of the need to update
      * @param checkoutAmount The amount to update the need by
-     * @throws IOException 
-     * @throws IllegalAccessException 
+     * @throws IOException If there is an error reading the file
+     * @throws IllegalAccessException If the user has insufficient permission
     */
     public void checkoutNeed(int id, double checkoutAmount, String key) throws IOException, IllegalAccessException {
         if (checkoutAmount <= 0) {
-            throw new IllegalArgumentException("Amount must be greather than 0");
+            throw new IllegalArgumentException("Amount must be greater than 0");
         }
         authService.authenticate(key);
         Need need = cupboardDAO.getNeed(id);