Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config mapping for java.util.Maps that also have an "unnamed" entry #32295

Closed
yrodiere opened this issue Mar 31, 2023 · 6 comments · Fixed by #33079
Closed

Config mapping for java.util.Maps that also have an "unnamed" entry #32295

yrodiere opened this issue Mar 31, 2023 · 6 comments · Fixed by #33079
Assignees
Labels
area/config kind/enhancement New feature or request
Milestone

Comments

@yrodiere
Copy link
Member

Description

In several extensions, we need to provide named configuration sections, while still allowing a "default", unnamed section with the same prefix.

E.g. for Hibernate Search/ORM we have a concept of persistence units, configured with the prefix quarkus.hibernate-search-orm.

There's a default persistence unit, configured like this:

quarkus.hibernate-search-orm.elasticsearch.version=7

And there are (optional) named persistence units, configured like this:

quarkus.hibernate-search-orm."user".elasticsearch.version=7
quarkus.hibernate-search-orm."inventory".elasticsearch.version=7

We usually end up implementing this as two separate methods, plus another one to aggregate the whole thing by hand:

    @WithParentName
    HibernateSearchElasticsearchBuildTimeConfigPersistenceUnit defaultPersistenceUnit();
    @WithParentName
    Map<String, HibernateSearchElasticsearchBuildTimeConfigPersistenceUnit> persistenceUnits();

    default Map<String, HibernateSearchElasticsearchBuildTimeConfigPersistenceUnit> getAllPersistenceUnitConfigsAsMap() {
        Map<String, HibernateSearchElasticsearchBuildTimeConfigPersistenceUnit> map = new TreeMap<>();
        HibernateSearchElasticsearchBuildTimeConfigPersistenceUnit defaultPersistenceUnit = defaultPersistenceUnit();

        if (defaultPersistenceUnit != null) {
            map.put(PersistenceUnitUtil.DEFAULT_PERSISTENCE_UNIT_NAME, defaultPersistenceUnit);
        }
        map.putAll(persistenceUnits());
        return map;
    }

And then in extension code we do something like:

// When we need the config of a specific persistence unit (generally the default one):
var defaultConfig = config.getAllPersistenceUnitConfigsAsMap().get(PersistenceUnitUtil.DEFAULT_PERSISTENCE_UNIT_NAME);

// When we just need to check whether a PU is configured (generally the default one):
config.getAllPersistenceUnitConfigsAsMap().containsKey(PersistenceUnitUtil.DEFAULT_PERSISTENCE_UNIT_NAME);

// When we need to iterate over every configured persistence unit:
for (var entry : config.getAllPersistenceUnitConfigsAsMap().entrySet()) {
   // ...
}

Since that's a rather frequent use case, maybe we could introduce a shortcut that would directly put the "root"/default entry in the map, with a provided key?

It would be nice to introduce this before we migrate all extensions to @ConfigMapping, so we can simplify extension config while we migrate.

See also https://github.com/quarkusio/quarkus/pull/32276/files#r1154222403

Implementation ideas

E.g.:

    @WithParentName
    @WithRootEntry(key = PersistenceUnitUtil.DEFAULT_PERSISTENCE_UNIT_NAME)
    Map<String, HibernateSearchElasticsearchBuildTimeConfigPersistenceUnit> persistenceUnits();

@WithRootEntry here would mean "with the same prefix, we also allow configuring a map entry without a name, with the same structure". And key = ... would mean "I want the unnamed map entry to be added to the map with this key, which I don't expect to conflict with named entries".

@radcortez
Copy link
Member

Support in SmallRye Config:
smallrye/smallrye-config#927

@yrodiere would you like to take a look? One difference is I named the annotation @WithUnnamedKey. Not sure if I like the name, but we refer to everything in a Map with a key as a named configuration, so unnamed seems fitting.

@yrodiere
Copy link
Member Author

@radcortez Sure the name looks fine. I can try to use it in Quarkus if you want, would it work if I just built locally from your PR and bumped the version in Quarkus? Or are there also breaking changes that will prevent that from working?

@radcortez
Copy link
Member

Yes, it should work without any additional changes.

@yrodiere
Copy link
Member Author

Thanks. I'll report back on Monday.

@yrodiere
Copy link
Member Author

@radcortez Looks good, thanks a lot! We'll need adjustment to doc generation in Quarkus, but that can wait; see #32854 for details.

@radcortez
Copy link
Member

Great! Good to hear. Yes, we need to figure out what to do with documentation.

Also, I still need to add the ability to retrieve / create a default entry.

@radcortez radcortez self-assigned this May 24, 2023
@quarkus-bot quarkus-bot bot added this to the 3.2 - main milestone Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants