HHH-19849 Add an SPI that allows attaching session-scoped "extensions" to the session/statelesssession implementors

[marko-bekhta]

https://hibernate.atlassian.net/browse/HHH-19849

so that Search wouldn't need to rely on .getProperties() and abuse it 😄.

I'm not "attached" 🙂 to the method names and if there are any suggestions for better alternatives, happy to change them 🙂

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion. For more information on licensing, please check here.

---

[sebersole]

Perhaps this is over-thinking, but what do you think about something like this instead:

/** For now, just a marker */
interface Extension {
}

interface ExtensionStorage {
    ...
}

interface SharedSessionContractImplementor {
    ...
    ExtensionStorage getExtensionStorage(Class<? extends Extension> extension);
}

[yrodiere]

Perhaps this is over-thinking, but what do you think about something like this instead

It looks good, but on the other hand I'm not sure ExtensionStorage would provide any useful feature to Hibernate Search. Hibernate Search really only needs to bind its SearchSession to the Session.

Perhaps you were thinking of something specific Envers could leverage?

[gavinking]

Let's avoid stringly-typed extensionNames. I think that's what Steve is suggesting.

[marko-bekhta]

Perhaps this is over-thinking, but what do you think about something like this instead:

/** For now, just a marker */
interface Extension {
}

interface ExtensionStorage {
    ...
}

interface SharedSessionContractImplementor {
    ...
    ExtensionStorage getExtensionStorage(Class<? extends Extension> extension);
}

Thanks, that might work 🙂. Just made an adjustment to the pr 🙂

In Search, we have a session and a session holder, we connect the holder to the ORM's session. With that in mind, I could see how we'd make the session holder an ExtensionStorage. I couldn't think of how we'd use Extension on the Search side in this case, so I didn't add it here.

[sebersole]

Perhaps this is over-thinking, but what do you think about something like this instead

It looks good, but on the other hand I'm not sure ExtensionStorage would provide any useful feature to Hibernate Search. Hibernate Search really only needs to bind its SearchSession to the Session.

Perhaps you were thinking of something specific Envers could leverage?

Here specifically I was thinking about avoiding String keys for a few reasons.

But I was thinking also that we've spoken a few times about "true extensions" (for lack of a better phrase) for other projects to expose via Session (Envers reader e.g.). So partially I was thinking about that. So maybe even something like this:

interface SharedSessionContractImplementor {
    ...
    <E extends Extension> E getExtensionStorage(Class<E> extensionType);
}

which would rely on projects registering this mechanism with ORM. E.g. for Search:

class SearchExtension implements Extension {
    // access to storage
}

SearchExtension ext = session.getExtension( SearchExtension.class );
ext.storeSomething( ... );

For envers:

class EnversExtension implements Extension {
    AuditReader getAssociatedAuditReader();
    ...
}
...

Just brainstorming Marko, no need to make changes yet :)

I know this is well beyond the scope of the issue here, but I think its worth considering.

[yrodiere]

Just brainstorming Marko, no need to make changes yet :)

I think Marko already did pretty much what you're suggesting here, give or take a few names :sweat_smile:

[sebersole]

Yes and no. I'm actually I am thinking something akin to Gradle's notion of extensions.

interface Extension {
}

/** Registered with SessionFactory */
interface ExtensionIntegration<E extends Extension> {
    Class<E> getExtensionType();
    E createExtension(SharedSessionContractImplementor session, ???);
}

interface SharedSessionContractImplementor {
    ...

    <E extends Extension> E getExtension(Class<E> extensionType);
}

[yrodiere]

We can go there if you think that's necessary, but we'll need to be careful about the exact API, because some usage patterns in Hibernate Search involve checking if the extension (search session) is there, and only using it if it already exists -- avoiding to create it if it doesn't exist yet, because that's pointless and costly in those cases.

In other words, we'd need this:

interface Extension {
}

/** Registered with SessionFactory */
interface ExtensionIntegration<E extends Extension> {
    Class<E> getExtensionType();
    E createExtension(SharedSessionContractImplementor session, ???);
}

interface SharedSessionContractImplementor {
    ...

    <E extends Extension> E getExtension(Class<E> extensionType);

    @Nullable
    <E extends Extension> E getExtensionIfExists(Class<E> extensionType);
}

In any case... I think Marko wanted this in 7.2 in order to avoid having to deploy workarounds for other problems. @sebersole let us know if you're sure the above is what you want? @marko-bekhta I'll let you determine if you still have time to get this into 7.2 or if you'll just go with workarounds for now (which seems more reasonable TBH).

[sebersole]

As long as its all incubating, anything is fine - we can always change it. Just trying to get in front of it so any changes needed later in Search, etc could be minimal.

I should also mention, @dreab8 and I were discussing some HR cases where this might be useful. Rule of 3 :)

@marko-bekhta , if you like this approach and don't have time let me know and I can work on it

[marko-bekhta]

@marko-bekhta , if you like this approach and don't have time let me know and I can work on it

Let me play with it a bit, to see how it would be better to integrate it within Search and I'll get back to you 😃