Add a common shared contract for the sessions

[DavideD]

See quarkusio/quarkus#46091 (comment)

[gavinking]

@FroMage do you really need this. I'm not very keen on it.

[DavideD]

I'm not sure about the extra .find methods of the original request, but what's the problem with having the Query creation methods in a separate interface, similar if not equal to QueryProducer? Isn't ORM already doing something similar?

[FroMage]

@FroMage do you really need this. I'm not very keen on it.

I don't really need it, I've abstracted around it using inheritance and subclasses to deal with the two session types, but they're calling the same damn methods with the same name and signature. And ORM has SharedSessionContract where these live.

Looks to me like this is an improvement worth having, and is in line with the ORM sessions types.

Not absolutely necessary, but definitely easier to deal with mixed session types, for 5-6 methods.

[gavinking]

The reason I'm hesitant is that these things are inner types and it's already a PITA to navigate around inside the Mutiny and Stage outer interfaces.

I mean, it's not a strong objection.

Also:

1. we would have to find a name for this thing because SharedSessionContract is a mouthful, and
2. in your proposal you have given this thing a find() method, but StatelessSession does not have find().

Perhaps it's an analog of QueryProducer you're looking for.

[FroMage]

in your proposal you have given this thing a find() method, but StatelessSession does not have find().

Yeah, I suppose this one has different names, we can leave it out.

[FroMage]

I'll need a few more operations, hold on.

[gavinking]

Yeah, I suppose this one has different names

Well, not just different names; different semantics.

[FroMage]

potato, tomato 😅

[FroMage]

"get me this thing by id"

[gavinking]

"Get me a managed object" and "get me a detached object" are pretty different.

[FroMage]

I've heard you say that, and yet the query operations are the same to get one or the other type :)

[FroMage]

Two more methods needed:

	Filter enableFilter(String filterName);
	void disableFilter(String filterName);

[gavinking]

Haha you got me there :)

[FroMage]

I know, and that's going to be hard to defend 🤣

remove vs delete I can sort of believe the argument because there's the side-effect of removing the entity from the set of managed/tracked entities. Even though I'd argue that we know what session we're talking to, we also know that in both cases the underlying operation is "delete", and the stop tracking side-effect is specific to the session type, and I don't think it's justified to babysit me into realising that there's more to "delete" than just the SQL deletion for managed sessions.

Assuming I know how managed sessions work (and with ORM, I need to understand the difference between managed/auto-update and manual state tracking) then the fact that the entity is removed from the tracking list is just one of many side-effects that deleting an entity has. There's more side-effects, but those are not explicitly called out by the method name. Why this one is singled out, I'm not sure. I suppose it's historical. But still I feel the difference between delete and remove are superfluous and get in the way of remembering the APIs and muscle memory.

Perhaps that's just me, though.

[FroMage]

Just give me my shared session interface or I will keep arguing into renaming more things 🤣

[gavinking]

Well you have to look at the bigger picture. The whole model if explicit CRUD is very different to to the model with stateful sessions, and we've seen lots of problems over the years when we let users muddle those two things up in their heads.

remove vs delete I can sort of believe the argument because there's the side-effect of removing the entity from the set of managed/tracked entities.

But that's not even the most important difference. The most important differences are:

1. one is immediate/synchronous, the other is asynchronous and only happens if the session gets flush()ed
2. one accepts a detached object, the other requires a managed object

But still I feel the difference between delete and remove are superfluous and get in the way of remembering the APIs and muscle memory.

OK, now do update() or even better upsert() vs merge().

[gavinking]

What I'm saying is you're making a mistake by focussing on one method. These methods combine to form a whole protocol:

• get, fetch, insert, update, upsert, delete, refresh
• find, persist, remove, merge, refresh, lock, detach

And these protocols are really different in nature.

Now, sure, it's true that they used to be more different before, when we didn't have second-level caching and collections in stateless sessions, and it's true that we reuse the query APIs between the two flavors of session (only because we don't want to copy/paste the whole damn thing), but it's very important to keep a clear distinction between them.

[DavideD]

we would have to find a name for this thing because SharedSessionContract is a mouthful, and

Is this your only issue with the name? Because, it doesn't sound too terrible to me, and it's the same name used in ORM (and I think it helps to have similar names between the two). For queries there is CommonQueryContract, should we used something similar? CommonSessionContract. Anyway, I'm open to suggestions.

[DavideD]

ReactiveSession, HibernateSession?

[gavinking]

it doesn't sound too terrible to me

I've no idea what "Shared" means here. Shared between who? Certainly not between clients. Between its subtypes?? Ugh. And "Contract" is extremely redundant since every public type is a contract.

If you didn't already know what a SharedSessionContract was, would you have any chance of guessing from the name? I doubt it.

Now, in my proposal for JPA4, the analogous thing is called EntityHandler, with subtypes EntityManager and EntityAgent. I don't think EntityHandler is a great name, and I don't think it works for us here, but that name is not set in stone, and frankly I would have maybe preferred to wait and see what ultimately happens with the spec discussions before pulling the trigger on this.

But if you absolutely have to do this right now, AbstractSession seems like the most obvious name, since it's an abstraction over the different kinds of sessions.

[DavideD]

We can wait I guess

[gavinking]

We can wait I guess

I guess we should ask @FroMage if he is in a hurry.

[FroMage]

No hurry, I'm using a complicated setup of class inheritance to work around that.