Skip to content
This repository has been archived by the owner on Jan 17, 2019. It is now read-only.

Avoid requiring implementing Persistable is domain objects #28

Open
sdeleuze opened this issue Mar 8, 2016 · 8 comments
Open

Avoid requiring implementing Persistable is domain objects #28

sdeleuze opened this issue Mar 8, 2016 · 8 comments

Comments

@sdeleuze
Copy link

sdeleuze commented Mar 8, 2016

Any thoughts about requiring only an @Id annotation (like in Spring Data MongDB support) instead of implementing Persistable? That would be less intrusive and more acceptable in many use cases ...

@sdeleuze
Copy link
Author

sdeleuze commented Mar 9, 2016

I think I am going to send a PR that will use ReflectionEntityInformation to get ID if T does not implement Persistable for that feature, if that's fine to you @nurkiewicz.

@sdeleuze
Copy link
Author

sdeleuze commented Mar 9, 2016

I have created a draft and untested commit just for show the principle of such change: sdeleuze@0b1a509

The isNew logic need to be improved I think.

@jirutka
Copy link

jirutka commented Mar 10, 2016

FYI, I plan to address this request in my fork of this library. I'm working on (optional) support for upsert (PostgreSQL's on conflict do, merge for some others), then isNew will be quite redundant.

@sdeleuze
Copy link
Author

@jirutka Very interesting. Since I need to move fast on that topic, I am going to create an issue for discussion and a draft PR directly on your fork.

@nurkiewicz
Copy link
Owner

I chose Persistable so that I don't need to use reflection at any point, keeping the library very lightweight. But I guess properly written we can support both fast Persistable and convenient @Id.

That being said for the time being I have neither time nor interest in supporting this library. These days I would actually choose https://github.com/jOOQ/jOOQ myself for low-level SQL scenarios. However seeing there is some interest in the community I'd love someone to continue development.

@jirutka I see you did a wonderful work on your fork, including 0.5 release. I'm sorry for missing your PRs earlier (#26 and #27). Would you be interested in merging you changes back to this repository (I can give you commit rights)? Alternatively I can point to your repository in README.md, leaving this for historical reasons?

@sdeleuze
Copy link
Author

I have pushed a spring-data-jdbc-repository branch of my geospatial-messenger Kotlin project, based on @jirutka fork for the moment and published this blog post.

@nurkiewicz Indeed I see some interest to jOOQ or Query DSL SQL approach, but I REALLy hate code generation that why I ended up to use Exposed (a Kotlin SQL DSL that requires no code generation) in geospatial-messenger master branch. I don't know id such approach could work in pure Java.

@jirutka Based on your latest changes, it seems you prefer to continue to develop actively this project on your fork, right?

@jirutka
Copy link

jirutka commented Mar 22, 2016

Sorry for long delay, I somehow overlooked these comments.

I chose Persistable so that I don't need to use reflection at any point, keeping the library very lightweight.

I understand this reasoning. I also quite like explicit interface instead of using reflection just to get entity’s primary key. On the other side, I don’t like about Persistable that it also defines method isNew(), but since I use Groovy, it’s not big deal for me (see my Timestamped trait :) ).

But I guess properly written we can support both fast Persistable and convenient @id.

This is already solved in Spring Data Commons – it provides interface EntityInformation and implementations PersistableEntityInformation (uses Persistable interface), ReflectionEntityInformation (uses reflection) and some others. That’s what I’ve used in my fork (see these lines).

These days I would actually choose https://github.com/jOOQ/jOOQ.

This project is indeed very interesting, but I don’t use it for three reasons. It’s complex as hell; okish if you have high requirements, but bad for simpler things. It utilizes code generation and the same as @sdeleuze, I really don’t like generated code. It’s not actually open-source; there are some sources available, but if I remember it correctly, most of them is generated by some tool that is not open-source.

However seeing there is some interest in the community I'd love someone to continue development.

Challenge accepted! 😸

@jirutka I see you did a wonderful work on your fork, including 0.5 release.

Thanks. 😺

I'm sorry for missing your PRs earlier (#26 and #27).

No problem, I wanted to fork it anyway, so I can do more radical changes. Release 0.5.0 preserves compatibility with your 0.4.x (and I’ll support it for some time), but next release (0.6.0) will introduce many breaking changes (see changelog).

Would you be interested in merging you changes back to this repository (I can give you commit rights)? Alternatively I can point to your repository in README.md, leaving this for historical reasons?

I prefer to maintain it in my repository. However, if you like to merge some or all changes from 0.5.x branch back to your repository, I have no problem with it.

I’d be very happy if you point to it in your README!

@nurkiewicz
Copy link
Owner

I’d be very happy if you point to it in your README!

Done in fafe7dc. Have fun with the project! Hope you'll find it useful.

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

No branches or pull requests

3 participants