Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HAL links are duplicated when defining content in an Entity #148

Closed
thijslemmens opened this issue Mar 2, 2020 · 8 comments
Closed

HAL links are duplicated when defining content in an Entity #148

thijslemmens opened this issue Mar 2, 2020 · 8 comments

Comments

@thijslemmens
Copy link

thijslemmens commented Mar 2, 2020

When modeling an Entity:

@NoArgsConstructor
@Getter
@Setter
@Entity
public class MyEntity {
    @Id
    @GeneratedValue(strategy = GenerationType.AUTO)
    private Long id;

     // Next 3 properties are for content.
    @ContentId
    private String contentId;
    @ContentLength
    private long contentLength;
    @MimeType
    private String mimeType;
}

With default repository and store:

@RepositoryRestResource
public interface MyEntityRepository extends JpaRepository<MyEntity, Long> {}

@StoreRestResource
public interface MyEntityContentStore extends ContentStore<MyEntity, UUID> {}

calling the a url:
http://localhost:8080/myEntities

results in

{
  "_embedded" : {
    "myEntities" : [ {
      "contentId" : "2ac2eb9c-c01d-4c81-bc5d-24995dc55ca2",
      "contentLength" : 395510,
      "mimeType" : "application/zip",
      "_links" : {
        "self" : {
          "href" : "http://localhost:8080/myEntities/5"
        },
        "myEntity" : [ {
          "href" : "http://localhost:8080/myEntities/5"
        }, {
          "href" : "http://localhost:8080/myEntities/5"
        } ],
        "myEntities" : {
          "href" : "http://localhost:8080/myEntities/5"
        }
      }
    } ]
  },
  "_links" : {
    "self" : {
      "href" : "http://localhost:8080/myEntities{?page,size,sort}",
      "templated" : true
    },
    "profile" : {
      "href" : "http://localhost:8080/profile/myEntities"
    }
  },
  "page" : {
    "size" : 20,
    "totalElements" : 1,
    "totalPages" : 1,
    "number" : 0
  }
}

There is an array with the same reference twice:

 "myEntity" : [ {
          "href" : "http://localhost:8080/myEntities/5"
        }, {
          "href" : "http://localhost:8080/myEntities/5"
        } ],

One is made by Spring Rest and the other one by Spring content. To get the content vs metadata you can play with the Accept header. However this does not seem very clear to me.

@paulcwarren
Copy link
Owner

paulcwarren commented Mar 3, 2020

Thanks @thijslemmens.

As discussed in email, we aren't very smart when it comes to adding resource links atm, adding them regardless. We'll try and make it smarter to remove the duplicates.

That said I am not sure what we can do around the clarity of fetching content versus metadata issue. ALPs (the standard that defines the link schema) isn't any help as it ignores content-type altogether (last time that I checked). The one thing I would say is that when SD repositories and SC stores are exported into the same URI space then SC will ignore any GET request that accepts json or hal+json and will attempt to handle anything else.

The other alternative is to export the SC stores into a different URI space by either specifying a "path" on the StoreRestResource, or probably more preferably, setting the spring.content.rest.base-uri configuration property in your application. Using the example above this would export the StoreRestResource (path="myEntitiesContent") to http://localhost:8080/myEntitiesContent/5 , or (spring.content.rest.base-uri="/content") to http://localhost:8080/content/myEntities/5.

Any other options/suggestions welcome too

@paulcwarren
Copy link
Owner

I just wanted to clarify the StoreRestResource in your example @thijslemmens .

Did you mean to specify this one?
@StoreRestResource public interface AccountStateCollectionContentStore extends ContentStore<AccountStateCollection, UUID> {}

Exporting this store should (?) lead to content links of the form: http://localhost:8080/accountStateCollections/{id}

I am assuming you meant to specify something like:
@StoreRestResource public interface MyEntityContentStore extends ContentStore<MyEntity, UUID> {}

Just wanted to check before I started looking at this to make sure I understood.

@thijslemmens
Copy link
Author

I just wanted to clarify the StoreRestResource in your example @thijslemmens .

Did you mean to specify this one?
@StoreRestResource public interface AccountStateCollectionContentStore extends ContentStore<AccountStateCollection, UUID> {}

Exporting this store should (?) lead to content links of the form: http://localhost:8080/accountStateCollections/{id}

I am assuming you meant to specify something like:
@StoreRestResource public interface MyEntityContentStore extends ContentStore<MyEntity, UUID> {}

Just wanted to check before I started looking at this to make sure I understood.

You are correct. I made a mistake in generalizing my example.

@thijslemmens
Copy link
Author

Updated the original issue

@paulcwarren
Copy link
Owner

paulcwarren commented Mar 4, 2020

As it stands today if an entity is annotated with multiple content properties (i.e. multiple @ContentIds) then, for each, we generate a link relation from its @ContentID field name. This functionality is fairly new. I didn't change single content entities links to match because I didn't want to make a backward incompatible change.

But thinking about it now, in a wider context of content vs. metadata differentiation too, we could adopt this as a uniform approach so for the above example we would end up with something like this:

      "_links" : {
        "self" : {
          "href" : "http://localhost:8080/myEntities/5"
        },
        "myEntity" : {
          "href" : "http://localhost:8080/myEntities/5"
        },
        "content" : {
          "href" : "http://localhost:8080/myEntities/5"
        }
      }

Now, this is backward incompatible and may break consumers out there but if we introduced a contentRel on the StoreRestResource annotation allowing applications to override this behavior back to what it was then we would probably get away with that (plus some release notes obviously).

I think I am keen on this approach as overall it gives us more uniform system behavior and a more customizable framework. I would be interested as to how you view this solution from your perspective @thijslemmens? Presumably, as you are early on in your journey this would be ok?

@thijslemmens
Copy link
Author

@paulcwarren backwards compatibility is not yet an issue for us.
I do think it is confusing to have the same URL's for metadata and content, without the ability to see the difference in HAL.

Why not have a subresource? Something like this:

      "_links" : {
        "self" : {
          "href" : "http://localhost:8080/myEntities/5"
        },
        "myEntity" : {
          "href" : "http://localhost:8080/myEntities/5"
        },
        "content" : {
          "href" : "http://localhost:8080/myEntities/5/content"
        }
      }

@paulcwarren
Copy link
Owner

paulcwarren commented Mar 4, 2020

We already have support for links like this, for content collections, so I was considering similar. I'll look into what it would take to use this style of link for content links too.

Note: although from a hypermedia perspective, where the client follows link relations (linkrels) from a single known entry-point into the application, the actual URL is of little consequence actually. Changing the linkrel will break existing clients that are out there that might be following the existing linkrels.

paulcwarren pushed a commit that referenced this issue Mar 11, 2020
- also make the linkrel configurable
Fixes #148
@paulcwarren
Copy link
Owner

paulcwarren commented Mar 11, 2020

This should be fixed by this commit that is now available on 1.0.0.M7-SNAPSHOT.

In short there is now a spring boot property spring.content.rest.fullyQualifiedLinks. When set to true will produce fully qualified links. You can read more about the feature here.

Will cut a release this week.

I'm going to close this but feel free to open if you still have an issue.

paulcwarren pushed a commit that referenced this issue Oct 27, 2021
- also make the linkrel configurable
Fixes #148
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants