Merge pull request #59 from folded-ear/graphql-delete

new Deletion type for graphql deletions, so apollo can get the ID

src/main/java/com/brennaswitzer/cookbook/domain/Acl.java

@@ -6,6 +6,7 @@
 import jakarta.persistence.ManyToOne;
 import jakarta.persistence.MapKeyJoinColumn;
 import jakarta.validation.constraints.NotNull;
+import lombok.Getter;
 
 import java.util.Collections;
 import java.util.HashMap;
@@ -17,6 +18,7 @@
 @Embeddable
 public class Acl {
 
+    @Getter
     @NotNull
     @ManyToOne
     private User owner;
@@ -26,10 +28,6 @@ public class Acl {
     @Column(name = "level_id")
     private Map<User, AccessLevel> grants;
 
-    public User getOwner() {
-        return owner;
-    }
-
     public void setOwner(User owner) {
         this.owner = owner;
         // clear any explicit grant the new owner previously had
@@ -63,7 +61,7 @@ public AccessLevel setGrant(User user, AccessLevel level) {
         return grants.put(user, level);
     }
 
-    public AccessLevel deleteGrant(User user) {
+    public AccessLevel revokeGrant(User user) {
         if (user == null) throw new IllegalArgumentException("You can't revoke access from the null user.");
         if (user.equals(owner)) throw new UnsupportedOperationException();
         if (grants == null) return null;

src/main/java/com/brennaswitzer/cookbook/domain/PlanBucket.java

@@ -19,7 +19,7 @@
 @Table(name = "plan_bucket", uniqueConstraints = {
         @UniqueConstraint(columnNames = { "plan_id", "name" })
 })
-public class PlanBucket extends BaseEntity {
+public class PlanBucket extends BaseEntity implements Named {
 
     @NotNull
     @Getter

src/main/java/com/brennaswitzer/cookbook/graphql/LibraryMutation.java

@@ -2,6 +2,7 @@
 
 import com.brennaswitzer.cookbook.domain.Recipe;
 import com.brennaswitzer.cookbook.domain.Upload;
+import com.brennaswitzer.cookbook.graphql.model.Deletion;
 import com.brennaswitzer.cookbook.payload.IngredientInfo;
 import com.brennaswitzer.cookbook.services.ItemService;
 import com.brennaswitzer.cookbook.services.LabelService;
@@ -45,9 +46,8 @@ public Recipe setRecipePhoto(Long id, Part photo) {
         return recipeService.setRecipePhoto(id, Upload.of(photo));
     }
 
-    public boolean deleteRecipe(Long id) {
-        recipeService.deleteRecipeById(id);
-        return true;
+    public Deletion deleteRecipe(Long id) {
+        return Deletion.of(recipeService.deleteRecipeById(id));
     }
 
 }

src/main/java/com/brennaswitzer/cookbook/graphql/PantryMutation.java

@@ -1,6 +1,7 @@
 package com.brennaswitzer.cookbook.graphql;
 
 import com.brennaswitzer.cookbook.domain.PantryItem;
+import com.brennaswitzer.cookbook.graphql.model.Deletion;
 import com.brennaswitzer.cookbook.services.PantryItemService;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
@@ -54,8 +55,8 @@ public PantryItem combineItems(List<Long> ids) {
         return pantryItemService.combineItems(ids);
     }
 
-    public boolean deleteItem(Long id) {
-        return pantryItemService.deleteItem(id);
+    public Deletion deleteItem(Long id) {
+        return Deletion.of(pantryItemService.deleteItem(id));
     }
 
 }

src/main/java/com/brennaswitzer/cookbook/graphql/PlannerMutation.java

@@ -5,6 +5,7 @@
 import com.brennaswitzer.cookbook.domain.PlanBucket;
 import com.brennaswitzer.cookbook.domain.PlanItem;
 import com.brennaswitzer.cookbook.domain.PlanItemStatus;
+import com.brennaswitzer.cookbook.graphql.model.Deletion;
 import com.brennaswitzer.cookbook.services.PlanService;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
@@ -34,17 +35,16 @@ public Plan createPlan(String name) {
         return planService.createPlan(name);
     }
 
-    public Plan deleteBucket(Long planId, Long bucketId) {
-        return planService.deleteBucket(planId, bucketId).getPlan();
+    public Deletion deleteBucket(Long planId, Long bucketId) {
+        return Deletion.of(planService.deleteBucket(planId, bucketId));
     }
 
-    public PlanItem deleteItem(Long id) {
-        return planService.deleteItemForParent(id);
+    public Deletion deleteItem(Long id) {
+        return Deletion.of(planService.deleteItem(id));
     }
 
-    public boolean deletePlan(Long id) {
-        planService.deletePlan(id);
-        return true;
+    public Deletion deletePlan(Long id) {
+        return Deletion.of(planService.deletePlan(id));
     }
 
     public Plan duplicatePlan(String name, Long sourcePlanId) {
@@ -75,8 +75,8 @@ public PlanItem setStatus(Long id, PlanItemStatus status) {
         return planService.setItemStatus(id, status);
     }
 
-    public Plan deleteGrant(Long planId, Long userId) {
-        return planService.deleteGrantFromPlan(planId, userId);
+    public Plan revokeGrant(Long planId, Long userId) {
+        return planService.revokeGrantFromPlan(planId, userId);
     }
 
 }

src/main/java/com/brennaswitzer/cookbook/graphql/TimerMutation.java

@@ -1,6 +1,7 @@
 package com.brennaswitzer.cookbook.graphql;
 
 import com.brennaswitzer.cookbook.domain.Timer;
+import com.brennaswitzer.cookbook.graphql.model.Deletion;
 import com.brennaswitzer.cookbook.services.timers.UpdateTimers;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
@@ -28,8 +29,9 @@ public Timer addTime(Long id, int duration) {
         return update.addTime(id, duration);
     }
 
-    public boolean delete(Long id) {
-        return update.deleteTimer(id);
+    public Deletion delete(Long id) {
+        return new Deletion(update.deleteTimer(id).getId(),
+                            "Unnamed Timer");
     }
 
 }

src/main/java/com/brennaswitzer/cookbook/graphql/model/Deletion.java

@@ -0,0 +1,17 @@
+package com.brennaswitzer.cookbook.graphql.model;
+
+import com.brennaswitzer.cookbook.domain.Identified;
+import com.brennaswitzer.cookbook.domain.Named;
+import lombok.Value;
+
+@Value
+public class Deletion {
+
+    Long id;
+    String name;
+
+    public static <T extends Identified & Named> Deletion of(T it) {
+        return new Deletion(it.getId(), it.getName());
+    }
+
+}

src/main/java/com/brennaswitzer/cookbook/repositories/PantryItemRepository.java

@@ -2,12 +2,11 @@
 
 import com.brennaswitzer.cookbook.domain.PantryItem;
 import org.springframework.data.jpa.repository.Query;
-import org.springframework.data.repository.CrudRepository;
 
 import java.time.Instant;
 import java.util.List;
 
-public interface PantryItemRepository extends CrudRepository<PantryItem, Long>, PantryItemSearchRepository {
+public interface PantryItemRepository extends BaseEntityRepository<PantryItem>, PantryItemSearchRepository {
 
     @Query("""
            select item

src/main/java/com/brennaswitzer/cookbook/services/PantryItemService.java

@@ -158,9 +158,10 @@ public PantryItem combineItems(List<Long> ids) {
     }
 
     @PreAuthorize("hasRole('DEVELOPER')")
-    public boolean deleteItem(Long id) {
+    public PantryItem deleteItem(Long id) {
+        var it = pantryItemRepository.getReferenceById(id);
         pantryItemRepository.deleteById(id);
-        return true;
+        return it;
     }
 
     private void needsDupesFound(PantryItem item) {

src/main/java/com/brennaswitzer/cookbook/services/PlanService.java

@@ -426,20 +426,14 @@ public PlanMessage setItemStatusForMessage(Long id, PlanItemStatus status) {
         return buildUpdateMessage(item);
     }
 
-    public PlanItem deleteItemForParent(Long id) {
-        val item = setItemStatus(id, PlanItemStatus.DELETED);
-        if (item.hasParent()) {
-            return item.getParent();
-        } else {
-            throw new IllegalArgumentException(String.format(
-                    "ID '%s' is a plan",
-                    id));
-        }
+    public PlanItem deleteItem(Long id) {
+        return setItemStatus(id, PlanItemStatus.DELETED);
     }
 
-    public void deletePlan(Long id) {
+    public Plan deletePlan(Long id) {
         val plan = getPlanById(id, AccessLevel.ADMINISTER);
         planRepo.delete(plan);
+        return plan;
     }
 
     public void severLibraryLinks(Recipe r) {
@@ -455,10 +449,9 @@ public Plan setGrantOnPlan(Long planId, Long userId, AccessLevel level) {
         return plan;
     }
 
-    @SuppressWarnings("UnusedReturnValue")
-    public Plan deleteGrantFromPlan(Long planId, Long userId) {
+    public Plan revokeGrantFromPlan(Long planId, Long userId) {
         Plan plan = getPlanById(planId, AccessLevel.ADMINISTER);
-        plan.getAcl().deleteGrant(userRepo.getById(userId));
+        plan.getAcl().revokeGrant(userRepo.getById(userId));
         return plan;
     }
 

src/main/java/com/brennaswitzer/cookbook/services/RecipeService.java

@@ -76,11 +76,12 @@ public Optional<Recipe> findRecipeById(Long id) {
         return recipeRepository.findById(id);
     }
 
-    public void deleteRecipeById(Long id) {
+    public Recipe deleteRecipeById(Long id) {
         Recipe recipe = getMyRecipe(id);
         removePhotoInternal(recipe);
         planService.severLibraryLinks(recipe);
         recipeRepository.delete(recipe);
+        return recipe;
     }
 
     private void setPhotoInternal(Recipe recipe, Upload photo) {

src/main/java/com/brennaswitzer/cookbook/services/timers/UpdateTimers.java

@@ -47,15 +47,11 @@ public Timer addTime(Long id, int duration) {
         return t;
     }
 
-    public boolean deleteTimer(Long id) {
-        val optionalTimer = repo.findById(id);
-        if (optionalTimer.isEmpty()) {
-            return false;
-        }
-        val t = optionalTimer.get();
+    public Timer deleteTimer(Long id) {
+        val t = repo.getReferenceById(id);
         t.ensurePermitted(principalAccess.getUser(), AccessLevel.CHANGE);
         repo.delete(t);
-        return true;
+        return t;
     }
 
 }

src/main/java/com/brennaswitzer/cookbook/web/PlanController.java

@@ -205,7 +205,7 @@ public void deleteItem(
         if (!planId.equals(item.getPlan().getId())) {
             throw new IllegalArgumentException("Item belongs to a different plan");
         }
-        planService.deleteItemForParent(id);
+        planService.deleteItem(id);
     }
 
     @PostMapping("/{id}/buckets")
@@ -249,11 +249,11 @@ public GrantInfo addGrant(
 
     @DeleteMapping("/{id}/acl/grants/{userId}")
     @ResponseStatus(HttpStatus.NO_CONTENT)
-    public void deleteGrant(
+    public void revokeGrant(
             @PathVariable("id") Long id,
             @PathVariable("userId") Long userId
     ) {
-        planService.deleteGrantFromPlan(id, userId);
+        planService.revokeGrantFromPlan(id, userId);
     }
 
 }

src/main/resources/graphqls/__master.graphqls

@@ -47,3 +47,8 @@ type ShareInfo {
     slug: String!
     secret: String!
 }
+
+type Deletion {
+    id: ID!
+    name: String
+}

src/main/resources/graphqls/library.graphqls

@@ -213,7 +213,7 @@ type LibraryMutation {
     createRecipe(info: IngredientInfo!, photo: Upload, cookThis: Boolean): Recipe!
     updateRecipe(id: ID!, info: IngredientInfo!, photo: Upload): Recipe!
     setRecipePhoto(id: ID!, photo: Upload!): Recipe!
-    deleteRecipe(id: ID!): Boolean!
+    deleteRecipe(id: ID!): Deletion!
 }
 
 input IngredientInfo {

src/main/resources/graphqls/pantry.graphqls

@@ -92,5 +92,5 @@ type PantryMutation {
     combineItems(ids: [ID!]!): PantryItem
     """Delete a pantry item, which MUST be unreferenced.
     """
-    deleteItem(id: ID!): Boolean!
+    deleteItem(id: ID!): Deletion!
 }

src/main/resources/graphqls/planner.graphqls

@@ -89,14 +89,13 @@ type PlannerMutation {
     """Create a new plan by duplicating the specified source plan."""
     duplicatePlan(name: String!, sourcePlanId: ID!): Plan!
     """Delete a bucket from a plan."""
-    deleteBucket(planId: ID!, bucketId: ID!): Plan!
-    """Deletes the grant for a user w/in a plan, if one exists."""
-    deleteGrant(planId: ID!, userId: ID!): Plan!
-    """Deletes an item from a plan, and return its former parent (plan or item).
-    This operation cascades."""
-    deleteItem(id: ID!): PlanItem
+    deleteBucket(planId: ID!, bucketId: ID!): Deletion!
+    """Revokes the grant for a user w/in a plan, if one exists."""
+    revokeGrant(planId: ID!, userId: ID!): Plan!
+    """Deletes an item from a plan. This operation cascades."""
+    deleteItem(id: ID!): Deletion!
     """Deletes the given plan, and all its related data."""
-    deletePlan(id: ID!): Boolean!
+    deletePlan(id: ID!): Deletion!
     """Move the given items under the given parent, in order, optionally after a
     specific item already under that parent. The parent's info is returned."""
     mutateTree(itemIds: [ID!]!, parentId: ID!, afterId: ID): PlanItem!

src/main/resources/graphqls/timers.graphqls

@@ -28,7 +28,7 @@ type TimerMutation {
     """Ensure the specified timer has been deleted, regardless of its status or
     existence, returning whether any action was taken.
     """
-    delete(id: ID!): Boolean!
+    delete(id: ID!): Deletion!
 }
 
 """Represents a pause-able timer of user-specified length.

src/test/java/com/brennaswitzer/cookbook/domain/AclTest.java

@@ -2,7 +2,10 @@
 
 import org.junit.jupiter.api.Test;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class AclTest {
 
@@ -28,7 +31,7 @@ public void grants() {
         assertEquals(AccessLevel.VIEW, acl.getGrant(bob));
         assertNull(acl.getGrant(eve));
 
-        acl.deleteGrant(bob);
+        assertEquals(AccessLevel.VIEW, acl.revokeGrant(bob));
         assertNull(acl.getGrant(bob));
     }
 
@@ -65,7 +68,7 @@ public void cantRemoveOwnerGrants() {
         acl.setOwner(alice);
 
         assertThrows(UnsupportedOperationException.class, () ->
-                acl.deleteGrant(alice));
+                acl.revokeGrant(alice));
     }
 
     @Test