Skip to content

Read spring.config.location instead of a property that does not exist - #83

Merged
arefbehboudi merged 1 commit into
ClawRunr:mainfrom
JoseWalker:fix/config-location-property
Aug 13, 2026
Merged

Read spring.config.location instead of a property that does not exist#83
arefbehboudi merged 1 commit into
ClawRunr:mainfrom
JoseWalker:fix/config-location-property

Conversation

@JoseWalker

Copy link
Copy Markdown
Contributor

ConfigurationManager resolves the config file it writes to from the Spring property
spring.allConfig.location:

this.configPath = resolveConfigPath(environment.getProperty("spring.allConfig.location"));

That property does not exist. Nothing in the repository sets it, and it is not a Spring Boot
property, so getProperty always returns null and resolveConfigPath always takes its
hardcoded fallback branch. The effect is that the entire resolveConfigPath logic below the
null check — directory handling, the file: prefix strip, the comma-separated list handling —
is unreachable in practice.

This changes the lookup to spring.config.location, which is what the surrounding code is
clearly written for: stripping a file: prefix and taking the first entry of a comma-separated
list only makes sense for Boot's own config-location property.

Tests

ConfigurationManager had no test coverage. This adds ConfigurationManagerTest (9 tests)
covering the resolution rules and the write path:

  • location pointing at a file, and at a directory
  • file: prefix stripped
  • first entry taken from a comma-separated list
  • writes land in the configured location
  • nested keys are written without discarding siblings
  • updateProperties writes once and publishes one ConfigurationChangedEvent
  • a missing config file yields an empty map (first-run onboarding)

The assertions key off a marker value that only exists in the test's @TempDir. That detail
matters: an earlier draft of these tests asserted on an agent: key and passed before the
fix, because with the typo in place the manager resolves the fallback path and reads the
repository's own application.private.yaml. Pinning to a unique marker makes the tests fail
without the fix, which they now do (8 of 9 red before, all green after).

Scope, and two things left out on purpose

Deliberately limited to the property name.

While writing the tests I ran into a related but separate problem: the location this manager
writes to and the location Boot reads private config from are not the same, so onboarding
does not persist in a packaged jar or in the Docker image. That is a design question rather
than a typo, so I am filing it as its own issue rather than growing this PR.

Making the property live for the first time also exposes two pre-existing gaps in
resolveConfigPath, which this PR does not touch. Flagging them so they are a known choice
rather than a surprise:

  • a classpath: location is not handled — only file: is stripped, so Path.of("classpath:/…")
    becomes a literal path that will not exist, and readApplicationYaml then quietly returns an
    empty map;
  • candidate.replace("file:", "") replaces every occurrence rather than a leading prefix.

Happy to fix either here or in a follow-up, whichever you prefer.

Verified with ./gradlew test (128 tests, 0 failures) and the Modulith / ArchUnit gates.

ConfigurationManager resolved the file it reads and writes from the Spring
property `spring.allConfig.location`. That property is not a Spring Boot
property and nothing in the project sets it, so the lookup always returned
null, the configured location was always ignored, and resolveConfigPath always
took its hardcoded fallback branch. Its directory handling, `file:` prefix
strip and comma-separated list handling were unreachable in practice.

Read `spring.config.location`, which is what the surrounding resolution logic
is written for.

The class had no test coverage; add ConfigurationManagerTest covering location
resolution and the write path. The assertions key off a marker value that only
exists in the test's @tempdir, so they cannot pass by falling back to the
repository's own application.private.yaml.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@arefbehboudi
arefbehboudi self-requested a review August 13, 2026 11:32

@arefbehboudi arefbehboudi 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.

Thank you @JoseWalker. Great job!

@arefbehboudi
arefbehboudi merged commit 323daa0 into ClawRunr:main Aug 13, 2026
1 check 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