Return the CrudRepository method with the insert and update methods

[otaviojava]

Changes

• Return the CrudRepository interface
• Move the insert and update methods to the CrudRepository (I understood that is the goal to return this interface)
• I added to return with the entities, mainly to be compatible with some NoSQL database or even SQL data that might work with Java records eventually

[njr-11]

I'm fine with the approach of moving the insert and update methods to a CrudRepository rather than having them on BasicRepository. However, I don't agree with changing their return types. Not needing to fetch the entities is one of the distinctions that these patterns have that make them different from save/saveAll which already offers that ability.

[otaviojava]

I'm fine with the approach of moving the insert and update methods to a CrudRepository rather than having them on BasicRepository. However, I don't agree with changing their return types. Not needing to fetch the entities is one of the distinctions that these patterns have that make them different from save/saveAll which already offers that ability.

let's take a step back:

• insert: create an information into any database.
• update: change information that already does exist. With NoSQL it might be different. Please, let me know if what I put in the documentation makes sense.
• save: it is an abstraction: As our documentation says: If the entity has an ID or key that exists in the database, the method will update the existing record. Otherwise, it will insert a new record.

If I want to explicitly insert and update I should use those methods instead of "save".

Does it make sense?

[njr-11]

@otaviojava It's fine to make things unsupported where the database isn't capable of it, but the user needs to be able to rely on guarantees. Using ambiguous language as is added under e52fbce and 56b7c42 will deprive them of the ability to rely on what the method does and make it useless. Could you revert those commits and not write it in terms of "might work for relational databases". Relational databases shouldn't be mentioned at all since some other databases can be capable of the same. Instead, specifically call out what types of databases are not capable as raising UnsupportedOperationException.

[njr-11]

I'm fine with the approach of moving the insert and update methods to a CrudRepository rather than having them on BasicRepository. However, I don't agree with changing their return types. Not needing to fetch the entities is one of the distinctions that these patterns have that make them different from save/saveAll which already offers that ability.

let's take a step back:

* insert: create an information into any database.

* update: change information that already does exist. With NoSQL it might be different. Please, let me know if what I put in the documentation makes sense.

* save: it is an abstraction: As our documentation says: If the entity has an ID or key that exists in the database, the method will update the existing record. Otherwise, it will insert a new record.

If I want to explicitly insert and update I should use those methods instead of "save".

Does it make sense?

Your summary here sounds great. It's your changes in the pull regarding return types that aren't consistent with it. There is nothing about create/insert and update that should involve fetching entities.

[otaviojava]

@otaviojava It's fine to make things unsupported where the database isn't capable of it, but the user needs to be able to rely on guarantees. Using ambiguous language as is added under e52fbce and 56b7c42 will deprive them of the ability to rely on what the method does and make it useless. Could you revert those commits and not write it in terms of "might work for relational databases". Relational databases shouldn't be mentioned at all since some other databases can be capable of the same. Instead, specifically call out what types of databases are not capable as raising UnsupportedOperationException.

hum, I got it.

Could you help me to write this point?

Those databases can insert and update but using BASE instead of ACID.

In theory, both can use those methods, however, with different data consistency models.

We also have append model, it is inserting and updating, but different from SQL...

[otaviojava]

@njr-11
I am glad that you enjoyed the divide-and-conquer strategy :)
The insert and update methods are a Pandora's box. I need help making it work for SQL and NoSQL and, if possible, allow us to use Java Record on those methods.

[njr-11]

I am glad that you enjoyed the divide-and-conquer strategy :)
The insert and update methods are a Pandora's box. I need help making it work for SQL and NoSQL and, if possible, allow us to use Java Record on those methods.

How about using divide-and-conquer strategy in an even more fine grained way? In this pull, only move the methods to CrudRepository without changing the return types. We can all approve that and merge it.
Then, in a separate pull, work on whatever precise wording we need to state when UnsupportedOperationException is raised or when EntityExistsException should not be raised.
If you still believe you need insert and update retrieving entities from the database, open a separate issue for that. I'm not going to approve a pull that includes making insert and update perform retrieval of entities because that isn't CRUD.

[otaviojava]

I am glad that you enjoyed the divide-and-conquer strategy :)
The insert and update methods are a Pandora's box. I need help making it work for SQL and NoSQL and, if possible, allow us to use Java Record on those methods.

How about using divide-and-conquer strategy in an even more fine grained way? In this pull, only move the methods to CrudRepository without changing the return types. We can all approve that and merge it. Then, in a separate pull, work on whatever precise wording we need to state when UnsupportedOperationException is raised or when EntityExistsException should not be raised. If you still believe you need insert and update retrieving entities from the database, open a separate issue for that. I'm not going to approve a pull that includes making insert and update perform retrieval of entities because that isn't CRUD.

@njr-11
Okay, you are right, I returned the PR to only extract those methods, once we merged we can discuss more about insert and update.

So, the next one will be hard.

[njr-11]

This looks good now. We can discuss the related topics that came up in separate issues.