insert and update methods

[njr-11]

Add insert and update methods with an entity parameter.
This is split out separately from #266 , which covered save and delete with an entity parameter.

[otaviojava]

Hey everyone, I understand there was a recent vote to do a git revert and implicitly include the insert and update methods in the Jakarta Data spec as dynamic methods. However, I'd like to encourage a thorough reconsideration of this decision for the following reasons:

1. Preserving Abstraction: The cornerstone of the Repository pattern is Abstraction, and it has historically encapsulated insert and update operations within the save method, as per our documentation. This design choice simplifies the API and keeps the core concept intact.

2. Dynamic Provider Extension: One of the strengths of the Jakarta Spec is its flexibility. By allowing providers to include these methods dynamically, we maintain the optionality of this feature. It aligns well with our spec's philosophy, where customization and adaptation are encouraged rather than enforced.

3. Consistency and Spec Clarity: If we proceed with inserting explicit insert and update methods, aligning our spec with this decision is crucial. Thus, I recommend renaming the Repository as a Data Access Object (DAO). Or include these methods as DAO capability in the next version.

In summary, my primary concern is that by introducing explicit insert and update methods, we might inadvertently dilute the essence of the Repository pattern. An analogy that comes to mind is creating an annotation for Singleton and then adding a feature to allow multiple instances of a Singleton. It could lead to confusion and undermine the core concept. Therefore, we should carefully consider the implications of this change and whether it truly aligns with the goals of the Jakarta Data spec.

P.S.: I am not saying the insert and update are unnecessary; just as with any pattern, it brings trade-offs.

[otaviojava]

Even inserting and updating several NoSQL databases does not necessarily mean it will work as a relational database.
It might go to several databases that will work in the append model as some wide-column and key-value databases, for example, Apache Cassandra and som key-values databases.

[njr-11]

It might go to several databases that will work in the append model as some wide-column and key-value databases, for example, Apache Cassandra and som key-values databases.

Thanks for pointing this out. Those types of databases don't offer the insert capability that guarantees you caused the entity to be created and so should raise UnsupportedOperationException when the insert operation is attempted. I added commit 94c74b8 to explicitly state this in the spec and JavaDoc.

[njr-11]

1. it has historically encapsulated insert and update operations within the save method, as per our documentation. This design choice simplifies the API and keeps the core concept intact.

The save method provides a different functionality to users than insert and update. With insert, a user can guarantee that an entity only gets created with its initial values once, and that if another attempt to do so is made, that it will not wipe out the state of the entity back to its original state like a save would. With update, a user can guarantee that the entity they are trying to modify actually exists and find out if it didn't. This is unlike save, which is incapable of doing so because it adds the entity if it wasn't there. The insert and update operations also offer the potential for better performance by making less trips to the database than save.

If we are only interested at simplifying the API at the expense of functionality and performance, then we should also be asking why CrudRepository has methods like count() and existsById, which you could instead get with findAll().count() and findById(id).isPresent(). You could get rid of those methods and even get the same behavior (and worse performance) in the name of simplifying the API. But that doesn't mean we should avoid having count() and existsById, and neither does it mean we should avoid having insert and update.

2. One of the strengths of the Jakarta Spec is its flexibility. By allowing providers to include these methods dynamically, we maintain the optionality of this feature. It aligns well with our spec's philosophy, where customization and adaptation are encouraged rather than enforced.

Don't confuse flexibility with optionality. By offering insert and update on the built-in repository we offer the greatest flexibility to users to choose what best suits the needs of their business logic. You might be thinking of flexibility for providers to not implement stuff. We should place the needs and desires of users ahead of that of providers.

3. If we proceed with inserting explicit insert and update methods, aligning our spec with this decision is crucial. Thus, I recommend renaming the Repository as a Data Access Object (DAO).

You're recommending to rename CrudRepository (CRUD = Create, Read, Update, Delete) to something else because we added insert (which is create) and update methods to it. It would make more sense to rename it to something else if we don't include methods for create and update. That said, I don't have any particular attachment to the CrudRepository name and am fine if we rename it something else.

[otaviojava]

1. it has historically encapsulated insert and update operations within the save method, as per our documentation. This design choice simplifies the API and keeps the core concept intact.

The save method provides a different functionality to users than insert and update. With insert, a user can guarantee that an entity only gets created with its initial values once, and that if another attempt to do so is made, that it will not wipe out the state of the entity back to its original state like a save would. With update, a user can guarantee that the entity they are trying to modify actually exists and find out if it didn't. This is unlike save, which is incapable of doing so because it adds the entity if it wasn't there. The insert and update operations also offer the potential for better performance by making less trips to the database than save.

As I said, I am not against the insert and update method, I am saying that it is not a repository anymore, because we are defining directly to the database. The spec is not forbidden, thus, the provider is free to include those as functions as extra features.

If we are only interested at simplifying the API at the expense of functionality and performance, then we should also be asking why CrudRepository has methods like count() and existsById, which you could instead get with findAll().count() and findById(id).isPresent(). You could get rid of those methods and even get the same behavior (and worse performance) in the name of simplifying the API. But that doesn't mean we should avoid having count() and existsById, and neither does it mean we should avoid having insert and update.

We are asking for a domain, we are not exploring direct command, a simple sample might be the Collection of Java. It provides count, returns information, and adds, and we do not know where the information came from, that is the beauty of abstraction. The huge point would be to read the book.

2. One of the strengths of the Jakarta Spec is its flexibility. By allowing providers to include these methods dynamically, we maintain the optionality of this feature. It aligns well with our spec's philosophy, where customization and adaptation are encouraged rather than enforced.

Don't confuse flexibility with optionality. By offering insert and update on the built-in repository we offer the greatest flexibility to users to choose what best suits the needs of their business logic. You might be thinking of flexibility for providers to not implement stuff. We should place the needs and desires of users ahead of that of providers.

I am not against insert and update, as I said with a singleton with multiple instances. We can use multiple instances, but it is not a singleton pattern anymore.

3. If we proceed with inserting explicit insert and update methods, aligning our spec with this decision is crucial. Thus, I recommend renaming the Repository as a Data Access Object (DAO).

You're recommending to rename CrudRepository (CRUD = Create, Read, Update, Delete) to something else because we added insert (which is create) and update methods to it. It would make more sense to rename it to something else if we don't include methods for create and update. That said, I don't have any particular attachment to the CrudRepository name and am fine if we rename it something else.

The save method can do both by abstraction, the CRUD is there where there are situations where the provider inserts and other can provider update, if the user can do it directly, they can use the DAO pattern.

IMHO: We could also discuss it more in the next release version as the next pattern to cover.

[njr-11]

I am saying that it is not a repository anymore, because we are defining directly to the database.

That is not true. I haven't defined insert and update in terms of the database. In this pull I defined an operation that must create something anew and another operation that modifies something that already exists. That is not database details. They are very useful patterns to write business logic around.

We are asking for a domain, we are not exploring direct command, a simple sample might be the Collection of Java. It provides count, returns information, and adds, and we do not know where the information came from, that is the beauty of abstraction. The huge point would be to read the book.

Since you bring it up, let's take a look at the Collection.add method. As a user of a Collection, I don't know if invoking add will put duplicate elements into the collection, or replace existing elements with ones that I try to add, or maybe leave existing elements there and ignore what I try to add, or if the operation will be rejected outright. It's documented as, "Ensures that this collection contains the specified element (optional operation)", which is extremely vague. I'm sure it's vague by design because its real purpose is to be a common ancestor of the various collection subtypes that give the user more guarantees against which they can write more meaningful business logic. If you go just one step up to the Set.add method then you start to get more useful guarantees such as "adds the specified element e to this set if the set contains no element e2 such that Objects.equals(e, e2). If this set already contains the element, the call leaves the set unchanged and returns false." And that corresponds to the sort of thing you get from an insert method: only add if it isn't already there. Abstraction is useful when implementation details are abstracted away. Not as inaccurate justification to omit capabilities that a user needs to write their business logic.

The save method can do both by abstraction, the CRUD is there where there are situations where the provider inserts and other can provider update

