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

initial proposal for a stateless EntityAgent #675

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gavinking
Copy link
Contributor

@gavinking gavinking commented Oct 18, 2024

This is just a rough draft to incite snarky comments, personal insults, and other feedback. :-)

The pull request introduces:

  • EntityAgent, a stateless entity manager, as proposed in stateless entity manager #374,
  • EntityHandler as place for operations common to EntityAgent and EntityManager, and
  • new callbacks, as described here.

I have not even started working on the specification side of this.

@beikov
Copy link

beikov commented Oct 19, 2024

How about naming it EntityRepository, EntityAccessor, EntityService or EntityStore?

@beikov
Copy link

beikov commented Oct 19, 2024

Or EntityController, EntityRegistry, EntityHub?

@gavinking
Copy link
Contributor Author

gavinking commented Oct 19, 2024

The name is just a suggestion, but I chose it based on the following reasoning:

  1. StatelessEntityManager is (a) verbose, (b) ambiguous (is it a manager for stateless entities?), and (c) potentially a little misleading because it's not necessarily a truly stateless object.
  2. I was looking for something that is similar to "manager", but which implied a more hands-off relationship with the entities. "Agent" is a good word for this.
  3. I didn't like options which implied coordination/orchestration, since that's not its role (it's the application program which does orchestration).
  4. Other options were things like "dispatcher" or "executor", and they almost work as a description of its role in insert/update/delete. But they're not a great description of an object which runs queries and returns data.
  5. EntitySupervisor doesn't work because "supervisor" is a synonym for "manager", and this thing doesn't actually "manage" the entities. None of its entities are in the managed state.

EntityRepository

This would collide with terminology already adopted in Jakarta Data.

EntityService

It's not a service, because it's not a singleton. Well, at least, that's not how StatelessSession works in Hibernate, but admittedly this is something which still needs to be thought through and nailed down. In principle it maybe could be a singleton per persistence unit, but that would mean it couldn't have any operations for setting properties, cache mode, etc. Anyway, in the attached proposal it's not a service.

EntityStore

That sounds like a better name for the EntityManagerFactory, but doesn't sound like a good name for this thing.

EntityController

This is more along the lines of what I'm looking for, but I think EntityAgent is better. In particular, I'm trying to emphasize that it actually exercises less "control" than the EntityManager.

EntityRegistry

I don't like this suggestion at all. The whole point of a stateless session is that it doesn't maintain any sort of registry of entities. So this has almost the precise opposite of the word sense I'm looking for.

EntityHub

Not sure why it's a hub. What are the spokes here? The entities? Again, the point of this object is that it doesn't maintain its relationship with the entities in a stateful way.

@rbygrave
Copy link

rbygrave commented Oct 20, 2024

Q: Should <T> T getReference(Class<T>, Object id) be here? Maybe I missed it? The thought is that to perform insert() we need to build an entity bean and want to use getReference() for the ManyToOne properties when we do that.

Syntactic thought: get() could be namedfindOne() and getMultiple() could be named findList()

Syntactic thought: The Java Collections API uses addAll(), removeAll(). The thought is that the "Multiple" part could be "All" to be closer to the collections API. For myself, I don't see a lot of API use "Multiple" per se so that sticks out a little bit to me.

Currently, the get() doesn't say that it throws EntityNotFoundException [yet]. The 3 options I believe are: (1) throw EntityNotFoundException, (2) Return Optional<T> or (3) Mark the response as JSpecify @Nullable. I'm thinking this is going to tend towards option 1 for more consistency with EntityManager?

@gavinking
Copy link
Contributor Author

Should <T> T getReference(Class<T>, Object id) be here?

I was about to answer "no", but then I realized it does make sense in combination with fetch() and for setting up associations. And so, yeah, I guess or right, it should be there. Nice catch.

get() could be named findOne() and getMultiple() could be named findList()

I want to be very explicit about the huge semantic difference between EntityManager.find() and EntityAgent.get() by giving them different names.

As to getAll() vs getList() vs getMultiple(), the thinking here is that getAll() sounds like it gets all entities of a given type. With respect to List vs Multiple I don't have a strong opinion. @sebersole and I weakly prefer Multiple but I doubt either of us are very motivated to argue about it.

@rbygrave
Copy link

getAll()

Yes, I would not propose that. It was more findList() vs getMultiple() ... but if the find part isn't really an option then "All" probably doesn't work because getAll would not be great.

very explicit about the huge semantic difference between EntityManager.find() and EntityAgent.get() by giving them different names.

Makes sense. The other option is along the lines of using something more like StatelessSession vs EntityAgent but I get it, naming is hard here when we have both concepts in close proximity. Hmmm.

@quaff
Copy link

quaff commented Oct 21, 2024

Should insert(Object entity) be void to align with insertMultiple(List<Object> entities)? since the id is filled if the operation is successful.

@gavinking
Copy link
Contributor Author

@quaff I don't care much. I agree that returning the id is not necessary, but sometimes it might be convenient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants