rest icon indicating copy to clipboard operation
rest copied to clipboard

Jakarta REST 5.0 Epic

Open spericas opened this issue 2 years ago • 19 comments

/The proposal is carry out all this work in the release-4.0 branch (we can rename this branch later if necessary). After checking out that branch, review presentation in JakartaRest40.pdf for some background info on the 4.0 release.

General

  • [x] Clean pom files, use dependency and plugin management, verify all versions
  • [x] Add static dependency with CDI
  • [x] Update README, LICENSE and review all other top-level files
  • [ ] Explore better integration with Jakarta Concurrency
  • [ ] Does Jakarta REST support CDI Lite in addition to CDI Full?
  • [ ] Are CDI executable methods usable by Jakarta REST implementations? (https://github.com/jakartaee/cdi/pull/639)
  • [ ] Review copyrights and licenses in all files (especially pom files)

API

  • [ ] Remove all use of @Context and @Suspended and related classes
  • [ ] Review support for Application subclasses: zero or more? zero or one?
  • [ ] If no Application subclasses, use CDI for scanning
  • [ ] New @Body annotation

Specification

  • [ ] Remove all uses of @Context and @AsyncResponse and related classes
  • [ ] Rewrite introduction to describe new API philosophy and integration with CDI
  • [ ] Define CDI scopes for resources, providers and applications

Examples

  • [ ] Verify all examples are updated with new API, no @Context etc.
  • [ ] Add new examples that show CDI integration
  • [ ] Remove any examples that are not longer relevant

TCKs

  • [ ] Port all TCK tests to run with new 5.0 API
  • [ ] New TCK tests to better show integration with CDI

spericas avatar Jun 26 '23 15:06 spericas

Is there an issue filed about removing @AsyncResponse? I couldn't find anything in my search, but I may have just missed it.

jamezp avatar Jun 26 '23 16:06 jamezp

Is there an issue filed about removing @AsyncResponse? I couldn't find anything in my search, but I may have just missed it.

I am not aware that we decided to do so.

@spericas Why using a new branch? Why not developing on master branch?

mkarg avatar Jun 27 '23 05:06 mkarg

Is there an issue filed about removing @AsyncResponse? I couldn't find anything in my search, but I may have just missed it.

I am not aware that we decided to do so.

There's no longer a need for the annotation if @Entity is introduced as you an see in the PDF file. The param type is sufficient to determine the type of call.

@spericas Why using a new branch? Why not developing on master branch?

We can switch after the TCK work (challenges) is complete.

spericas avatar Jun 27 '23 13:06 spericas

Is there an issue filed about removing @AsyncResponse? I couldn't find anything in my search, but I may have just missed it.

I don't think so. Note that it's easy to create issues by clicking on one of the epic bullets above ...

spericas avatar Jun 27 '23 13:06 spericas

Is there an issue filed about removing @AsyncResponse? I couldn't find anything in my search, but I may have just missed it.

I don't think so. Note that it's easy to create issues by clicking on one of the epic bullets above ...

Sorry for the delay. I'm not sure I fully grok this part yet so I need to do some reading.

Overall though, I think the proposal makes sense. I've got some CDI questions, especially around clients, but that seems more of a fit for CDI specific issue. I need to re-read that issue anyway since it's been a while :)

jamezp avatar Jun 30 '23 15:06 jamezp

Top-level files will be fine when my recently opened PRs are resolved. Checking the issue already, as there are no top-level files to be modified.

Update: PR was merged meanwhile.

mkarg avatar Aug 13 '23 18:08 mkarg

Hi all, I'm looking for some more information about the new @Entity annotation.

I can't find any discussion around it other than "Use of new annotation @Entity is required" in @spericas presentation PDF.

This seems to just make the method signature more verbose without adding any additional functionality. Is this correct? Or does this annotation impact additional behavior that I am unaware of?

WhiteCat22 avatar Aug 31 '23 16:08 WhiteCat22

@WhiteCat22 I could be wrong, but my understanding is we need a way to indicate which part of a method signature is the entity because the @Context and @Suspend annotations are being removed. In the document the following example is used:

@POST
@Path("async")
@Consumes(MediaType.TEXT_PLAIN)
public void putMessageAsync(
  @QueryParam("override") boolean override,
  @Entity String message,
  AsyncResponse ar) {
    Executors.newSingleThreadExecutor().submit(() -> {
        // …
        ar.resume("Done");
    });
}

Since @Suspend is not longer available, we need the @Entity to determine which parameter is the entity.

In some ways it could be considered less verbose because @Context and @Suspend will not be required as arguments for things like Sse, SseEvent, AsyncResponse, etc.

jamezp avatar Aug 31 '23 16:08 jamezp

@WhiteCat22 @jamezp Not just that, with CDI taking control of the method call, developers will be able to inject any CDI bean as a parameter, but only one of those injections will correspond to the request entity.

spericas avatar Aug 31 '23 16:08 spericas

Makes sense! Thank you!

WhiteCat22 avatar Aug 31 '23 17:08 WhiteCat22

Is removal of the JAXB dependency related to this issue?

Should Remove all use of @Context and @AsyncResponse and related classes read Remove all use of @Context and @Suspended and related classes?

WhiteCat22 avatar Aug 31 '23 21:08 WhiteCat22

Is removal of the JAXB dependency related to this issue?

Sort of, it is more related to 4.0 in general, not directly related to the CDI work.

Should Remove all use of @Context and @AsyncResponse and related classes read Remove all use of @Context and @Suspended and related classes?

Fixed, thanks.

spericas avatar Sep 01 '23 12:09 spericas

Hi all, after presenting RESTFulWS-4.0 to the rest of the Open Liberty organization @jim-krueger and I received feedback on the new @Entity annotation. I opened https://github.com/jakartaee/rest/issues/1183 for discussion.

WhiteCat22 avatar Sep 13 '23 21:09 WhiteCat22

There is action item above for adding TCK for the new Multipart function added in version 3.1. Should we also add an action item to add TCK for the requirement of the default exception mapper in 3.1? Issue 858. I don't believe any new tests were added.

jim-krueger avatar Oct 12 '23 20:10 jim-krueger

There is action item above for adding TCK for the new Multipart function added in version 3.1. Should we also add an action item to add TCK for the requirement of the default exception mapper in 3.1? Issue 858. I don't believe any new tests were added.

Sure, I'm not sure either if tested in the TCK. Opening an issue for investigation seems like the right step.

spericas avatar Oct 16 '23 13:10 spericas

Sure, I'm not sure either if tested in the TCK. Opening an issue for investigation seems like the right step.

Agreed. Created Issue 1188 and updated checklist on top.

jim-krueger avatar Oct 16 '23 13:10 jim-krueger

With the Milestone 1 deadline approaching December 5th. Are there thoughts on what things above must be in the spec/api's for M1 and what things are optional for that deadline?

jim-krueger avatar Oct 30 '23 22:10 jim-krueger

@jim-krueger I'd say optional would be TCKs and integrations with other specs, mostly everything else should be there. I'd say going over the spec and updating it to reflect the changes in 4.0 would be high priority. If someone can volunteer for that, it would be very helpful.

spericas avatar Nov 06 '23 18:11 spericas

@spericas I made a first pass at some spec changes for M1. Please give it a review when you can. Thanks. https://github.com/jakartaee/rest/pull/1192

jim-krueger avatar Nov 15 '23 21:11 jim-krueger