Skip to content

Refactor GrpcDeploymentService: unify instantiation, fix cleanup, move interceptors out of loop - #512

Merged
jmesnil merged 2 commits into
wildfly-extras:mainfrom
jmesnil:worktree-refactor-deployment-service
Oct 14, 2026
Merged

jmesnil merged 2 commits into
wildfly-extras:mainfrom
jmesnil:worktree-refactor-deployment-service

Conversation

@jmesnil

@jmesnil jmesnil commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Move createInterceptors() before the service loop — interceptors are deployment-scoped, not service-scoped; creating them inside the loop was wasteful and semantically wrong.
  • Unify CDI/reflection instantiation — createServiceInstance, createInterceptorInstance, createViaReflection, and createInterceptorViaReflection are collapsed into a single generic createInstance(Class<T>) method, removing ~50 lines of duplication.
  • Extract unregister() + runSafely() helpers — cleanup code shared between start() error recovery and stop() is now in one place; exceptions thrown during cleanup accumulate as suppressed exceptions rather than being silently dropped.
  • Fix @Priority sentinel — unannotated interceptors used Integer.MAX_VALUE - 1; changed to Integer.MAX_VALUE so the sentinel is unambiguously "lowest priority".

These are purely internal refactoring changes to GrpcDeploymentService.java. No functional behaviour changes, no API changes, no other files touched.

@jmesnil
jmesnil marked this pull request as ready for review October 7, 2026 12:30
@jmesnil
jmesnil requested a review from hpehl as a code owner October 7, 2026 12:30
@jmesnil
jmesnil removed the request for review from hpehl October 7, 2026 12:35
@jmesnil
jmesnil force-pushed the worktree-refactor-deployment-service branch from 36ec2be to 26d6ad7 Compare October 8, 2026 07:49
jmesnil and others added 2 commits October 13, 2026 10:47
…e interceptors out of loop

- Move createInterceptors() before the service loop: interceptors are
  deployment-scoped, not service-scoped; instantiating them once per
  service was wasteful and semantically wrong.

- Merge createServiceInstance/createInterceptorInstance and their
  reflection fallbacks into a single generic createInstance(Class<T>)
  method, eliminating ~50 lines of duplication.

- Extract cleanup into unregister() + runSafely() helpers so that
  exceptions thrown during stop() or error recovery accumulate as
  suppressed exceptions rather than being silently dropped.

- Fix @priority sentinel: unannotated interceptors were sorted with
  Integer.MAX_VALUE - 1; use Integer.MAX_VALUE so the sentinel is
  unambiguously "lowest priority".

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…failures

- Replace catch (Exception e) + instanceof chain with catch (StartException | RuntimeException e) + throw e, using Java's precise rethrow so each exception type is raised as-is with no wrapping and no loss of suppressed exceptions.
- Add LOGGER.failedToCleanupGrpcResources (WFLYGRPC-0013, WARN) to runSafely() so exceptions swallowed during stop() are visible in the server log rather than silently discarded.
- Fix comment typo: 'service sof' -> 'services of'.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@jmesnil
jmesnil force-pushed the worktree-refactor-deployment-service branch from e56e496 to 867a866 Compare October 13, 2026 08:47
@jmesnil
jmesnil requested a review from hpehl October 13, 2026 11:03

@hpehl hpehl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The refactoring looks good to me. Thanks @jmesnil

@jmesnil
jmesnil merged commit 5df93a9 into wildfly-extras:main Oct 14, 2026
3 checks passed
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.

2 participants