6.0 - IdentifierGeneratorFactory

[sebersole]

This discussion groups together all of the little discussions we've been having around id-generation the last week or so. The background comes from

• hibernate_sequence considered harmful #3809
• discussion on Zulip
• series of Jira issues I created for the individual parts
  • https://hibernate.atlassian.net/browse/HHH-14491
  • https://hibernate.atlassian.net/browse/HHH-14492
  • https://hibernate.atlassian.net/browse/HHH-14495
  • https://hibernate.atlassian.net/browse/HHH-14496
  • https://hibernate.atlassian.net/browse/HHH-14497

Much of the processing done as part of IdentifierGeneratorFactory is still highly slanted to hbm.xml. When annotation support was added, this code was not re-designed properly / at-all. Additional code added to accommodate OGM makes the process to resolve the IdentifierGenerator even more fulgy.

Here is the new series of contracts I am proposing and I'll discuss how it addresses the various parts of the discussion...

/**
* The idea is to be able to resolve the specific generator details.  It also is designed with "scoping" in mind
* in terms of JPA compliance for global scoping of the names versus Hibernate's better strategy.  This 
* contract can hide that detail and can encapsulate the hierarchical structure for local scoping
*/
@FunctionalInterface
interface GeneratorConfigResolver {
    /**
     * @param generationType Pass `null` to achieve legacy hbm handling if we still wish that to be different
     */
    GeneratorConfig resolveGeneratorDefinition(GenerationType generationType, String generatorName);
}

/**
 * Something that can have an identifier associated with it.  Practically speaking,
 * this will be an entity(-hierarchy) or an id-bag.
 */
interface IdentifiableModelPart extends ModelPart {
    /**
     * Used when determining the implicit name of a generator sequence/table,
     * combined with any defined suffix
     */
    String getImplicitDatabaseStructureNameBase();
}

interface UsageSiteDetails {
    enum Category { ENTITY, ID_BAG }

    Category getCategory();
    String getRole();

    // JTD?
    // Maybe just pass `IdentifiableModelPart`?  (that depends on timing)
    // ???
}

/**
 * Same role as our current contract, with an altered signature
 */
@FunctionalInterface
interface IdentifierGeneratorFactory {
    IdentifierGenerator createIdentifierGenerator(
            GenerationType generationType,
            String generatorName,
            JavaTypeDescriptor<?> javaTypeDescriptor,
            UsageSiteDetails usageSiteDetails,
            Properties config,
            GeneratorConfigResolver configResolver);
}

/**
 * Delegate for defining how to handle the various types of
 * {@link GenerationType} possibilities, including no GenerationType (hbm.xml)
 */
interface GenerationTypeStrategy {
    IdentifierGenerator createIdentifierGenerator(
            GenerationType generationType,
            String generatorName,
            JavaTypeDescriptor<?> javaTypeDescriptor,
            Properties config,
            GeneratorConfigResolver configResolver);
}

/**
 * Same role as our current contract, with an altered signature
 */
interface IdentifierGenerator {
    ...

    void configure(
            JavaTypeDescriptor<?> javaTypeDescriptor,
            GeneratorConfigResolver configResolver,
            ???,
            ServiceRegistry serviceRegistry);
}

/**
 * {@link java.util.ServiceLoader} contract for registering GenerationTypeStrategy impls.
 * 
 * This is a direct replacement for the `IdGeneratorStrategyInterpreter` contract originally added for OGM
 */
interface GenerationTypeStrategyRegistration {
    void registerStrategies(
        Function<GenerationType,GenerationTypeStrategy> currentRegistrationAccess,
        BiConsumer<GenerationType, GenerationTypeStrategy> registrar,
        ServiceRegistry serviceRegistry);
}

IdentifierGeneratorFactory is still the central contract. Internally it would keep a registry of GenerationTypeStrategy delegates by GenerationType.

GenerationTypeStrategyRegistration services can override a registered strategy. currentRegistrationAccess allows for access to the previous registration for composition if desired. registrar is used to register the new strategy.

GeneratorDefinitionResolver allows accessing the generator configuration information, if any, based on name. This would perform a hierarchical, scoped lookup similar to what we do today (local, global).

I think another thing we should consider is whether we want to continue with the Properties-based config of generators. Or whether we want to start having a more structured configuration contract. E.g.

interface GeneratorConfig {
    Map<?,?> getSettings();
}

interface RelationalGeneratorConfig extends GeneratorConfig {
    QualifiedName getStructureName();
    int getInitialValue();
    int getAllocationSize();
    Optimizer getOptimizer();
}

interface SequenceGeneratorConfig extends RelationalGeneratorConfig {
    boolean forceTableUse();
}

interface TableGeneratorConfig extends RelationalGeneratorConfig {
    String getSegmentColumn();
    String getSegmentValue();
    int getSegmentColumnLength();
    String getValueColumn();
}

[gavinking]

@sebersole Have you considered if there's some reasonable way we could unify at least some of this code with the contracts we had to introduce in Reactive?

Basically we have our whole own parallel API here:

https://github.com/hibernate/hibernate-reactive/tree/main/hibernate-reactive-core/src/main/java/org/hibernate/reactive/id

Now, at least some of that is unavoidable, but I wonder if there's a way that core could accommodate the needs of HR at least a little more elegantly?

[gavinking]

The stuff I really hate is in ReactiveIdentifierGeneratorFactory.augmentWithReactiveGenerator() where we have to do a bunch of instanceofs on identifier generator implementation classes to convert the to our reactive versions. I'm not sure how to fix it though.

Also there's some code in IdentifierGeneration which is essentially copy/pasted from core. Stuff like determineSequenceName().

Can HR re-use stuff like DatabaseStructure?

errm I think so. HR can't uses stuff that depends on JDBC metadata (for now, but we're fixing that).

[sebersole]

Can HR re-use stuff like DatabaseStructure?

errm I think so. HR can't uses stuff that depends on JDBC metadata (for now, but we're fixing that).

Sure. For the most part is just a "struct" of data. It does rely on Session, Dialect, Optimizer and AccessCallback though. It tries to avoid hard JDBC deps. Not sure if I suceeded though, which is why I ask ;)

[sebersole]

The stuff I really hate is in ReactiveIdentifierGeneratorFactory.augmentWithReactiveGenerator() where we have to do a bunch of instanceofs on identifier generator implementation classes to convert the to our reactive versions. I'm not sure how to fix it though.

Also there's some code in IdentifierGeneration which is essentially copy/pasted from core. Stuff like determineSequenceName().

Sure ORM has this (in a different way) too. It was the intention for GeneratorConfig and GeneratorDefinitionResolver (which is probably better modeled as GeneratorConfigResolver)

[gavinking]

I do seem to sort-of use DatabaseStructure in augmentWithReactiveGenerator(). At least I do an instanceof on it. So probably safe.

[gavinking]

Sure ORM has this (in a different way) too. It was the intention for GeneratorConfig and GeneratorDefinitionResolver (which is probably better modeled as GeneratorConfigResolver)

So if that gives us a way to cleanly get access to the information used to build the generator, instead of having to come along later and do instanceofs, it would help, I think.

Though it also depends a bit on the lifecycle of when is that info available. (i.e. is it available inside the EntityPersister.)

[gavinking]

I believe all this was done in 6.0, right @sebersole ?

[sebersole]

No. I added many of the contracts but they are not really hooked in yet

[gavinking]

ah ok