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

PAYARA-4083: Do not use priority annotations for classes without public API #4195

Merged
merged 3 commits into from Sep 5, 2019

Conversation

@pdudits
Copy link
Contributor

commented Sep 3, 2019

Marking CDI bean with @Priority makes it an alternative. Since these classes doesn't implement any public API, application have no means of overriding these classes. And this also breaks one of CDI TCK tests that expect only application alternatives to be present.

36969e1 is refinement of 792efdd, removing unnecessary interface/impl split and injection, since there is only one possible implementation.

pdudits added 3 commits Sep 3, 2019
The interface is not public, and therefore shouldn't be overridable anyway.
The class doesn't implement any public api that would allow it to be
replaced
@pdudits pdudits requested review from jGauravGupta and MarkWareham Sep 3, 2019
@pdudits

This comment has been minimized.

Copy link
Contributor Author

commented Sep 3, 2019

jenkins test please

Copy link
Member

left a comment

Do we also need to make changes to the FaultToleranceInterceptor class?

@jbee

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

Do we also need to make changes to the FaultToleranceInterceptor class?

I don't quite follow the reasoning that @Priority makes things an alternative. At least for FT I can say it is used because it gives order to the interception which is part of the spec so it cannot be removed without breaking the TCK that actually checks it.

@pdudits

This comment has been minimized.

Copy link
Contributor Author

commented Sep 4, 2019

Meaning of @Priority is different amongst an interceptor and regular CDI bean.

For bean it is governed by 5.1.1.1. Declaring selected alternatives for an application, while for interceptors its 9.4. Interceptor enablement and ordering.

@jbee

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

Thanks @pdudits , now this makes sense to me and it should also answer @Pandrex247 question. As FT uses it on an interceptor this should stay as it is.

@pdudits pdudits merged commit 7304b03 into payara:master Sep 5, 2019
51 of 54 checks passed
51 of 54 checks passed
security/snyk - appserver/ant-tasks/pom.xml (payara-ci) Test in progress
security/snyk - appserver/concurrent/pom.xml (payara-ci) Test in progress
security/snyk - appserver/pom.xml (payara-ci) Test in progress
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/persistence/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.