That is not true. The save cannot do both: it cannot do insert/create and it cannot do update. It can only do a combination update-or-create-if-not-present. That matters because while sometimes a update-or-create-if-not-present is useful, it isn't always what a user's business logic needs. Often it will matter whether you were able to create something if it didn't exist, and that in attempting to do so you don't overwrite data that was already there. save can't do that. It will wipe out the existing data. You might argue that a user should try to find it first, but first confirming it absent with find/exists query doesn't guarantee that someone else didn't add it in the mean time and you still might be overwriting it. Not having an insert burdens the user with complexity they shouldn't need to face. Similar for update. Some business logic will care that the thing it was trying to modify isn't there anymore and doesn't want a new entity put into the database because the fact that the entity isn't in the database is actually important. The save method is certainly a useful tool, but it's often not the right tool for the job. We really need to be offering all of these very important patterns to our users: the insert, the update, and the save.

[njr-11]

I still don't agree in any way with the counterargument that is being made against having insert and update operations in our CrudRepository, but it's clear this won't be going in with Otavio objecting to it and then we won't have the methods in the specification document either, so I have added commit da02bb7 removing these methods from CrudRepository. I hope that at some point we can make a convincing argument that these two CRUD operations belong on the CrudRepository but there is no point in holding up other progress over them.

@otaviojava please review da02bb7 removing insert and update from CrudRepository as you are requiring.

[otaviojava]

1. it has historically encapsulated insert and update operations within the save method, as per our documentation. This design choice simplifies the API and keeps the core concept intact.

The save method provides a different functionality to users than insert and update. With insert, a user can guarantee that an entity only gets created with its initial values once, and that if another attempt to do so is made, that it will not wipe out the state of the entity back to its original state like a save would. With update, a user can guarantee that the entity they are trying to modify actually exists and find out if it didn't. This is unlike save, which is incapable of doing so because it adds the entity if it wasn't there. The insert and update operations also offer the potential for better performance by making less trips to the database than save.

If we are only interested at simplifying the API at the expense of functionality and performance, then we should also be asking why CrudRepository has methods like count() and existsById, which you could instead get with findAll().count() and findById(id).isPresent(). You could get rid of those methods and even get the same behavior (and worse performance) in the name of simplifying the API. But that doesn't mean we should avoid having count() and existsById, and neither does it mean we should avoid having insert and update.

2. One of the strengths of the Jakarta Spec is its flexibility. By allowing providers to include these methods dynamically, we maintain the optionality of this feature. It aligns well with our spec's philosophy, where customization and adaptation are encouraged rather than enforced.

Don't confuse flexibility with optionality. By offering insert and update on the built-in repository we offer the greatest flexibility to users to choose what best suits the needs of their business logic. You might be thinking of flexibility for providers to not implement stuff. We should place the needs and desires of users ahead of that of providers.

3. If we proceed with inserting explicit insert and update methods, aligning our spec with this decision is crucial. Thus, I recommend renaming the Repository as a Data Access Object (DAO).

You're recommending to rename CrudRepository (CRUD = Create, Read, Update, Delete) to something else because we added insert (which is create) and update methods to it. It would make more sense to rename it to something else if we don't include methods for create and update. That said, I don't have any particular attachment to the CrudRepository name and am fine if we rename it something else.

I am saying that it is not a repository anymore, because we are defining directly to the database.

That is not true. I haven't defined insert and update in terms of the database. In this pull I defined an operation that must create something anew and another operation that modifies something that already exists. That is not database details. They are very useful patterns to write business logic around.

We are asking for a domain, we are not exploring direct command, a simple sample might be the Collection of Java. It provides count, returns information, and adds, and we do not know where the information came from, that is the beauty of abstraction. The huge point would be to read the book.

Since you bring it up, let's take a look at the Collection.add method. As a user of a Collection, I don't know if invoking add will put duplicate elements into the collection, or replace existing elements with ones that I try to add, or maybe leave existing elements there and ignore what I try to add, or if the operation will be rejected outright. It's documented as, "Ensures that this collection contains the specified element (optional operation)", which is extremely vague. I'm sure it's vague by design because its real purpose is to be a common ancestor of the various collection subtypes that give the user more guarantees against which they can write more meaningful business logic. If you go just one step up to the Set.add method then you start to get more useful guarantees such as "adds the specified element e to this set if the set contains no element e2 such that Objects.equals(e, e2). If this set already contains the element, the call leaves the set unchanged and returns false." And that corresponds to the sort of thing you get from an insert method: only add if it isn't already there. Abstraction is useful when implementation details are abstracted away. Not as inaccurate justification to omit capabilities that a user needs to write their business logic.

