I agree with all points Piotr raises. I am in favor of reverting back to the system we have in `2.x`, because the new property system has more disadvantages than benefits in its current state and it hinders the ongoing operation of sync'ing `2.x` and `main`, and generating API docs from sources.
Recently I added two new properties to `main` for recyclers. Compared to `2.x`, I find the new way of doing things unnecessarily complicated, without any tangible benefit, and undocumented. The new way is also not practiced everywhere. One can see this from property-related warnings during build. The property system rewrite was carried out as a part of the property-toggled modules – both of which are undocumented. The fact that, as Piotr suggests, we can revert the property system rewrite yet keep the property-toggled modules shows that the former could have been addressed in a separate PR. I was the one who tipped Spring's @ConfigurationProperties <https://docs.spring.io/spring-boot/docs/current/reference/html/features.html#features.external-config.typesafe-configuration-properties.java-bean-binding> to Piotr. That is already a well-established practice and compatible with the existing property system we have in `2.x`. Unless we offer something better, I don't see a reason why we shouldn't simply reuse it. On Thu, Jan 4, 2024 at 10:13 AM Piotr P. Karwasz <piotr.karw...@gmail.com> wrote: > Hi all, > > Looking at the `PropertyKey` abstraction in `3.x`, I was wondering > what advantages it brings over simple string constants. From my > perspective it has more cons than pros. > > Pros: > > * it is typesafe; a committer must create an instance of it to use it; > > Cons: > > * it is hard to list for documentation purposes. It would be easier > to document all available properties if they were declared as: > > /** > * Makes Foo into bars. > */ > @Log4jProperty > private static final String FOO_BAR = "Foo.bar"; > > Such a definition can be trivially detected by an annotation > processor, whereas currently I would need to call `PropertyKey#getKey` > to even find the name of the property. > > * the split into "component" and "name" isn't really used in real > code, so `PropertyKey` is basically a wrapper for a single String, > * IMHO the `getSystemKey` method does not belong in `PropertyKey`. It > is something that is used only by those property sources that are > global in the JVM (like env variables or system properties). Other > property sources like `TestPropertySource` have no use of it. > * since a `PropertyKey` is not a compile-time constant, we have > several `Constant` classes that contain string constants, so that we > can use them in tests (e.g. in the `@SetSystemProperty` annotation). > > If you don't have any objections, I would prefer to refactor all these > `PropertyKey` implementations to simple classes that contain only > constant string fields. > > In a **longer** term it would be nice to reintroduce type safety, but > for the **values** of properties instead of their keys. We could copy > the way Spring does it, create classes like: > > /** > * Configures a https://logging.apache.org/log4j/3.x/recycler.html > */ > @Log4jPropertySource(prefix = "Recycler") > public class RecyclerConfig { > > /** > * Type of recycler. > */ > public String type; > > /** > * Size of the recyclers queue. > */ > public int size; > } > > and we could retrieve all the properties at once with > `PropertyEnvironment#getProperties(Recycler.class)`. > > What do you think? >
