Skip to content

Conversation

@ppkarwasz
Copy link
Contributor

This replaces PropertiesUtil with PropertyEnvironment in modules that depend on Log4j Core.

It also ensures that every logger context has a different version of PropertyEnvironment.

Review kit

The most important changes are:

  • the replacement of PropertyKey implementations in all modules (except log4j-api). The new classes are called CoreKeys, AsyncLoggerKeys, etc.
  • all the constant default values are moved to the *Keys classes,
  • the SystemBundle is removed and service classes are registered programmatically.

The changes in log4j-api and log4j-api-test are transitional (until Log4j API 3.x is removed) and may be disregarded.

@ppkarwasz ppkarwasz force-pushed the fix/apply-property-environment branch from 7757e81 to c3f4261 Compare March 22, 2024 09:40
Copy link
Member

@vy vy left a comment

Choose a reason for hiding this comment

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

This is a big change, but I can see the reason why the PR is massive. Left some minor questions. Did not spot any blockers. Feel free to proceed as you wish. Great job! 💯

ppkarwasz added a commit that referenced this pull request Mar 27, 2024
This replaces `PropertiesUtil` with `PropertyEnvironment` in modules
that depend on Log4j Core.

It also ensures that every logger context has a different version of
`PropertyEnvironment`.
@ppkarwasz ppkarwasz force-pushed the fix/apply-property-environment branch 2 times, most recently from ec802f4 to 54b412a Compare March 27, 2024 17:19
Property names are often inconsistent. This commit:

* changes `log4j.configuration.file` to `log4j.configuration.location`,
  since the value is not necessarily a file,
* the names of property classes end in `Properties`, whereas their
  prefix starts with a small letter,
* the `AsyncLogger` and `AsyncLoggerConfig` properties are merged into
  one class, since they should not be used together,
* the `AuthenticationProvider` is removed from `ConfigurationProperties`
  and placed in a standalone class, since multiple components use it.
@ppkarwasz ppkarwasz force-pushed the fix/apply-property-environment branch from 54b412a to de42610 Compare March 27, 2024 19:04
@ppkarwasz ppkarwasz merged commit 397ef10 into feature/log4j-sdk Mar 28, 2024
@ppkarwasz ppkarwasz deleted the fix/apply-property-environment branch March 28, 2024 10:27
@ppkarwasz ppkarwasz mentioned this pull request Mar 28, 2024
8 tasks
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