The save method can do both by abstraction, the CRUD is there where there are situations where the provider inserts and other can provider update

That is not true. The save cannot do both: it cannot do insert/create and it cannot do update. It can only do a combination update-or-create-if-not-present. That matters because while sometimes a update-or-create-if-not-present is useful, it isn't always what a user's business logic needs. Often it will matter whether you were able to create something if it didn't exist, and that in attempting to do so you don't overwrite data that was already there. save can't do that. It will wipe out the existing data. You might argue that a user should try to find it first, but first confirming it absent with find/exists query doesn't guarantee that someone else didn't add it in the mean time and you still might be overwriting it. Not having an insert burdens the user with complexity they shouldn't need to face. Similar for update. Some business logic will care that the thing it was trying to modify isn't there anymore and doesn't want a new entity put into the database because the fact that the entity isn't in the database is actually important. The save method is certainly a useful tool, but it's often not the right tool for the job. We really need to be offering all of these very important patterns to our users: the insert, the update, and the save.

Let's take one step back on this discussion.

I am not against insert and update methods; those are very useful!

We both agree that insert and update are super critical for the user!

What we need to define is the bundle around patterns.

What do you have in mind for the API in the future?

Do you see putting those features here breaking into small pieces or having a package for each pattern?

Because we can redefine it as a Facade instead of a Repository as we are doing now.

[otaviojava]

I still don't agree in any way with the counterargument that is being made against having insert and update operations in our CrudRepository, but it's clear this won't be going in with Otavio objecting to it and then we won't have the methods in the specification document either, so I have added commit da02bb7 removing these methods from CrudRepository. I hope that at some point we can make a convincing argument that these two CRUD operations belong on the CrudRepository but there is no point in holding up other progress over them.

@otaviojava please review da02bb7 removing insert and update from CrudRepository as you are requiring.

Please, don't get me wrong, let's discuss more about it.

It is more related to the API future than the insert and update method; as I said, those are crucial. I see how it fits in the API's subsequent versions.

Doing a ballparking in persistence patterns, I can see those beyond Repository that might come in new versions:

• Data Access Object
• Active Record
• Mapper
• Data Transfer Object
• Command Query Responsibility Segregation
• Reactive API

What I do in my mind is to have a domain for each pattern instead of putting all of those into a single interface, currently called CrudRepository.

What do you have in mind for Jakarta Data's following versions?

It is more to define bundles of each pattern we might have in the future instead of having all those into a single point.

[njr-11]

t is more related to the API future than the insert and update method; as I said, those are crucial. I see how it fits in the API's subsequent versions.

Doing a ballparking in persistence patterns, I can see those beyond Repository that might come in new versions:

* Data Access Object

* Active Record

* Mapper

* Data Transfer Object

* Command Query Responsibility Segregation

* Reactive API

What I do in my mind is to have a domain for each pattern instead of putting all of those into a single interface, currently called CrudRepository.

What do you have in mind for Jakarta Data's following versions?

It is more to define bundles of each pattern we might have in the future instead of having all those into a single point.

Yes, that's smart to look ahead toward future plans to ensure what we are doing now is consistent with it. I agree we wouldn't be adding all these other patterns onto CrudRepository. CrudRepository should only cover the basic CRUD operations (Create, Read, Update, Delete) that a user would expect from something with CRUD in its name.

If you are agreeing to put the methods back onto CrudRepository, let me know and I can put them back. Otherwise, review this PR without them and further discussion can continue about whether they can be included or if CrudRepository needs to be renamed because it isn't CRUD without them.

[otaviojava]

For you, what is the difference between a DAO and a Repository?

My point is, in a repository, we are doing it, but not as imperative; we explore the save. The user doesn't have this control.

DAO is where we should have inserted, and updated methods, and we should not have the save methods.
For me, insert, update, and save are antagonists, thus, where there is one should not cover others.

[njr-11]

You're pointing out it's not a repository if we include create and update operations on it, and I'm pointing out that it isn't CRUD if we don't. In last week's meeting it was decided that we would include all of (insert, update, save), to give the user the most flexibility to use whichever pattern they want, and that's how this pull was originally written. I realize the different patterns won't be purely followed if a user intermixes them. Many users don't care as much about purely following patterns as they do being able to achieve what their business logic requires. I don't see that as a problem.

[njr-11]

If everyone else believes it's more important to adhere to a pure repository pattern than to offer all of the CRUD operations (although it didn't seem that way on the last meeting), then we should rename CrudRepository to something else, maybe BasicRepository, and stop claiming it provides CRUD. If that needs to happen, it doesn't belong under this pull because this pull no longer touches the CrudRepository. This sort of debate should not be happening here and is just holding up progress.

[otaviojava]

If everyone else believes it's more important to adhere to a pure repository pattern than to offer all of the CRUD operations (although it did seem that way on the last meeting), then we should rename CrudRepository to something else, maybe BasicRepository, and stop claiming it provides CRUD. If that needs to happen, it doesn't belong under this pull because this pull no longer touches the CrudRepository. This sort of debate should not be happening here and is just holding up progress.

I like the BasicRepository approach.

[otaviojava]

You're pointing out it's not a repository if we include create and update operations on it, and I'm pointing out that it isn't CRUD if we don't. In last week's meeting it was decided that we would include all of (insert, update, save), to give the user the most flexibility to use whichever pattern they want, and that's how this pull was originally written. I realize the different patterns won't be purely followed if a user intermixes them. Many users don't care as much about purely following patterns as they do being able to achieve what their business logic requires. I don't see that as a problem.

Yes, I liked the proposal of renaming it to "BasicRepository" if it makes it more accessible and avoids the CRUD expectative.

You have a good point: users sometimes do not care about it, mainly to archive business requirements. But, by default, the standards keep moving to the proper path.

Such as in the EJB Singleton annotation guide to a unique instance, at CDI ApplicationScope being to the scope of the application, at JAX-RS GET annotation to HTTP get, and so on.

[gavinking]

Hey everyone, I understand there was a recent vote to do a git revert and implicitly include the insert and update methods in the Jakarta Data spec as dynamic methods. However, I'd like to encourage a thorough reconsideration of this decision for the following reasons:

Preserving Abstraction: The cornerstone of the Repository pattern is Abstraction, and it has historically encapsulated insert and update operations within the save method, as per our documentation. This design choice simplifies the API and keeps the core concept intact.

@otaviojava Abstraction is always a nice thing to have when two things are fundamentally the same. But when you try to abstract over things which are just too different from each other, you get in a mess.

"Make persistent" and "update" are different operations even in Jakarta Persistence, where we're dealing with just one sort of technology. Believe me, if there were a reasonable way to abstract over these operations with one single method, we would have designed Jakarta Persistence to work that way!

Now if we start to consider other of technologies, we'll find that "save this change" has different semantics again. For example, I can imagine that one single "save" method actually might indeed make sense in a document database. (But I am definitely not an expert on the topic of document databases!)

So I think Jakarta Data needs to own up to these differences, instead of trying to sweep them under the rug.

[gavinking]

What I do in my mind is to have a domain for each pattern instead of putting all of those into a single interface, currently called CrudRepository.

@otaviojava I think you need to consider that in my proposal, where there is an abstracted notion of "lifecycle methods", which are specialized by an extensible set of lifecycle annotations, that you don't need to multiply the set of BasicRepository types. This is fundamentally more flexible.

Your "basic repository" is just a repository with a lifecycle method with a @Save annotation. Nathan's CRUDRepository is a repository with a lifecycle methods annotated @Insert and @Update. And there's no longer a need to fight over whose model is "better" or more "basic", because both models exist an an equal footing and are matched to the datastore / use case.

[njr-11]

This is obsolete now that the lifecycle annotations were introduced and documentation was put there instead.