Skip to content

refactor(#158): collapse NotificationServiceImpl to a single constructor#164

Merged
dfcoffin merged 1 commit into
mainfrom
refactor/158-notification-single-constructor
Jun 5, 2026
Merged

refactor(#158): collapse NotificationServiceImpl to a single constructor#164
dfcoffin merged 1 commit into
mainfrom
refactor/158-notification-single-constructor

Conversation

@dfcoffin

@dfcoffin dfcoffin commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Follow-up cleanup to #158 (PR #163).

The #158 sender migration introduced two constructors on NotificationServiceImpl — a production one (built RestClient internally) and a test-seam one (injected a pre-built RestClient). That shape was the footgun that briefly turned CI red: with two constructors and neither @Autowired, Spring reported "No default constructor found" and every full-context @SpringBootTest in common failed. The fix at the time was to add @Autowired — but the cleaner answer is to not have two constructors at all.

Change

  • Single constructor injecting the autoconfigured RestClient.Builder, building the client once. As the sole constructor, Spring selects it for injection with no @Autowired needed — the disambiguation hazard is gone.
  • Test passes its own RestClient.builder() (still points the JDK HttpServer stub at the service); no test-only production constructor remains.

Why RestClient.Builder (not internal RestClient.create())

RestClientAutoConfiguration always provides a prototype RestClient.Builder when spring-web is on the classpath, so injecting it is safe in every context and keeps the client configurable/test-injectable through one path — which was the only reason the second constructor existed.

Verified locally: DataCustodianApplicationH2Test boots the full context, and the codec + producer wire-contract tests pass.

The #158 migration introduced two constructors (production + test-seam)
with the production one @Autowired. That two-constructor shape was the
footgun that briefly broke CI ("No default constructor found" when the
@Autowired was initially missing).

Replace both with a single constructor that injects the autoconfigured
RestClient.Builder and builds the client once. Being the sole constructor,
Spring selects it for injection without @Autowired, and tests pass their
own RestClient.builder(). Net simplification; removes the disambiguation
hazard entirely.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dfcoffin dfcoffin merged commit 36cabc3 into main Jun 5, 2026
4 checks passed
@dfcoffin dfcoffin deleted the refactor/158-notification-single-constructor branch June 5, 2026 19:56
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.

1 participant