katharsis-project / katharsis-framework

Katharsis adds powerful layer for RESTful endpoints providing implementenation of JSON:API standard
http://katharsis.io
Apache License 2.0
135 stars 65 forks source link

katharsis-client: it should be possible to ignore relations #268

Open Ramblurr opened 7 years ago

Ramblurr commented 7 years ago

A large api can often have dozens of Resources. These resources are often very intertwined due to the @JsonApiToMany and @JsonApiToOne relationships among them.

Often it is sufficient for a client to only access a small subset of these resources. In this case only the "interesting" resources need to be modeled as POJOs. Those relationships that are not needed can be ignored.

However katharsis client expects that the relationships exist on the resource pojos when deserializing. These essentially requires us to fully model all the resources despite the fact we need only a small subset.

Example

For this project I only care about the User resource. The full domain model is rich with many other resources, but I only care about the User. The User has one relationship groupMemberships, and I do not want to deserialize it or touch it.

In katharsis 2.6 using Object as the relationship type worked fine. However now in katharsis 2.8 ModuleRegistry#build() throws an UnsupportedOperationException.

@JsonApiResource(type = "users")
public class User
{

    @JsonApiId
    String id;
    String username;

    @JsonApiToMany
    Object groupMemberships;
}

If you omit the groupMemberships field on the pojo entirely, katharsis client fails during deserialization with

io.katharsis.resource.exception.ResourceException: Invalid relationship name: groupMemberships for users
    at io.katharsis.dispatcher.controller.resource.ResourceUpsert.setRelations(ResourceUpsert.java:203)
    at io.katharsis.client.internal.BaseResponseDeserializer$ClientResourceUpsert.setRelations(BaseResponseDeserializer.java:162)
    at io.katharsis.client.internal.BaseResponseDeserializer.deserialize(BaseResponseDeserializer.java:96)
    at io.katharsis.client.internal.BaseResponseDeserializer.deserialize(BaseResponseDeserializer.java:48)

Ideally it should be possible to omit relations and have that payload ignored, however I could also accept the Object placeholder.

@remmeier any thoughts on this?

Ramblurr commented 7 years ago

Here is a potential patch.

These are the assumptions:

  1. Client-side, if a developer wants to ignore a relationship on a resource -> they do not include the field in the resource POJO
  2. When the response is fetched from the server, ResourceUpsert ignores the non-existing relationship field instead of throwing an exception.

This makes this test fail, obviously. Should it be deleted? Is there a way to have different behavior depending on whether it is a client or server?

io.katharsis.dispatcher.controller.resource.ResourcePostTest#onResourceWithInvalidRelationshipNameShouldThrowException

How does this affect the server implementation? I'm not sure yet, but it solves my client side issue.

diff --git a/katharsis-core/src/main/java/io/katharsis/dispatcher/controller/resource/ResourceUpsert.java b/katharsis-core/src/main/java/io/katharsis/dispatcher/controller/resource/ResourceUpsert.java
index 2639092d..2b3585b6 100644
--- a/katharsis-core/src/main/java/io/katharsis/dispatcher/controller/resource/ResourceUpsert.java
+++ b/katharsis-core/src/main/java/io/katharsis/dispatcher/controller/resource/ResourceUpsert.java
@@ -199,19 +199,18 @@ public abstract class ResourceUpsert extends BaseController {

                ResourceInformation resourceInformation = registryEntry.getResourceInformation();
                                ResourceField field = resourceInformation.findRelationshipFieldByName(propertyName);
-                               if(field == null){
-                                        throw new ResourceException(String.format("Invalid relationship name: %s for %s", property.getKey(), resourceInformation.getResourceType()));
-                               }
-               if (Iterable.class.isAssignableFrom(field.getType())){
-                    //noinspection unchecked
-                    setRelationsField(newResource,
-                            registryEntry,
-                            (Map.Entry) property,
-                            queryAdapter,
-                            parameterProvider, links);
-                } else {
-                    //noinspection unchecked
-                    setRelationField(newResource, registryEntry, (Map.Entry) property, queryAdapter, parameterProvider);
+                if(field != null){
+                    if (Iterable.class.isAssignableFrom(field.getType())){
+                        //noinspection unchecked
+                        setRelationsField(newResource,
+                                registryEntry,
+                                (Map.Entry) property,
+                                queryAdapter,
+                                parameterProvider, links);
+                    } else {
+                        //noinspection unchecked
+                        setRelationField(newResource, registryEntry, (Map.Entry) property, queryAdapter, parameterProvider);
+                    }
                 }

             }
remmeier commented 7 years ago

I have a look, since I'm already working on the serializers anyway.

Ramblurr commented 7 years ago

@remmeier any progress on this? I'm using katharsis client 3.0, with queryparams repositories (because my API uses page[number])

and if my client's resource POJO does not define a relationship, then I get

io.katharsis.errorhandling.exception.ResourceException: Invalid relationship name: groupMemberships for users
    at io.katharsis.core.internal.dispatcher.controller.ResourceUpsert.setRelations(ResourceUpsert.java:216)

I thought ignoring it like the following would work, but it doesn't as the code that handles the de-serialization of the response requires the relation to exist.

 @JsonIgnore
 Object unusedRelationship;