refactor: migrate @ConfigurationProperties from @Component to @EnableConfigurationProperties#264
Conversation
… @EnableConfigurationProperties Remove @component and @propertysource from DevLoginConfigProperties and WebAuthnConfigProperties so they are passive data holders per Spring Boot convention. Registration is now handled by dedicated @configuration classes with @EnableConfigurationProperties: - DevLoginAutoConfiguration: guards with @Profile("local") and @ConditionalOnProperty, owns @propertysource - WebAuthnAutoConfiguration: always active (WebSecurityConfig requires the properties), owns @propertysource Closes #263
There was a problem hiding this comment.
Pull request overview
This pull request refactors two @ConfigurationProperties classes to follow Spring Boot conventions by removing @Component from the properties classes and introducing dedicated auto-configuration classes with @EnableConfigurationProperties.
Changes:
DevLoginConfigPropertiesandWebAuthnConfigPropertiesare now passive data holders without lifecycle annotations- New
DevLoginAutoConfigurationandWebAuthnAutoConfigurationclasses register the properties and own the activation logic - Guards (
@Profile,@ConditionalOnProperty) and property source declarations were moved from properties classes to configuration classes
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
WebAuthnConfigProperties.java |
Removed @Component and @PropertySource annotations to make it a passive data holder |
WebAuthnAutoConfiguration.java |
New configuration class that registers WebAuthnConfigProperties via @EnableConfigurationProperties and declares the property source |
DevLoginConfigProperties.java |
Removed @Component, @PropertySource, @Profile, and @ConditionalOnProperty annotations |
DevLoginAutoConfiguration.java |
New configuration class that registers DevLoginConfigProperties via @EnableConfigurationProperties, owns the activation guards (@Profile, @ConditionalOnProperty), and declares the property source |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PR Review: Migrate
|
- Rename DevLoginAutoConfiguration → DevLoginConfiguration and WebAuthnAutoConfiguration → WebAuthnConfiguration to avoid implying Spring Boot SPI semantics (these are component-scanned, not SPI-registered via AutoConfiguration.imports) - Remove @propertysource from DevLoginConfiguration: the annotation cannot influence its own @ConditionalOnProperty evaluation (condition is checked before @propertysource is processed), and defaults are already available via WebAuthnConfiguration which unconditionally loads dsspringuserconfig.properties
Follow-up ReviewThe second commit (23047ca) directly addresses both issues raised in the initial review:
Current state looks goodOne minor observation (non-blocking): One design note worth acknowledging: VerdictBoth review items resolved. The refactor is clean — properties classes are now passive POJOs, activation guards live in dedicated configuration classes, and the |
Summary
@Componentand@PropertySourcefromDevLoginConfigPropertiesandWebAuthnConfigPropertiesso they are passive data holders per Spring Boot conventionDevLoginAutoConfigurationwith@Profile("local")+@ConditionalOnPropertyguards and@EnableConfigurationPropertiesto registerDevLoginConfigPropertiesWebAuthnAutoConfigurationwith@EnableConfigurationPropertiesto registerWebAuthnConfigProperties(always active sinceWebSecurityConfigrequires it)Closes #263
Test plan
./gradlew test— all existing tests pass with no changes neededDevLoginIntegrationTest— beans register with local profile + enabled propertyDevLoginDisabledTest— beans don't register when disabledDevLoginControllerTest— unit test unaffectedWebAuthnFeatureEnabledIntegrationTest— WebAuthn beans register when enabledWebAuthnFeatureDisabledIntegrationTest— WebAuthn beans don't register when disabled