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

FISH-7340 Remove obsolete EJBContext methods #6268

Merged
merged 2 commits into from Aug 18, 2023

Conversation

eirbjo
Copy link
Contributor

@eirbjo eirbjo commented Apr 20, 2023

The EJBContext methods getEnvironment, getCallerIdentity and isCallerInRole(Identity) were removed in Jakarta EE 9.

This PR removes these methods from EJBContextImpl.

Since this was probably caused by missing @Override annotations, I went ahead and added that for all overriding methods such that future removals may be detected.

Description

This is an enhancement which removes dead code and also allows Payara to compile on future JDKs where the deprecated Identity class has been removed.

Important Info

Testing

Testing Performed

Ran mvn clean install successfully.

Testing Environment

% mvn -v 
Apache Maven 3.8.6 (84538c9988a25aec085021c365c560670ad80f63)
Maven home: /Users/eirbjo/Development/apache-maven-3.8.6
Java version: 11.0.18, vendor: Oracle Corporation, runtime: /Library/Java/JavaVirtualMachines/jdk-11.jdk/Contents/Home
OS name: "mac os x", version: "12.6.3", arch: "x86_64", family: "mac"

…etCallerIdentity removed in Jakarta EE 9. Also add @OverRide for all overriding methods such that future removals may be detected.
@eirbjo eirbjo marked this pull request as ready for review April 20, 2023 07:01
@eirbjo eirbjo changed the title Remove obsolete EJBContextImpl methods Remove obsolete EJBContex methods Apr 20, 2023
@JamesHillyard
Copy link
Member

Hi @eirbjo,

Thank you for submitting a PR to remove these obsolete methods, it's much appreciated. The internal issue FISH-7340 has been raised to track your changes.

As I commented on your other contribution, #6267, we require you to sign the CLA and send it to us before we can review and accept your changes. Payara/PayaraCLA.pdf at master · payara/Payara

Thank you,
James

@JamesHillyard JamesHillyard changed the title Remove obsolete EJBContex methods FISH-7340 Remove obsolete EJBContext methods Apr 20, 2023
@JamesHillyard JamesHillyard added the PR: Awaiting CLA Contributor does not have a CLA or has submitted an unconfirmed CLA. label Apr 20, 2023
@eirbjo
Copy link
Contributor Author

eirbjo commented Apr 20, 2023

As I commented on your other contribution, #6267, we require you to sign the CLA

CLA signed, it will be good to get these removed. Here is the OpenJDK issue for the removal of java.security.Identity, in case you want to track that for your Payara 5 Enterprise offerings:

https://bugs.openjdk.org/browse/JDK-8191136

My testing on Payara 5 CE indicates it will run just fine since you throw unconditionally in both of the EJBContext methods taking java.security.Identity as parameter or return value.

@smillidge smillidge added PR: CLA CLA submitted on PR by the contributor and removed PR: Awaiting CLA Contributor does not have a CLA or has submitted an unconfirmed CLA. labels Apr 20, 2023
@smillidge
Copy link
Contributor

CLA received

@JamesHillyard
Copy link
Member

Jenkins test please

@aubi
Copy link
Contributor

aubi commented Aug 17, 2023

Jenkins test please

@aubi
Copy link
Contributor

aubi commented Aug 17, 2023

@eirbjo,
Thank you for your PR! I updated the copyright year and if you don't mind, I did some formatting cleanup as we touch the file already...

Copy link
Contributor

@aubi aubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I didn't spot any problem this could cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: CLA CLA submitted on PR by the contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants