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

Deprecate both @Context and @Suspended #1210

Conversation

jim-krueger
Copy link
Contributor

@jim-krueger jim-krueger commented Jan 24, 2024

Fixes #1209
Deprecate @Context and @Suspended in release-3.2 in preparation for removal in release-4.0

@jim-krueger jim-krueger force-pushed the 1209-deprecate_@Context_@Suspended branch from 5dc806e to df42edf Compare January 24, 2024 22:37
@jim-krueger jim-krueger force-pushed the 1209-deprecate_@Context_@Suspended branch from b5e5dd3 to f6dbe89 Compare January 24, 2024 23:07
@jim-krueger jim-krueger merged commit ece5f07 into jakartaee:release-3.2 Jan 24, 2024
4 checks passed
@mkarg
Copy link
Contributor

mkarg commented Jan 27, 2024

This PR is a violation of the committer rules of this project if we take them word-by-word: https://github.com/jakartaee/rest/wiki/Committer-Conventions. There were no three +1 votes, there was no two-weeks voting time. We need to discuss if we set up lower barriers for 3.2 (which I am +1 actually; one way could be to extend the rules by the notion of "working branches", hence
release-3.2 would be merged into a "releasing-branch" (like 3.2) later using a separate PR). Until then I would kindly ask you to respect the rules. Thanks.

@jim-krueger
Copy link
Contributor Author

My apologies Markus. I was thinking of release-3.2 as a "working branch" since we do not have a release plan etc. for it yet and Santiago had requested that we determine if certain issues could be worked through concerning deprecation prior to taking that step. I should have stated that clearly. That being said it sounds like you are not adverse to proceeding this way until such a time as 3.2 is "official", at which time the content of PRs already applied could be confirmed via a vote, with the content removed if found to not be acceptable.

@mkarg
Copy link
Contributor

mkarg commented Jan 27, 2024

I am not standing in the way of dealing with release-3.2 as a "working branch" in that sense, and I think nobody else would consider to reject that idea. Nevertheless it would be best to have an official rule. Hence I would encourage you to start a voting on the mailing list to get this new exemption added into our official ruleset.

@paulrutter
Copy link

paulrutter commented Mar 20, 2024

Just came across this PR while moving to 3.1.
What is the designated replacement for @Suspended? Is it no longer needed, as using AsyncResponse is enough? See #1158 (comment).

I know that @Context is replaced with @Inject, to align with CDI. Thanks.

@jim-krueger

@jamezp
Copy link
Contributor

jamezp commented Mar 20, 2024

@paulrutter Effectively it will be the same, @Inject, but AIUI implied on the method. In other words you simply just remove the @Suspended annotation from your parameter and the implementation will resolve the value.

As an example:

@POST
@Path("async")
@Consumes(MediaType.TEXT_PLAIN)
public void putMessageAsync(
    @QueryParam("override") boolean override,
    String message,
    @Suspended AsyncResponse ar) {
    managedExecutor.submit() -> {
        ar.resume("done");
    });
}

Would become:

@POST
@Path("async")
@Consumes(MediaType.TEXT_PLAIN)
public void putMessageAsync(
    @QueryParam("override") boolean override,
    @Body String message,
    AsyncResponse ar) {
    managedExecutor.submit() -> {
        ar.resume("done");
    });
}

@paulrutter
Copy link

Thanks for clarifying, that makes perfect sense.

@paulrutter
Copy link

paulrutter commented Mar 25, 2024

@jim-krueger one more question about @Context being removed, how would the following example look like after it has been removed?

    @Path("/resetpassword")
    @POST
    @Consumes(APPLICATION_FORM_URLENCODED)
    @Produces(APPLICATION_JSON)
    public Response requestResetPassword(@FormParam(USERNAME) final String username,
            @Context final HttpServletRequest request) {
        // do something with the HttpServletRequest
    }

Is it still possible to get a handle on the HttpServletRequest after @Context has been sunset?

@jim-krueger
Copy link
Contributor Author

@paulrutter This is still a work in progress, but I believe the goal is for it to look like this:

@Path("/resetpassword")
    @POST
    @Consumes(APPLICATION_FORM_URLENCODED)
    @Produces(APPLICATION_JSON)
    public Response requestResetPassword(@FormParam(USERNAME) final String username,
            final HttpServletRequest request) {
        // do something with the HttpServletRequest
    }

You'll want to keep an eye on Issue 1214 as that is where this problem is being tracked.

@paulrutter
Copy link

Thanks, will do.

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.

4 participants