Merge branch 'master' into feature/datastore-jpa

yahoo · Apr 5, 2019 · 1975555 · 1975555
2 parents 485a9a8 + 982e99b
commit 1975555
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 2 deletions.
diff --git a/changelog.md b/changelog.md
@@ -1,5 +1,8 @@
 # Change Log
 ## 4.3.4
+**Fixes**
+ * Throw proper exception on invalid PersistentResource where id=null
+
 **Features**
  * Added [JPA Data Store](https://github.com/yahoo/elide/pull/747)
 

diff --git a/elide-core/src/main/java/com/yahoo/elide/jsonapi/models/Resource.java b/elide-core/src/main/java/com/yahoo/elide/jsonapi/models/Resource.java
@@ -42,6 +42,9 @@ public class Resource {
     public Resource(String type, String id) {
         this.type = type;
         this.id = id;
+        if (id == null) {
+            throw new InvalidObjectIdentifierException(id, type);
+        }
     }
 
     public Resource(@JsonProperty("type") String type,
@@ -59,7 +62,7 @@ public Resource(@JsonProperty("type") String type,
     }
 
     public String getId() {
-        return (id == null) ? null : id;
+        return id;
     }
 
     public void setRelationships(Map<String, Relationship> relationships) {
@@ -141,12 +144,15 @@ public boolean equals(Object obj) {
         return false;
     }
 
-    public PersistentResource toPersistentResource(RequestScope requestScope)
+    public PersistentResource<?> toPersistentResource(RequestScope requestScope)
         throws ForbiddenAccessException, InvalidObjectIdentifierException {
         Class<?> cls = requestScope.getDictionary().getEntityClass(type);
         if (cls == null) {
             throw new UnknownEntityException(type);
         }
+        if (id == null) {
+            throw new InvalidObjectIdentifierException(id, type);
+        }
         return PersistentResource.loadRecord(cls, id, requestScope);
     }
 }
diff --git a/elide-core/src/test/java/com/yahoo/elide/core/PersistentResourceTest.java b/elide-core/src/test/java/com/yahoo/elide/core/PersistentResourceTest.java
@@ -603,10 +603,44 @@ public void testSuccessfulOneToOneRelationshipAdd() throws Exception {
         goodScope.saveOrCreateObjects();
         verify(tx, times(1)).save(left, goodScope);
         verify(tx, times(1)).save(right, goodScope);
+        verify(tx, times(1)).getRelation(tx, left, "one2one", Optional.empty(), Optional.empty(), Optional.empty(),
+                goodScope);
         Assert.assertEquals(updated, true, "The one-2-one relationship should be added.");
         Assert.assertEquals(left.getOne2one().getId(), 3, "The correct object was set in the one-2-one relationship");
     }
 
+    /**
+     * Avoid NPE when PATCH or POST defines relationship with null id
+     * <pre>
+     * <code>
+     * "relationships": {
+     *   "left": {
+     *     "data": {
+     *       "type": "right",
+     *       "id": null
+     *     }
+     *   }
+     * }
+     * </code>
+     * </pre>
+     */
+    @Test(expectedExceptions = InvalidObjectIdentifierException.class,
+            expectedExceptionsMessageRegExp = "Unknown identifier 'null' for right")
+    public void testSuccessfulOneToOneRelationshipAddNull() throws Exception {
+        User goodUser = new User(1);
+        DataStoreTransaction tx = mock(DataStoreTransaction.class, Answers.CALLS_REAL_METHODS);
+        Left left = new Left();
+        left.setId(2);
+
+        RequestScope goodScope = new RequestScope(null, null, tx, goodUser, null, elideSettings, false);
+
+        PersistentResource<Left> leftResource = new PersistentResource<>(left, null, "2", goodScope);
+
+        Relationship ids = new Relationship(null, new Data<>(new Resource("right", null, null, null, null, null)));
+
+        leftResource.updateRelation("one2one", ids.toPersistentResources(goodScope));
+    }
+
     @Test
     /*
      * The following are ids for a hypothetical relationship.