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

PanacheChildEntityResource #28450

Open
gmuellerinform opened this issue Oct 7, 2022 · 20 comments
Open

PanacheChildEntityResource #28450

gmuellerinform opened this issue Oct 7, 2022 · 20 comments
Labels
area/panache kind/enhancement New feature or request

Comments

@gmuellerinform
Copy link
Contributor

Description

PanacheEntityResource already provides a nice and quick way to expose entities.
It would be great to have a PanacheChildEntityResource which uses the path parameter containing the parent id, so endpoints like
/product/1/variations
are possible.
Thanks!

Implementation ideas

No response

@gmuellerinform gmuellerinform added the kind/enhancement New feature or request label Oct 7, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 7, 2022

/cc @FroMage, @loicmathieu

@geoand
Copy link
Contributor

geoand commented Oct 7, 2022

cc @Sgitario

@Sgitario
Copy link
Contributor

Sgitario commented Oct 10, 2022

I really like this enhancement.

To elaborate on the implementation details, let's introduce this example:

@Entity
public class Item extends PanacheEntity {
  public String name;
  public Resource resource;
}

@Entity
public class Resource extends PanacheEntity {
  public String type;
  public List<Item> items;
}

At the moment, when having these two entities, Panache will expose the following endpoints:

GET /items - list all the item entities
GET /items/XX - get the item with ID XX
POST /items/XX - update the item with ID XX
DELETE /items/XX - delete the item with ID XX

GET /resources - list all the resource entities
GET /resources/XX - get the item with ID XX
POST /resources/XX - update the item with ID XX
DELETE /resources/XX - delete the item with ID XX

And the idea of this feature is somehow to also expose the child entities like the resource field in the Item entity, so we can have the following additional endpoints:

GET /items/XX/resource - list all the resource entities within the item entity with ID XX.
GET /items/XX/resource/YY - get the resource with ID YY within the item entity with ID XX.
POST /items/XX/resource/YY - update the resource with ID YY within the item entity with ID XX.
DELETE /items/XX/resource/YY - delete the resource with ID YY within the item entity with ID XX.

The question now is how to configure the child entities to expose these additional resources. These are some options:

a. Either use an interface or abstract class like PanacheChildEntityResource as suggested in the issue description

Example:

@Entity
public class Resource extends PanacheEntity implements PanacheChildEntityResource {
  public String type;
  public List<Item> items;
}

I don't like this approach because you could not expose the child entities for only one single resource, but for all. Also, the child word does not make sense in the context of an entity definition itself, but when we're setting the composition of the parent entity (Item in the example).

b. Or use a new annotation @SubResourceProperties with the same attribute as the @ResourceProperties annotation

Example:

@Entity
public class Item extends PanacheEntity {
  public String name;
  @SubResourceProperties
  public Resource resource;
}

The advantages of this approach that I foresee are:

  • We could easily configure the child resource properties like the path to use, the hal flag, or the paged attribute.
  • This approach would work the same if we use a repository (the PanacheRepositoryResource class) to expose the entities.

Let me know your thoughts about the implementation details to proceed with.

@gmuellerinform
Copy link
Contributor Author

Hi! Thanks for picking this up so quickly. I have no opinion on the implementation. I just would like to see a solution and best practice provided for the problem. Thanks!

@Sgitario
Copy link
Contributor

@geoand are you ok with proceeding with the second approach?

@geoand
Copy link
Contributor

geoand commented Oct 10, 2022

Seems reasonable to me, but let's also see what @loicmathieu thinks

@Sgitario
Copy link
Contributor

Sgitario commented Oct 11, 2022

UPDATE
I changed my mind about my suggested solution to this issue. Since the generated resources are managed by the presence of either PanacheEntityResource or PanacheRepositoryResource (see here: https://quarkus.io/guides/rest-data-panache#hr-generating-resources).
Perhaps, the best place where to configure sub-resources is also using the existing annotation @ResourceProperties. Example:

@ResourceProperties(subResources = { @SubResourceProperties(of = "resource") })
public interface ItemResource extends PanacheEntityResource<Item, Long> {
}

Or using the repository:

@ResourceProperties(subResources = { @SubResourceProperties(of = "resource") })
public interface ItemRepository extends PanacheRepositoryResource<Item, Long> {
}

The additional benefit is that we keep the entity for persistence configuration only and the PanacheEntityResource and the PanacheRepositoryResource classes to purely configure the autogenerated resources.
The drawback is that we need to specify the field or property to expose (in the example, is resource).

@gmuellerinform
Copy link
Contributor Author

Also, for multiple child resources there would be a need for such a config anyway...

@FroMage
Copy link
Member

FroMage commented Oct 12, 2022

Well, forgive me for ruining the party, but what is the real use-case here? What benefit do we have for getting nested resources by URI over fetching them from the parent? Or is it that they're not included in the parent representation?

@Sgitario
Copy link
Contributor

As far as I see it, the use case here is to individually customize the child entities' resources (sub-resources). For example, users want to secure the parent resource with the role "admin", but allow other roles like "user" to use the child's resources, so administrators can manage the parent entities and allow other issues with fewer permissions to list/add/remove/update the child entities.

@gmuellerinform reported this related issue #28507 which goes in this direction.

@FroMage
Copy link
Member

FroMage commented Oct 13, 2022

This doesn't ring true to me. In my experience, admins in small shops have every permission, while other users are restricted access based on ownership/permissions of the resource itself, which is never on static "roles".
Also, the permission check would be exactly just as complex for /subresource/1 as for /parent/1/subresources/1 so this doesn't convince me.

@Sgitario
Copy link
Contributor

This doesn't ring true to me. In my experience, admins in small shops have every permission, while other users are restricted access based on ownership/permissions of the resource itself, which is never on static "roles".

On the other hand, the example I introduced was very true to me. The idea is that users, using it in one way or the other, can use this extension.

Also, the permission check would be exactly just as complex for /subresource/1 as for /parent/1/subresources/1 so this doesn't convince me.

Sorry, I don't understand your point here. I was not speaking about the complexity of doing this, but that users can't do this at the moment because we're not generating "/parent/1/subresources/1".

However, I only wanted to give an example (the roles) of a real use case. Another point is that having to provide the whole entity to update one sub-resource entity, it's sub-optimal. Maybe, @gmuellerinform can share his use case here too, but I do think it's a good enhancement.

@gmuellerinform
Copy link
Contributor Author

I am just looking for an easy to use CRUD resource, which includes permissions and child management.
Currently one would need to add a custom resource for every entity with relations. Or is there already a way to get all subresources for specific parent without writing additional code?

Being able to define your Entities and spin up a complete and secure CRUD API would drastically improve productivity.
Thanks!

@gmuellerinform
Copy link
Contributor Author

@FroMage
Copy link
Member

FroMage commented Oct 14, 2022

Currently one would need to add a custom resource for every entity with relations. Or is there already a way to get all subresources for specific parent without writing additional code?

OK, so what I understand here is that you don't want to expose the child resources as toplevel resources, because they only make sense as belonging to other parent resources? Like GitHub issues belonging to GitHub projects?

Now, this makes a lot of sense to me. That's a use-case that is clear and justified.

So, if you had the following entities:

@Entity
public class Project extends PanacheBaseEntity {
 @Id
 public String name;

 @OneToMany(mappedBy = "project")
 public List<Issue> issues;
}

@Entity
public class Issue extends PanacheBaseEntity {
 @Id
 public long number;

 @ManyToOne
 public Project project;
}

public interface ProjectResource extends PanacheEntityResource<Project, String> {
}

Then what would the representation look like for a given project with two entities? As it currently stands?

@gmuellerinform
Copy link
Contributor Author

It would look like

/projetct/{project_id}/issues/{issued_id}

I think this would be a nice way for the representation. Top-level children would be okay too, if there was a filter in the PanacheEntityResource, like /issues?project=1

My agenda is to promote Quarkus in my company and I would like to show a fully functional CRUD without any resource code.

@FroMage
Copy link
Member

FroMage commented Oct 24, 2022

I was asking about the entity representation: what does the JSON look like ATM?

@gmuellerinform
Copy link
Contributor Author

Oh, sorry. I guess this depends on your API style?! I currently use @JsonIdentityReference(alwaysAsId = true) to return a list of ids:

{
  "name": "myproject",
  "issues": [1, 2]
}

With the nested routes, issues could be left out in the json as well, but this should be the developers choise?!

I also plan to use JsonViews in future to let the client chose verbosity.

@FroMage
Copy link
Member

FroMage commented Oct 26, 2022

OK, so we currently return the IDs. Up to the caller to know how to map them into a URI. I'd probably rather we return links to the issues, no?

@gmuellerinform
Copy link
Contributor Author

gmuellerinform commented Mar 15, 2023

Hi! Sorry for the quiet time, was busy with other projects. @FroMage was aking for a use case earlier. One I encountered a couple of times now is "give me all issues for project X filtered by date". Currently I need to fetch the project, and query every issue. Even if the issues are exported, since I cannot query for the project in the issues resource (only primitive types accoding to the docs). So, the initially suggested sub resource is a way to allow this selection. But, being able to query for relations (via id) on the issues resource would actually be better, and maybe easier. Thank you for your time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/panache kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants