Alter Configuration draft PR with Single String path and String path separator

[jansupol]

Redesigned Configuration path and a separator from the original @ljnelson draft PR.

[jansupol]

@radcortez I assume this would satisfy your comment in the original draft PR.

[jansupol]

Note that another PR with altering the Loader class using the same single String path and separator comes later. While the single Configuration change is more Option 1, the additional Loader change is Option 2.

[radcortez]

@radcortez I assume this would satisfy your comment in the original draft PR.

Better, but I'm not sure if it is a good thing to allow setting the separator (at least per class).

Let's have a look into the API usage:

Proposed:

@Configuration(path = {"my", "prefix"})
interface MyConfig {
}

Loader.load(List.of("my", "prefix"), MyConfig.class);
// or I guess that just passing in the class name should be enough, but I only found the above method in the Option 2 PR
Loader.load(MyConfig.class)

With the separator:

@Configuration(path = "my.prefix")
interface MyConfig {
}

Loader.load("my.prefix", MyConfig.class);
Loader.load(MyConfig.class)
// Or if we allow to load the same class but override the prefix, I think adding the path as second argument works better:
Loader.load(MyConfig.class, "another.prefix");

With a different separator:

@Configuration(path = "my.prefix")
interface MyConfig {
}

@Configuration(path = "another_prefix", separator = "_")
interface AnotherConfig {
}

Loader.load("my.prefix", MyConfig.class);
Loader.load("another_prefix", AnotherPrefix.class);

my.prefix.foo=bar
another_prefix_foo=bar

Is this something we want to support? I think it won't be very clear when you should use dot or any other separator, you will have to go and check each mapping class to know which one to use.

Is there any reason to not adopt dot as a single representation to separate paths to load the configuration?

[jansupol]

@radcortez Thank you for your reviewing it.

The separator is a matter of use in the path. The path might have a different separator than the source (config.properties,...), which we have not discussed yet. So at the moment, this PR assumes the source would be assumed to have a dot separator, whereas the path used for loading it can have a different separator. Strictly speaking, the separator is a matter of the canonical representation i.e. mapping to the configuration key, as was set in the terminology in this project.

So, even though the source would be my.prefix.foo=bar, you can query the configuration with Loader.bootstrap().separator("/").path("my/prefix").load(MyConfig.class).

The use-case would be to set the separator as ".", even though the source (csv for instance in the future) would have "|", or use menu-like navigation such as separator("->").path("menu->file->save as")

That being said, I assume the usage of a separator would be rare.

You have brought up a good question though. Should the programmatic API recognize the values from the Configuration annotation or not.

If yes, you could have just Loader.load( AnotherPrefix.class);. If not, you would need to set separator, i.e. Loader.load("another_prefix", "_", AnotherPrefix.class); or Loader.separator("_").path("another_prefix").load(AnotherPrefix.class)

I assume you would like to inherit the values set by the annotation?

[radcortez]

The separator is a matter of use in the path. The path might have a different separator than the source (config.properties,...), which we have not discussed yet. So at the moment, this PR assumes the source would be assumed to have a dot separator, whereas the path used for loading it can have a different separator. Strictly speaking, the separator is a matter of the canonical representation i.e. mapping to the configuration key, as was set in the terminology in this project.

Yes, I know we discussed keeping loading separate from the source, and that is fine, so a loading separator does not have to be the same as its particular source. Even, multiple sources can have different separators

So, even though the source would be my.prefix.foo=bar, you can query the configuration with Loader.bootstrap().separator("/").path("my/prefix").load(MyConfig.class).

The use-case would be to set the separator as ".", even though the source (csv for instance in the future) would have "|", or use menu-like navigation such as separator("->").path("menu->file->save as")

That being said, I assume the usage of a separator would be rare.

Being rare, should we give the option to change it? Because its is for the loading part, and we can control how to handle the loading, we can specify what should be the separator in the canonical representation of the path on the loading part. At the very least, if we want to make this configurable, let's make it global and not per path segment, so it is clear which separator is being used per application and not per config object.

You have brought up a good question though. Should the programmatic API recognize the values from the Configuration annotation or not.

If yes, you could have just Loader.load( AnotherPrefix.class);. If not, you would need to set separator, i.e. Loader.load("another_prefix", "_", AnotherPrefix.class); or Loader.separator("_").path("another_prefix").load(AnotherPrefix.class)

I assume you would like to inherit the values set by the annotation?

I'm a fan of reusing information we already have :) If the programmatic call is passing the same values already available in the annotation, then I should have an API that uses those instead and another API that could allow me to override them (if there is any sense in providing such API).

[jansupol]

At the very least, if we want to make this configurable, let's make it global and not per path segment, so it is clear which separator is being used per application and not per config object.

As you said, multiple sources can have different separators. The same is for resolving the configuration object, each resolve can have a different separator.

if we want to make this configurable, let's make it global and not per path segment

The separator is rather per resolve than per path segment. So it can be specified by the annotation (Configuration#separator) or overridden by the programmatic API, i.e. loader.separator(String).

so it is clear which separator is being used per application and not per config object.

I must be missing something. Only the annotation on the configuration interface needs to be observed for the separator/path (as well as the path), or (if overridden) the very programmatic API call is observed for the separator/path using the Loader object. What do you mean by unclear?

If the programmatic call is passing the same values already available in the annotation, then I should have an API that uses those instead and another API that could allow me to override them (if there is any sense in providing such API).

That's exactly what the two PRs provide. First, one can specify the configuration object using the annotation.

@Configuration(path = "another_prefix", separator = "_")
interface AnotherConfig {
}

And then the programmatic API just uses the values from the annotation

Loader.bootstrap().load(AnotherConfig.class)

or it can be overridden by

Loader.bootstrap().separator(".").path("another.prefix").load(AnotherConfiguration.class)

@radcortez Do we say the same thing?

The configuration interface does not need to be annotated by @Configuration so we need the path (and the separator) on the Loader anyway.

[jansupol]

I have force-pushed the changes reflecting comments to keep the 2 commits of different committers that are not to be merged.

[radcortez]

I must be missing something. Only the annotation on the configuration interface needs to be observed for the separator/path (as well as the path), or (if overridden) the very programmatic API call is observed for the separator/path using the Loader object. What do you mean by unclear?

I've got it that this is per config object, but the unclear portion is that by providing such customization, you may end up with a very confusing setting. Consider:

one.object.value=1234
second_object_value=1234

Looking into this definition, is not clear which separator to use (I have to search each object load to learn which one to use). It is ok for implementations to provide such feature, but it shouldn't be included in a specification. Also, if there is a complete split between sources and canonical representations of a path, there is no need to allow for custom separators.

The configuration interface does not need to be annotated by @Configuration so we need the path (and the separator) on the Loader anyway.

Depends. One thing I haven't seen specified or discussed yet is if we will have a way to pre-discover the configuration interface candidates. The annotation could be used to mark such interfaces as being Config aware, or have some manual way to register them (maybe via ServiceLoader). This would allow to pre-validate the Config interface rules and fail the application start instead of throwing a runtime exception mid-flight when calling the loader (right now, I'm assuming that everything happens on the fly, since I haven't seen anything that indicates otherwise. Please correct me if I'm wrong).

[Emily-Jiang]

In this first try, can we just stay with "." as the separator and then add a separator later if needed?

[jansupol]

I've got it that this is per config object, but the unclear portion is that by providing such customization, you may end up with a very confusing setting. Consider:

one.object.value=1234
second_object_value=1234

Looking into this definition, is not clear which separator to use (I have to search each object load to learn which one to use). It is ok for implementations to provide such feature, but it shouldn't be included in a specification. Also, if there is a complete split between sources and canonical representations of a path, there is no need to allow for custom separators.

---

First of all, yes,

one.object.value=1234
second_object_value=1234

has no clear separator, completely agree. But this is a source definition, which we have not discussed yet and I do not think it would be wise to support multiple separators within a single source.

Second, yes, there would be a complete split between source keys and path. Both can have different separators. This way, the config Spec defines just the default separator, but the user use-case can be different and may want to use another separator. Examples of the sources can be the hard drive folder as a source with an OS-specific path separator, or a CSV file with a semicolon separator. The same would be for the config paths. The customers can use a separator from their domain, and make queries using it directly - similarly to passing SQL commands via Java interface, the customers can pass config queries directly to a Loader through some interface using a separator they are used to, for instance, the space character:

Loader.bootstrap().separator(" ").path("George Washington").load(Person.class);

---

Depends. One thing I haven't seen specified or discussed yet is if we will have a way to pre-discover the configuration interface candidates. The annotation could be used to mark such interfaces as being Config aware, or have some manual way to register them (maybe via ServiceLoader). This would allow to pre-validate the Config interface rules and fail the application start instead of throwing a runtime exception mid-flight when calling the loader (right now, I'm assuming that everything happens on the fly, since I haven't seen anything that indicates otherwise. Please correct me if I'm wrong).

The discussion so far was to have the main Loader mechanism, and the @Configuration annotation was added later, mainly for the cdi-enabled environments. So the implementation can have pre-discovery using the @Configuration annotations if it wanted, but it should not deny annotation-less dynamic resolves of interfaces. That's my understanding of what has been discussed so far.

What we also have not discussed yet is the mutability of sources. The mutability (if any in the future) would change the possibility of pre-discovery.

Also, with pre-discovery, there are some questions, such as whether or not to validate the interfaces that are marked with the @Configuration interface and are not used within the application. So the "pre-discovery" or "on-the-fly" options are up to the implementations at the moment, I believe.

[OndroMih]

Hi folks, I like all your ideas but I’d like to keep things simple so that we can deliver the Config spec 1.0 rather sooner than later.

My view:

Ad. separators: I understand that config sources and API can use different separators. I see the value of different separators on config source level (CSV files use , or |, property files use ., OS paths use / or . But I don’t think there’s a real usecase for having different separators other than . in the API, at least not 1.0 version. Keep things simple please, we can add this easily later, without breaking anything.

Ad. Path in configuration interface. It should be possible to specify the prefix in programmatic API, where the annotation isn't even needed. If not specified, it makes sense to use the one on the configuration as the default prefix. When injecting the interface, it should also be possible to specify the prefix, if not specified, the one on the annotation should be used by default. EDIT: The annotation is only needed for injection so that the implementation can create a producer that creates injectable instances.

[jansupol]

@OndroMih Thank you for chiming in. We would be happy to see you in our weekly meetings every Thursday to discuss the API.

I have created another PR without the separator, though I am not in favor of it.

[jansupol]

Closing in favor of #190