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

Make JsonApiModel public #59

Closed
wants to merge 1 commit into from

Conversation

samuelsalon
Copy link

Hey there. We were using JsonApiModel in our company to implement RepresentationModelAssembler which takes as a generics object we want to represent as a model and representation model as a D extends RepresentationModel<?>. Now we are rewriting the module into reactive web flux so we implementing ReactiveRepresentationModelAssembler where the model is represented by D extends RepresentationModel<D>. This specified D in RepresentationModel<D> forces us to use some representation model instead of ?, so we wanted to use JsonApiModel, but it is package-private. Is there any possibility that this model will be changed to public?

@toedter
Copy link
Owner

toedter commented May 23, 2022

I had lots of discussion with the HATEOAS team about that and the recommendation was to not expose concrete representation models for custom media types as public API. I will discuss this again and let you know about the outcome.

@codecov
Copy link

codecov bot commented May 24, 2022

Codecov Report

Merging #59 (1a9ef46) into master (29e3ad5) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #59   +/-   ##
=========================================
  Coverage     93.81%   93.81%           
  Complexity      377      377           
=========================================
  Files            31       31           
  Lines          1100     1100           
  Branches        196      196           
=========================================
  Hits           1032     1032           
  Misses           22       22           
  Partials         46       46           
Impacted Files Coverage Δ
...m/toedter/spring/hateoas/jsonapi/JsonApiModel.java 90.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29e3ad5...1a9ef46. Read the comment docs.

@toedter
Copy link
Owner

toedter commented May 26, 2022

After considering your suggestion I decided to leave the visibility of JsonApiModel as it is right now. The main reasons are not not expose the implementation details as public API and to go only with the Spring HATEOAS public API when it comes to representation models, which is the recommended behavior for custom Spring HATEOAS media types.

I would suggest that you implement your very own RepresentationModelAssembler that knows about your DTOs and uses the JsonApiModelBuilder internally to create things like Mono<RepresentationModel<?>>

If I find the time, I will try to provide an example in https://github.com/toedter/spring-hateoas-jsonapi-examples

@toedter toedter closed this May 26, 2022
@jimirocks
Copy link
Contributor

jimirocks commented May 30, 2022

@toedter My colleague may wasn't specific enough - below is the example of trying to implement ReactiveRepresentationModelAssembler on top of very specific DTO class. The code is not compilable (also the kotlin equivalent won't be). That is because it's impossible to declare correct generic type of D extends RepresentationModel<D> which would be compatible with what JsonApiModelBuilder.build() returns.

import com.toedter.spring.hateoas.jsonapi.JsonApiModelBuilder;
import org.springframework.hateoas.CollectionModel;
import org.springframework.hateoas.RepresentationModel;
import org.springframework.hateoas.server.reactive.ReactiveRepresentationModelAssembler;
import org.springframework.web.server.ServerWebExchange;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

public class JavaAssembler extends ReactiveRepresentationModelAssembler<MyStupidDto, RepresentationModel<?>> {
    @Override
    public Mono<RepresentationModel<?>> toModel(final MyStupidDto entity, final ServerWebExchange exchange) {
        return Mono.just(JsonApiModelBuilder.jsonApiModel().model(entity).build());
    }

    @Override
    public Mono<CollectionModel<RepresentationModel<?>>> toCollectionModel(final Flux<? extends MyStupidDto> entities, final ServerWebExchange exchange) {
        throw new UnsupportedOperationException("not implemented");
    }
}

@toedter
Copy link
Owner

toedter commented May 31, 2022

Just fyi, I filed an issue at Spring HATEOAS:
spring-projects/spring-hateoas#1796

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

Successfully merging this pull request may close these issues.

3 participants