Improvements to the Rest Whiteboard plugin model #11
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jul 11, 2023
There are numerous threading difficulties with maintaining a whiteboard. The existing model would often issue unnecessary change count updates, and sometimes send an update before a resource was ready. The cleanup lifecycle was also somewhat inconsistent. Rather than trying to find and fix the threading bugs this commit substantially rewrites the JerseyServiceRuntime to: * Do the service tracking and management itself, simplifying the provider plugins * Use an "update thread" which is responsible for triggering changes in the whiteboard separately from the service binding thread * Providers now create Jersey Container objects for each application path which are then managed by the JerseyService runtime The overall lifecycle is now substantially simpler. I believe that there is also more code that could be deleted in a subsequent cleanup commit. N.B. One other significant change is that the whiteboard updates are now asynchronous. Where previously a thread registering a whiteboard service could get away with immediately calling it, that thread must now wait for the change count to be updated. Signed-off-by: Tim Ward <timothyjward@apache.org>
The TCK project was only running using the Jetty integration project. This commit adds an additional run for the Servlet Whiteboard, and some fixes to the ServletContextHelper/Servlet registration paths to make the TCK pass with the Servlet Whiteboard Signed-off-by: Tim Ward <timothyjward@apache.org>
maho7791
approved these changes
Jul 11, 2023
| @@ -65,7 +65,7 @@ public void lifeCycleStopping(LifeCycle lifeCycle) { | |||
|
|
|||
| @Override | |||
| public void lifeCycleStopped(LifeCycle lifeCycle) { | |||
| state = JerseyServiceRuntime.State.STARTED; | |||
| state = JettyBackedWhiteboardComponent.State.STARTED; | |||
There was a problem hiding this comment.
Is state STARTED really correct?
The method has the name lifeCycleStopped, so I would expect a similar State STOPPED
There was a problem hiding this comment.
That's a good question. It's not something that I changed (other than the name of the enclosing type). I agree that it looks odd. I think it might be best to look at issue #12 to see if we can remove this completely and rely on the native Jersey implementation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These commits re-work the lifecycle of the whiteboard to simplify providing implementations, and to make more use of native Jersey plug points.