Skip to content

MCR-3703 Parse configuration properties into tree structure eagerly - #3075

Open
toKrause wants to merge 4 commits into
mainfrom
issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly
Open

toKrause wants to merge 4 commits into
mainfrom
issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly

Conversation

@toKrause

@toKrause toKrause commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Link to jira.

Pull Request Checklist (Author)

Please go through the following checklist before assigning the PR for review:

Ticket & Documentation

  • The issue in the ticket is clearly described and the solution is documented.
  • Design decisions (if any) are explained.
  • The ticket references the correct source and target branches.
  • The fixed-version is correctly set in the ticket and matches the PR's target branch (main).

Feature & Improvement Specific Checks

  • Instructions on how to test or use the feature are included or linked (e.g. to documentation).
  • For UI changes: before & after screenshots are attached.
  • New features or migrations are documented.
  • Does this change affect existing applications, data, or configurations?
    • Yes: Is a migration required? If yes, describe it.
    • Breaking change is marked in the commit message.

Testing

  • I have tested the changes locally.
  • The feature behaves as described in the ticket.
  • Were existing tests modified?
    • Yes: explain the changes for reviewers.

MCR Conventions & Metadata

  • MCR naming conventions are followed
  • If the public API has changed:
    • Old API is deprecated or a migration is documented.
    • If not, no action needed.
  • Java license headers are added where necessary.
  • Javadoc is written for non-self-explanatory classes/methods (Clean Code).
  • All configuration options are documented in Javadoc and mycore.properties.
  • No default properties are hardcoded — all set via mycore.properties.

Multi-Repo Considerations

  • Is an equivalent PR in MIR required?
    • If yes, is it already created?

@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch from 51a916f to 9ac86a4 Compare July 27, 2026 20:07
@toKrause toKrause changed the title MCR-3703 parse configuration properties into tree structure eagerly MCR-3703 Parse configuration properties into tree structure eagerly Jul 27, 2026
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch 7 times, most recently from 8f16049 to cf276f8 Compare July 28, 2026 20:22
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch 5 times, most recently from a9b37c1 to 912f7d1 Compare July 30, 2026 16:09
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch from 912f7d1 to 59e49c8 Compare August 1, 2026 12:56
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch 2 times, most recently from 7c2f162 to f8763d6 Compare August 18, 2026 10:03
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch from f8763d6 to 0bcd956 Compare August 31, 2026 12:58
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch from 0bcd956 to 25d880a Compare September 10, 2026 12:32
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch 2 times, most recently from 89058ee to bbd4aaa Compare September 21, 2026 12:35
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch 2 times, most recently from 579ff2f to 0985f6b Compare September 29, 2026 08:41
@toKrause
toKrause marked this pull request as draft September 29, 2026 08:41
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch from 0985f6b to 3eab65c Compare October 6, 2026 11:07
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch 3 times, most recently from a4876b7 to dd3aee3 Compare October 6, 2026 11:28
@toKrause
toKrause marked this pull request as ready for review October 6, 2026 11:37
@toKrause
toKrause marked this pull request as draft October 6, 2026 12:54
@toKrause
toKrause marked this pull request as ready for review October 7, 2026 10:52
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch 2 times, most recently from 5a64df9 to ad8be26 Compare October 19, 2026 07:32

@sebhofmann sebhofmann left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One more: the record ConfigurationKey in MCRConfiguration2 is unused now that the configuration cache is removed and can be deleted.

});
MCRProperTree properties = annotation.absolute() ? configuration.fullProperties() : configuration.properties();
Map<String, String> rawProperties = treeFinder.apply(properties).toProperties();
rawProperties.remove(MCRInstanceConfiguration.CLASS_KEY);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Behaviour change for prefix patterns like Values.*: The Javadoc of @MCRRawProperties says all properties starting with the given prefix and ., and the old code filtered with key.startsWith(prefix). Now tree.deeplyNested("Values").toProperties() also contains the value of the node itself under the key "", so Foo.Values=x adds "" -> "x" to the map. Conversely, Foo.Values.Class is now removed by this line, while it used to be included as Class.

This affects e.g. MCRXsonifyTransformer (Namespaces.*) and MCRPersistenceUnitDescriptor (Properties.*). MCRInstantiatorRawPropertiesBasicTest covers neither case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added support for those uncommon configurations and expanded the test class. Exact same test pass under main and this PR branch.

Throwables.throwIfUnchecked(e.getCause());
throw new MCRException("Failed to create instance configuration for " + name, e.getCause());
}
return MCRInstanceConfiguration.ofName(superClass, name, getAllPropertiesTree(), Options.IMPLICIT);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With the CONFIGURATIONS cache gone, every getInstanceOf / getSingleInstanceOf call runs resolveValueClass and thus MCRClassTools.forName again, even if the singleton is already in instanceHolder. For classes from plugin JARs in mycore-home/lib, Class.forName first throws a ClassNotFoundException on every call before the extendedClassLoader is tried. The cache was explicitly reintroduced in MCR-3668 (7d9a135). Is dropping it intended, or should the resolved configuration (or at least the value class) still be cached?

}
}

public MCRProperTree nested(String key) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question: Class is now hidden on every level of the tree (keys(), nested()). A map entry named Class (e.g. Prefix.Map.Class=...) used to be possible and is now silently ignored. If that is intended, it should be documented.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed by excluding Class properties only when using a tree node to instantiate a class. Additionally, sentinel values are excluded after they are interpreted.

Comment thread mycore-base/src/main/java/org/mycore/common/config/MCRConfiguration2.java Outdated
Comment thread mycore-base/src/main/java/org/mycore/frontend/cli/MCRDeveloperCommands.java Outdated
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch 4 times, most recently from 73c5ee1 to 8ac5e85 Compare October 22, 2026 12:27
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch from 8ac5e85 to 66d0d7d Compare October 22, 2026 12:31
@toKrause
toKrause force-pushed the issues/MCR-3703_Parse-configuration-properties-into-tree-structure-eagerly branch from 66d0d7d to 61d419e Compare October 22, 2026 12:42
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