Skip to content

MCR-3764 Warn about missing or deprecated classes in configuration#3048

Open
yagee-de wants to merge 1 commit into
2025.06.xfrom
issues/MCR-3764-check-class-configuration
Open

MCR-3764 Warn about missing or deprecated classes in configuration#3048
yagee-de wants to merge 1 commit into
2025.06.xfrom
issues/MCR-3764-check-class-configuration

Conversation

@yagee-de

@yagee-de yagee-de commented Jul 16, 2026

Copy link
Copy Markdown
Member

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.

Bugfix-Specific Checks

  • Affected version is listed in the ticket.
  • Minimal code changes were made (no refactoring).
  • This PR truly fixes only the reported bug.
  • No breaking changes are introduced.
  • A relevant test was added (if feasible).

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?

@yagee-de
yagee-de changed the base branch from main to 2025.06.x July 16, 2026 16:03

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2f7de7eeba

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}

private static boolean isClassProperty(String key, String value, Pattern classPattern) {
return key.endsWith(".Class") || classPattern.matcher(value).find();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Include lowercase class suffixes in the scan

When an application configures a custom class using the supported lowercase .class suffix (for example Foo.Bar.class=com.acme.Missing), this predicate returns false unless the value also matches the org.mycore regex. The configurable-instance code treats .Class and .class equivalently, so valid lowercase properties in non-MyCoRe packages are never included in the startup report, defeating the new missing/deprecated-class check for those extension configs.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

".class" does not respect the mycore.properties naming scheme for properties. After a '.' a capital letter must follow if not other circumstances require a lower case letter. But for configured classes this is not the case so only ".Class" is supported.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, this is only the case since MCR-3670, so only in 2026.06.x and later. In 2025.06.x we still have, for example:

  • MCR.Crypt.Cipher.accesskey.class=...
  • MCR.LayoutService.FoFormatter.class=...

@toKrause
toKrause self-requested a review July 16, 2026 16:08
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