Skip to content
This repository was archived by the owner on Nov 15, 2022. It is now read-only.

Fix #22156: Bug in annotation handling in case of plural annotation handlers#22164

Merged
yaminikb merged 4 commits intojavaee:masterfrom
mskdeepak-oracle:null_ejb_provider
Aug 4, 2017
Merged

Fix #22156: Bug in annotation handling in case of plural annotation handlers#22164
yaminikb merged 4 commits intojavaee:masterfrom
mskdeepak-oracle:null_ejb_provider

Conversation

@mskdeepak-oracle
Copy link
Copy Markdown
Member

@mskdeepak-oracle mskdeepak-oracle commented Aug 3, 2017

Fix #22156 - Using a plural annotation such as AdministeredObjectDefinitionsHandler would lead to a call to the processAnnotation function of its singular version AdministeredObjectDefinitionHandler. During this, the ejbProvider is not passed properly leading to a bug in the handling of these annotations.

I have added an additional constructor to facilitate this. Changed the code in all plural annotation handlers.

@mskdeepak-oracle
Copy link
Copy Markdown
Member Author

@glassfishrobot Run CI Tests please

@glassfishrobot
Copy link
Copy Markdown
Contributor

Starting CI tests run

@glassfishrobot
Copy link
Copy Markdown
Contributor

All CI tests successful

Copy link
Copy Markdown
Collaborator

@honghzzhang honghzzhang left a comment

Choose a reason for hiding this comment

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

The changes look ok to me. Were there any tests failed due to this bug and passed with your change?

@mskdeepak-oracle
Copy link
Copy Markdown
Member Author

mskdeepak-oracle commented Aug 3, 2017

No, this was just something I have observed while analysing the jms failure.

Copy link
Copy Markdown
Collaborator

@honghzzhang honghzzhang left a comment

Choose a reason for hiding this comment

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

In general, at this late point of development lifecycle, I would say only submit changes that are really necessary to maintain the stability of the code base. But the changes look harmless enough so I will approve it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants