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

Support to receive aggregate references as request parameters #2239

Closed
jacko9et opened this issue Mar 8, 2023 · 7 comments
Closed

Support to receive aggregate references as request parameters #2239

jacko9et opened this issue Mar 8, 2023 · 7 comments
Assignees
Labels
type: enhancement A general enhancement

Comments

@jacko9et
Copy link

jacko9et commented Mar 8, 2023

In the current design, the default behavior is not to serialize the id, but to automatically parse the href into an entity to trigger the query of the entity. The actual business only needs the id as the query identifier. Or is it possible to provide an annotation that resolves id by href without triggering a query.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 8, 2023
@mp911de
Copy link
Member

mp911de commented Mar 8, 2023

I currently do not understand what you're asking for. The core idea of REST is to provide links to the client so that the client doesn't need to construct URLs to obtain an object but can rather navigate across the graph. If that doesn't help, please provide additional details and ideally a scenario that you want to address.

@mp911de mp911de added the status: waiting-for-feedback We need additional information before we can continue label Mar 8, 2023
@odrotbohm
Copy link
Member

I assume what @jacko9et refers to is that with UriToEntityConverter we provide a way for the client to submit a URI (/actors/4711) and bind that to the Actor instance with the identifier of 4711. There currently is no support to actually bind that to the 4711, i.e. simply only extract the ID but not already resolve the entity.

I will have to look into this. It's not as trivial as it seems as currently, referring to the Actor helps us find the type information needed (find the repository for the domain type, find out about the identifier type, use the identifier type to convert the raw 4711 into that, use the repository to eventually resolve the actor). Without the Actor, we don't know about which repository to inspect for identifier type information.

I guess we could rely on the user providing a compatible type (for example Long) but I wonder if this could create situations in which there's a misconfiguration that we cannot properly handle. For instance, the actual identifier type being a UUID and the user declaring a long. I guess the transformation would then just fail with something vaguely equivalent to "String can't be converted to Long", which might just be good enough.

@jacko9et
Copy link
Author

jacko9et commented Mar 9, 2023

The actual query only needs the ID of the entity as a query parameter, but does not need to query the entity, similar to the following query.

List<Account> findByUser(User user);

But at present, the query of the parameter entity(User) is triggered, and then the desired query will continue to be executed. This kind of secondary query, I don’t know if data rest can solve it, or it is a problem with jpa, but it cannot be avoided in data rest. I didn't investigate @backendId carefully, it's not clear if it can be solved.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 9, 2023
@jacko9et
Copy link
Author

jacko9et commented Mar 9, 2023

@odrotbohm got it right. If we write the query method like this: List<Account> findByUser_Id(Long userId); we need to process href parsing or export id. The practice of exporting the id is inconsistent with the thinking of hateoas, so I want to avoid it as much as possible.

@jacko9et
Copy link
Author

jacko9et commented Mar 9, 2023

The ad-hoc way I tried, I hope it helps, but it's tedious and not necessarily correct.

public abstract class EntityHrefUtils {

    private EntityHrefUtils() {
    }

    public static Optional<?> resolveIdByHref(EntityManager entityManager,
                                                  EntityLinks entityLinks,
                                                  String href, Class<?> entityClass) {
        Class<?> idClass = entityManager.getMetamodel().entity(entityClass)
                .getIdType().getJavaType();
        UriTemplate uriTemplate = new UriTemplate(
                entityLinks.linkFor(entityClass).withSelfRel().getHref().concat("/{id}"));
        return Optional.ofNullable(uriTemplate.match(href).get("id"))
                .map(value -> idClass.isAssignableFrom(Long.class) ? Long.parseLong(value) : value);
    }

}
@GetMapping(path = "/search/findByUser")
public @ResponseBody ResponseEntity<?> getUserAccounts(@RequestParam("user") String userHref) {
    return EntityHrefUtils.resolveIdByHref(entityManager, entityLinks, userHref, User.class)
            .map(id -> accountRepository.findByUser_Id((Long) id)
                    .map(account -> EntityModel.of(account,
                            entityLinks.linkToItemResource(Account.class, account.getId()).withSelfRel(),
                            Link.of(userHref, "user")
                    )))
            .map(accounts -> ResponseEntity.ok(
                    CollectionModel.of(accounts.collect(Collectors.toList()),
                            linkTo(methodOn(AccountController.class).getUserAccounts(userHref)).withSelfRel())))
            .orElse(ResponseEntity.notFound().build());
}

@odrotbohm
Copy link
Member

That's interesting information as it further complicates the picture 🙃. To the MVC infrastructure, request parameters and path variables are Strings in the first place. This means, that for the controller you show, a declaration of Long would try to find a converter to convert from String to Long, not URI to Long. Our current conversion service integration registers a converter to convert from URI to a domain type.

I am very sure I don't want to generally hook into the String to primitive ID conversion, as that would cause conflict with Spring's general number parsing. In a type-based conversion system, I don't think we should augment what usually translates "42" into 42 with something that translates "/actor/42" into 42.

I wonder if we should introduce a dedicated type RestReference<T> (name tbd) to provide access to the original URI and the T that could be some entity, identifier or even jMolecules Association and our UriToEntityConverter only ultimately backing the entity resolution.

@jacko9et
Copy link
Author

jacko9et commented Mar 9, 2023

I realize my thinking is a bit naive, this should be unavoidable, and trying to solve this is a bit cross-border, the question should be: how to make URI map domain type and implicitly extract id to apply to query. This is not data rest can things done alone.
But @odrotbohm ‘s proposal should be a good idea (RestReference).

@odrotbohm odrotbohm self-assigned this Mar 10, 2023
@odrotbohm odrotbohm added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Mar 10, 2023
@odrotbohm odrotbohm added this to the 4.1 M3 (2023.0.0) milestone Mar 10, 2023
mp911de added a commit that referenced this issue Mar 20, 2023
Fix invalid Javadoc references.

See #2239
@mp911de mp911de changed the title Is it possible to automatically resolve an entity by href without triggering a query for that entity? Support to receive aggregate references as request parameters Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants