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

Support for immutable @Id without Wither [DATACMNS-1639] #2065

Closed
spring-projects-issues opened this issue Dec 17, 2019 · 4 comments
Closed

Support for immutable @Id without Wither [DATACMNS-1639] #2065

spring-projects-issues opened this issue Dec 17, 2019 · 4 comments

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Dec 17, 2019

Lovro Pandžić opened DATACMNS-1639 and commented

Currently Wither is required for @Id fields which seems unnecessary.

Can support be added so that instead of Wither, when one isn't present, the class constructor is called?


Issue Links:

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 17, 2019

Oliver Drotbohm commented

I.e. you're suggesting that in the context of "Here's an entity. Here's an ID. 'Set' the id on that entity", we could/should flip from "setting the property" to "create a fresh instance of the type using the persistence constructor"? The issue here is that currently we're working with an abstraction around an object instance that as of today does not have to know about object creation in the first place, but only needs to abstract over different ways to "set" property values, in which "set" might mean to call wither methods that create new instances.

Object creation has traditionally been something much more related to the conversion subsystem and individual stores as they in turn define the source for constructor arguments and massage them into the right types in the first place.

What's the reason you'd want to avoid the wither in the first place? Spring Data can deal with (package) protected and even private wither methods, so that you can add them without exposing the method to other code.

All of that said, I just threw together a quick POC that effectively covers the happy path and generally works. I just have to take it to the team to discuss the architectural implications

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 18, 2019

Lovro Pandžić commented

Oliver Drotbohm thanks for quick reply!

Regarding @Id and Wither, I understand your point of view as a maintainer.

Here's my perspective as a user:
While reading the latest docs I've got the impression that Spring Data JDBC fully supports immutable entities and example with Wither for Id seemed more of a 'here's a list of all the different approaches you can take while designing your entity' than a hard requirement and I was surprised by it.

Wither makes sense to me similar like a setter would be for mutable entities for cases where a property might change during the lifecycle of that data (or entity). I don't expect any user to actually want to change Id of an entity other than on creation through a constructor.

 

So I expect that this either be made more clear in the docs or that Wither is a feature rather than a requirement for @Id on immutable entities

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 18, 2019

Oliver Drotbohm commented

Thanks, Lovro. That makes a lot of sense. I'll take your request to improve upon the reference documentation immediately. I'll also take the prototype that's using the persistence constructor as a copy constructor to the team and see what we can do. It'll require some changes to internal APIs though so that it's unlikely that we can introduce this in a service release in Moore but rather keep it as a new feature for the upcoming Neumann release

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 18, 2019

Lovro Pandžić commented

Great, thanks Oliver!

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

Successfully merging a pull request may close this issue.

None yet
2 participants