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

4.x MediaContext requires .build() #7212

Closed
Tracked by #7023
romain-grecourt opened this issue Jul 17, 2023 · 2 comments
Closed
Tracked by #7023

4.x MediaContext requires .build() #7212

romain-grecourt opened this issue Jul 17, 2023 · 2 comments
Assignees
Labels
4.x Version 4.x builder Related to the builder support design This issue requires design/architecture decisions Níma Helidon Níma
Milestone

Comments

@romain-grecourt
Copy link
Contributor

  • Helidon Version: 4.0.0-ALPHA6

The following does NOT include the default media support that is registered by io.helidon.nima.http.media.MediaContextBuilderInterceptor.

 WebServer.builder().mediaContext(MediaContext.builder());

Instead you get a MediaContext that is empty and unable to read or write any entity types.

However if you invoke .build() everything works ok.

WebServer.builder().mediaContext(MediaContext.builder().build());

The issue is that the builder implements the config type, and the config type is consumed by other parts.

// this works because the builder implements MediaContextConfig
// not because the method is overloaded to accept Supplier<MediaContextConfig>
// and the call the .get() is equivalent to .build()
WebServer.builder().mediaContext( MediaContext.builder());
@romain-grecourt romain-grecourt added 4.x Version 4.x Níma Helidon Níma builder Related to the builder support labels Jul 17, 2023
@romain-grecourt romain-grecourt added this to the 4.0.0-M2 milestone Jul 17, 2023
@m0mus m0mus added the design This issue requires design/architecture decisions label Jul 20, 2023
@tomas-langer
Copy link
Member

Proposed approach:

  • BuildBase no longer implements prototype interface
  • we generate getters for all fields, using Optional as return type for:
    • any field that is required (even primitives - as we generate boxed types for such cases, to be able to validate)
    • any field that does not have a default value and is of an object type
    • any field that returns Optional on prototype itself
  • refactor all cases where this is used in builder interceptors

@tomas-langer
Copy link
Member

Part 1 is done - we no longer accept builder where it should not have been accepted.
Part 2 - add methods that accept Supplier<? extends X> when a type is a Prototype

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x Version 4.x builder Related to the builder support design This issue requires design/architecture decisions Níma Helidon Níma
Projects
Archived in project
Development

No branches or pull requests

3 participants