Re: [data] dto and isNew
So isNew is broken for openjpa and one should live with it? This will
basically make deltaspike data not usable because you kinda need merge to
work properly...
On 17 June 2014 19:11, Thomas Hug thomas@gmail.com wrote:
Actually the simple mapper is doing that since the last update, just to
find the entity based on the PK so you receive an attached entity (also
valid for chained mappers, when injected)
BTW, if you need something different in save, you can still define your own
reusable repository methods:
http://deltaspike.apache.org/data.html#extensions
(just don't name it save ;-)
On Tue, Jun 17, 2014 at 4:41 PM, Romain Manni-Bucau rmannibu...@gmail.com
wrote:
Yes, but that s not an issue since you can get it injected
Le lundi 16 juin 2014, Karl Kildén karl.kil...@gmail.com a écrit :
But then I need to use the entityManager in the mapper or am I missing
something?
On 16 June 2014 11:17, Romain Manni-Bucau rmannibu...@gmail.com
wrote:
Yes you need to merge it but the responsability is yours (user) IMO.
Le 16 juin 2014 09:56, Karl Kildén karl.kil...@gmail.com a écrit
:
Hrmm maybe I mixed things up now.
If you have a relationship to another pre existing entity can it be
detached when you save? All I am really looking for is the groupId
to
be
updated but maybe JPA can't determine this in a good way. And
updating
the
entity itself is solved by including it as an argument to the
mapper,
all
though I am a wondering if that solution is optimal.
Romain and Thomas, your comments on the best way to handle
relationships
in
the Mapper? If the entity has not changed then you can just use
user.getGroup() but as I described previously this is wrong when the
group
has changed.
On 16 June 2014 10:34, Romain Manni-Bucau rmannibu...@gmail.com
wrote:
Cause mapping can be done through several transactions (think
jaxrs)
so
it
would be wrong. PersistenceUtil has some good things to gey id or
null
if
new but IIRC some impl are broken.
Le 16 juin 2014 09:31, Thomas Andraschko
andraschko.tho...@gmail.com
a
écrit :
Why don't we use entityManager#contains instead of checking the
ID?
2014-06-16 10:22 GMT+02:00 Karl Kildén karl.kil...@gmail.com:
Hi,
On could argue that the real problem is that the algorithm
that
decides
if
it should be a save or persist. It uses some portable way of
deciding
this
that requires the entity to be managed.
That algorithm could be improved in each project.
@Override
@RequiresTransaction
public E save(E entity)
{
if (context.isNew(entity))
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
I would say that overriding this method would solve
EntityExistsException
in a cleaner way. For this project I have no natural keys
only
generated
long so it would be a cheap and easy way to fix it... This is
fully
sufficient for me:
@Override
@RequiresTransaction
public E save(E entity)
{
BaseEntity e = (BaseEntity) entity;
if (e.getId == 0)
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
If not overridden then what happens if the group has changed
in
my
example,
you are supposed to use entityManager and find it?
Maybe the API should provide something like the utility
method I
proposed
then... I will explain it a little better. All you need to do
is
inject
the
groupMapper or whatever mapper you need. To get the group if
it
changed
you
simply call fetch with a new Group instance, the DTO with the
new
information and the groupMapper. Why send in new group
instance?
Well
one
could also send in Group.class and use a reflection to create
a
new
group
if needed. But new Group() vs Group.class I actually prefer
the
first
because it avoids reflection. Because the new Group() argument
also
allows
for getClass() the entityManager has all the info required
except
the
id
but that is no problem since we also send in the mapper with
the
handy
#getPrimaryKey
method.
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto
Re: [data] dto and isNew
well we have to make it working with openjpa by default. We should
also introduce an interface the entity can implement to say if it is
new or not (testing business id typically)
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-08-18 18:08 GMT+02:00 Karl Kildén karl.kil...@gmail.com:
So isNew is broken for openjpa and one should live with it? This will
basically make deltaspike data not usable because you kinda need merge to
work properly...
On 17 June 2014 19:11, Thomas Hug thomas@gmail.com wrote:
Actually the simple mapper is doing that since the last update, just to
find the entity based on the PK so you receive an attached entity (also
valid for chained mappers, when injected)
BTW, if you need something different in save, you can still define your own
reusable repository methods:
http://deltaspike.apache.org/data.html#extensions
(just don't name it save ;-)
On Tue, Jun 17, 2014 at 4:41 PM, Romain Manni-Bucau rmannibu...@gmail.com
wrote:
Yes, but that s not an issue since you can get it injected
Le lundi 16 juin 2014, Karl Kildén karl.kil...@gmail.com a écrit :
But then I need to use the entityManager in the mapper or am I missing
something?
On 16 June 2014 11:17, Romain Manni-Bucau rmannibu...@gmail.com
wrote:
Yes you need to merge it but the responsability is yours (user) IMO.
Le 16 juin 2014 09:56, Karl Kildén karl.kil...@gmail.com a écrit
:
Hrmm maybe I mixed things up now.
If you have a relationship to another pre existing entity can it be
detached when you save? All I am really looking for is the groupId
to
be
updated but maybe JPA can't determine this in a good way. And
updating
the
entity itself is solved by including it as an argument to the
mapper,
all
though I am a wondering if that solution is optimal.
Romain and Thomas, your comments on the best way to handle
relationships
in
the Mapper? If the entity has not changed then you can just use
user.getGroup() but as I described previously this is wrong when the
group
has changed.
On 16 June 2014 10:34, Romain Manni-Bucau rmannibu...@gmail.com
wrote:
Cause mapping can be done through several transactions (think
jaxrs)
so
it
would be wrong. PersistenceUtil has some good things to gey id or
null
if
new but IIRC some impl are broken.
Le 16 juin 2014 09:31, Thomas Andraschko
andraschko.tho...@gmail.com
a
écrit :
Why don't we use entityManager#contains instead of checking the
ID?
2014-06-16 10:22 GMT+02:00 Karl Kildén karl.kil...@gmail.com:
Hi,
On could argue that the real problem is that the algorithm
that
decides
if
it should be a save or persist. It uses some portable way of
deciding
this
that requires the entity to be managed.
That algorithm could be improved in each project.
@Override
@RequiresTransaction
public E save(E entity)
{
if (context.isNew(entity))
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
I would say that overriding this method would solve
EntityExistsException
in a cleaner way. For this project I have no natural keys
only
generated
long so it would be a cheap and easy way to fix it... This is
fully
sufficient for me:
@Override
@RequiresTransaction
public E save(E entity)
{
BaseEntity e = (BaseEntity) entity;
if (e.getId == 0)
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
If not overridden then what happens if the group has changed
in
my
example,
you are supposed to use entityManager and find it?
Maybe the API should provide something like the utility
method I
proposed
then... I will explain it a little better. All you need to do
is
inject
the
groupMapper or whatever mapper you need. To get the group if
it
changed
you
simply call fetch with a new Group instance, the DTO with the
new
information and the groupMapper. Why send in new group
instance?
Well
one
could also send in Group.class and use a reflection to create
a
new
group
if needed. But new Group() vs Group.class I actually prefer
the
first
because it avoids reflection. Because the new Group() argument
also
Re: [data] dto and isNew
Yes, but that s not an issue since you can get it injected
Le lundi 16 juin 2014, Karl Kildén karl.kil...@gmail.com a écrit :
But then I need to use the entityManager in the mapper or am I missing
something?
On 16 June 2014 11:17, Romain Manni-Bucau rmannibu...@gmail.com wrote:
Yes you need to merge it but the responsability is yours (user) IMO.
Le 16 juin 2014 09:56, Karl Kildén karl.kil...@gmail.com a écrit :
Hrmm maybe I mixed things up now.
If you have a relationship to another pre existing entity can it be
detached when you save? All I am really looking for is the groupId to
be
updated but maybe JPA can't determine this in a good way. And updating
the
entity itself is solved by including it as an argument to the mapper,
all
though I am a wondering if that solution is optimal.
Romain and Thomas, your comments on the best way to handle
relationships
in
the Mapper? If the entity has not changed then you can just use
user.getGroup() but as I described previously this is wrong when the
group
has changed.
On 16 June 2014 10:34, Romain Manni-Bucau rmannibu...@gmail.com
wrote:
Cause mapping can be done through several transactions (think jaxrs)
so
it
would be wrong. PersistenceUtil has some good things to gey id or
null
if
new but IIRC some impl are broken.
Le 16 juin 2014 09:31, Thomas Andraschko
andraschko.tho...@gmail.com
a
écrit :
Why don't we use entityManager#contains instead of checking the ID?
2014-06-16 10:22 GMT+02:00 Karl Kildén karl.kil...@gmail.com:
Hi,
On could argue that the real problem is that the algorithm that
decides
if
it should be a save or persist. It uses some portable way of
deciding
this
that requires the entity to be managed.
That algorithm could be improved in each project.
@Override
@RequiresTransaction
public E save(E entity)
{
if (context.isNew(entity))
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
I would say that overriding this method would solve
EntityExistsException
in a cleaner way. For this project I have no natural keys only
generated
long so it would be a cheap and easy way to fix it... This is
fully
sufficient for me:
@Override
@RequiresTransaction
public E save(E entity)
{
BaseEntity e = (BaseEntity) entity;
if (e.getId == 0)
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
If not overridden then what happens if the group has changed in
my
example,
you are supposed to use entityManager and find it?
Maybe the API should provide something like the utility method I
proposed
then... I will explain it a little better. All you need to do is
inject
the
groupMapper or whatever mapper you need. To get the group if it
changed
you
simply call fetch with a new Group instance, the DTO with the new
information and the groupMapper. Why send in new group instance?
Well
one
could also send in Group.class and use a reflection to create a
new
group
if needed. But new Group() vs Group.class I actually prefer the
first
because it avoids reflection. Because the new Group() argument
also
allows
for getClass() the entityManager has all the info required except
the
id
but that is no problem since we also send in the mapper with the
handy
#getPrimaryKey
method.
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity)
entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
I have not tested this method at all, but something like it would
work
well
together with the default strategy for determine save or merge...
But
my
main wish now is to override the save logic actually.
On 16 June 2014 09:48, Thomas Hug thomas@gmail.com wrote:
Thanks Karl for the clarification!
If you assign a new group, I'd first make sure you have this
one
persisted
as well (or we do too much in the mapper IMO). Then save the
userDto
with
something like
User toEntity(User user, UserDto dto) {
Re: [data] dto and isNew
Actually the simple mapper is doing that since the last update, just to
find the entity based on the PK so you receive an attached entity (also
valid for chained mappers, when injected)
BTW, if you need something different in save, you can still define your own
reusable repository methods:
http://deltaspike.apache.org/data.html#extensions
(just don't name it save ;-)
On Tue, Jun 17, 2014 at 4:41 PM, Romain Manni-Bucau rmannibu...@gmail.com
wrote:
Yes, but that s not an issue since you can get it injected
Le lundi 16 juin 2014, Karl Kildén karl.kil...@gmail.com a écrit :
But then I need to use the entityManager in the mapper or am I missing
something?
On 16 June 2014 11:17, Romain Manni-Bucau rmannibu...@gmail.com wrote:
Yes you need to merge it but the responsability is yours (user) IMO.
Le 16 juin 2014 09:56, Karl Kildén karl.kil...@gmail.com a écrit :
Hrmm maybe I mixed things up now.
If you have a relationship to another pre existing entity can it be
detached when you save? All I am really looking for is the groupId to
be
updated but maybe JPA can't determine this in a good way. And updating
the
entity itself is solved by including it as an argument to the mapper,
all
though I am a wondering if that solution is optimal.
Romain and Thomas, your comments on the best way to handle
relationships
in
the Mapper? If the entity has not changed then you can just use
user.getGroup() but as I described previously this is wrong when the
group
has changed.
On 16 June 2014 10:34, Romain Manni-Bucau rmannibu...@gmail.com
wrote:
Cause mapping can be done through several transactions (think jaxrs)
so
it
would be wrong. PersistenceUtil has some good things to gey id or
null
if
new but IIRC some impl are broken.
Le 16 juin 2014 09:31, Thomas Andraschko
andraschko.tho...@gmail.com
a
écrit :
Why don't we use entityManager#contains instead of checking the
ID?
2014-06-16 10:22 GMT+02:00 Karl Kildén karl.kil...@gmail.com:
Hi,
On could argue that the real problem is that the algorithm that
decides
if
it should be a save or persist. It uses some portable way of
deciding
this
that requires the entity to be managed.
That algorithm could be improved in each project.
@Override
@RequiresTransaction
public E save(E entity)
{
if (context.isNew(entity))
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
I would say that overriding this method would solve
EntityExistsException
in a cleaner way. For this project I have no natural keys only
generated
long so it would be a cheap and easy way to fix it... This is
fully
sufficient for me:
@Override
@RequiresTransaction
public E save(E entity)
{
BaseEntity e = (BaseEntity) entity;
if (e.getId == 0)
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
If not overridden then what happens if the group has changed in
my
example,
you are supposed to use entityManager and find it?
Maybe the API should provide something like the utility method I
proposed
then... I will explain it a little better. All you need to do is
inject
the
groupMapper or whatever mapper you need. To get the group if it
changed
you
simply call fetch with a new Group instance, the DTO with the
new
information and the groupMapper. Why send in new group instance?
Well
one
could also send in Group.class and use a reflection to create a
new
group
if needed. But new Group() vs Group.class I actually prefer the
first
because it avoids reflection. Because the new Group() argument
also
allows
for getClass() the entityManager has all the info required
except
the
id
but that is no problem since we also send in the mapper with the
handy
#getPrimaryKey
method.
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity)
entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
I have not tested this method
Re: [data] dto and isNew
Thanks Karl for the clarification!
If you assign a new group, I'd first make sure you have this one persisted
as well (or we do too much in the mapper IMO). Then save the userDto with
something like
User toEntity(User user, UserDto dto) {
...
user.setGroup(groupMapper.toEntity(dto.getGroup()); // or check before
if ID changed
}
I guess that would even work if the group is not persisted if you adapt
cascading.
Makes sense?
On Fri, Jun 13, 2014 at 5:56 PM, Karl Kildén karl.kil...@gmail.com wrote:
Not sure I get myself ;)
Lets walk through how I see it:
1. User foo is created and is assigned to the preexisting group Admin.
It goes like this in the flow: user = new UserDTO() -
*user*.setGroupDTO(selectedGroup)
- save
Some logic must make sure that we don't get EntityExistsException trying to
save that group.
2. Many sessions later user foo is edited. Flow:
*user*.setGroupDTO(newGroup)
- save
The variable *user *is a random object that happens to be the latest
version of that user that also has a new group set.
So *PK, user.getGroup()*
*should lazyload the group - right? *This will result in the previous group
being loaded unless I am missing something. While it is technically correct
since the new group relationship has not been persisted yet it is actually
impossible to ever update group with this flow
On 13 June 2014 17:09, Thomas Hug thomas@gmail.com wrote:
Hi Karl
Sorry not sure if I get you right. Why would user.getGroup() be stale? As
in the update case user has just been fetched by the PK, user.getGroup()
should lazyload the group - right?
On Fri, Jun 13, 2014 at 2:51 PM, Karl Kildén karl.kil...@gmail.com
wrote:
Hi and hrmmm,
What if user group was changed? groupMapper.toEntity(user.getGroup(),
userDto.getGroup()); This would then operate on stale data unless you
fetch
and send in correct group. And managing new groups is easy for me I
think,
I would call the method using groupMapper.toEntity(new Group(),
userDto.getGroup());
I would much prefer all three methods to be non protected. Then I could
create my helper something along the lines of this:
I did not test this very well but unless I thought wrong completely it
would be a one time thing to implement. But using mappers from mappers
are
hard because if the relationship is declared in both ways you can get
circular references. Anyways here's my helper I theorycrafted together:
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity)
entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
One could always create their own base class overriding the protected
methods and adding that fetch helper I guess...
cheers
On 13 June 2014 12:54, Thomas Hug thomas@gmail.com wrote:
Hey Karl
Sorry missed your mail indeed - it's probably time I subscribe to the
user
mailing list too :-)
You can still chain those mappers together, but I agree the casting
and
wiring is clunky. As options I see:
- we add a public E toEntity(Dto dto) back which basically does the
same
as
mapParameter now (just hides the casting) for new entities
- we make the E toEntity(Entity e, Dto dto) public as well, so it's
quite
easy to chain mapper calls for updates
groupMapper.toEntity(user.getGroup(), userDto.getGroup());
You will still have to have some conditionals to see which one to
call,
also depends on your data model (required relations, lazy or eager
fetch).
How does that look? Better ideas?
On Fri, Jun 13, 2014 at 8:42 AM, Karl Kildén karl.kil...@gmail.com
wrote:
I wrote a response to the users list but not sure it came through.
It
kinda
belongs in this thread so here it goes.
So I ran into issues with the DTO mapper api and voiced my concerns
in
irc.
I saw this discussion and now I am trying the solution present in
the
current SNAPSHOT. However I have one comment / question:
What if my Entity has a relationship to another Entity?
Like this:
public class User extends BaseAuditEntity {
@Column
private String name;
@Column
@ManyToOne
@JoinColumn(name=group_id)
private Group group;
This means my userDTO may come with a GroupDTO and how do I map
that
relationship? Or to explain by code please fill in the question
marks
below
;) or if you feel my implementation is
Re: [data] dto and isNew
Hi,
On could argue that the real problem is that the algorithm that decides if
it should be a save or persist. It uses some portable way of deciding this
that requires the entity to be managed.
That algorithm could be improved in each project.
@Override
@RequiresTransaction
public E save(E entity)
{
if (context.isNew(entity))
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
I would say that overriding this method would solve EntityExistsException
in a cleaner way. For this project I have no natural keys only generated
long so it would be a cheap and easy way to fix it... This is fully
sufficient for me:
@Override
@RequiresTransaction
public E save(E entity)
{
BaseEntity e = (BaseEntity) entity;
if (e.getId == 0)
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
If not overridden then what happens if the group has changed in my example,
you are supposed to use entityManager and find it?
Maybe the API should provide something like the utility method I proposed
then... I will explain it a little better. All you need to do is inject the
groupMapper or whatever mapper you need. To get the group if it changed you
simply call fetch with a new Group instance, the DTO with the new
information and the groupMapper. Why send in new group instance? Well one
could also send in Group.class and use a reflection to create a new group
if needed. But new Group() vs Group.class I actually prefer the first
because it avoids reflection. Because the new Group() argument also allows
for getClass() the entityManager has all the info required except the id
but that is no problem since we also send in the mapper with the handy
#getPrimaryKey
method.
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity) entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
I have not tested this method at all, but something like it would work well
together with the default strategy for determine save or merge... But my
main wish now is to override the save logic actually.
On 16 June 2014 09:48, Thomas Hug thomas@gmail.com wrote:
Thanks Karl for the clarification!
If you assign a new group, I'd first make sure you have this one persisted
as well (or we do too much in the mapper IMO). Then save the userDto with
something like
User toEntity(User user, UserDto dto) {
...
user.setGroup(groupMapper.toEntity(dto.getGroup()); // or check before
if ID changed
}
I guess that would even work if the group is not persisted if you adapt
cascading.
Makes sense?
On Fri, Jun 13, 2014 at 5:56 PM, Karl Kildén karl.kil...@gmail.com
wrote:
Not sure I get myself ;)
Lets walk through how I see it:
1. User foo is created and is assigned to the preexisting group
Admin.
It goes like this in the flow: user = new UserDTO() -
*user*.setGroupDTO(selectedGroup)
- save
Some logic must make sure that we don't get EntityExistsException trying
to
save that group.
2. Many sessions later user foo is edited. Flow:
*user*.setGroupDTO(newGroup)
- save
The variable *user *is a random object that happens to be the latest
version of that user that also has a new group set.
So *PK, user.getGroup()*
*should lazyload the group - right? *This will result in the previous
group
being loaded unless I am missing something. While it is technically
correct
since the new group relationship has not been persisted yet it is
actually
impossible to ever update group with this flow
On 13 June 2014 17:09, Thomas Hug thomas@gmail.com wrote:
Hi Karl
Sorry not sure if I get you right. Why would user.getGroup() be stale?
As
in the update case user has just been fetched by the PK,
user.getGroup()
should lazyload the group - right?
On Fri, Jun 13, 2014 at 2:51 PM, Karl Kildén karl.kil...@gmail.com
wrote:
Hi and hrmmm,
What if user group was changed?
groupMapper.toEntity(user.getGroup(),
userDto.getGroup()); This would then operate on stale data unless you
fetch
and send in correct group. And managing new groups is easy for me I
think,
I would call the method using groupMapper.toEntity(new Group(),
userDto.getGroup());
I would much prefer all three methods to be non protected. Then I
could
create my helper something along the lines of this:
I did not test this very well but unless I thought wrong completely
it
Re: [data] dto and isNew
Why don't we use entityManager#contains instead of checking the ID?
2014-06-16 10:22 GMT+02:00 Karl Kildén karl.kil...@gmail.com:
Hi,
On could argue that the real problem is that the algorithm that decides if
it should be a save or persist. It uses some portable way of deciding this
that requires the entity to be managed.
That algorithm could be improved in each project.
@Override
@RequiresTransaction
public E save(E entity)
{
if (context.isNew(entity))
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
I would say that overriding this method would solve EntityExistsException
in a cleaner way. For this project I have no natural keys only generated
long so it would be a cheap and easy way to fix it... This is fully
sufficient for me:
@Override
@RequiresTransaction
public E save(E entity)
{
BaseEntity e = (BaseEntity) entity;
if (e.getId == 0)
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
If not overridden then what happens if the group has changed in my example,
you are supposed to use entityManager and find it?
Maybe the API should provide something like the utility method I proposed
then... I will explain it a little better. All you need to do is inject the
groupMapper or whatever mapper you need. To get the group if it changed you
simply call fetch with a new Group instance, the DTO with the new
information and the groupMapper. Why send in new group instance? Well one
could also send in Group.class and use a reflection to create a new group
if needed. But new Group() vs Group.class I actually prefer the first
because it avoids reflection. Because the new Group() argument also allows
for getClass() the entityManager has all the info required except the id
but that is no problem since we also send in the mapper with the handy
#getPrimaryKey
method.
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity) entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
I have not tested this method at all, but something like it would work well
together with the default strategy for determine save or merge... But my
main wish now is to override the save logic actually.
On 16 June 2014 09:48, Thomas Hug thomas@gmail.com wrote:
Thanks Karl for the clarification!
If you assign a new group, I'd first make sure you have this one
persisted
as well (or we do too much in the mapper IMO). Then save the userDto with
something like
User toEntity(User user, UserDto dto) {
...
user.setGroup(groupMapper.toEntity(dto.getGroup()); // or check
before
if ID changed
}
I guess that would even work if the group is not persisted if you adapt
cascading.
Makes sense?
On Fri, Jun 13, 2014 at 5:56 PM, Karl Kildén karl.kil...@gmail.com
wrote:
Not sure I get myself ;)
Lets walk through how I see it:
1. User foo is created and is assigned to the preexisting group
Admin.
It goes like this in the flow: user = new UserDTO() -
*user*.setGroupDTO(selectedGroup)
- save
Some logic must make sure that we don't get EntityExistsException
trying
to
save that group.
2. Many sessions later user foo is edited. Flow:
*user*.setGroupDTO(newGroup)
- save
The variable *user *is a random object that happens to be the latest
version of that user that also has a new group set.
So *PK, user.getGroup()*
*should lazyload the group - right? *This will result in the previous
group
being loaded unless I am missing something. While it is technically
correct
since the new group relationship has not been persisted yet it is
actually
impossible to ever update group with this flow
On 13 June 2014 17:09, Thomas Hug thomas@gmail.com wrote:
Hi Karl
Sorry not sure if I get you right. Why would user.getGroup() be
stale?
As
in the update case user has just been fetched by the PK,
user.getGroup()
should lazyload the group - right?
On Fri, Jun 13, 2014 at 2:51 PM, Karl Kildén karl.kil...@gmail.com
wrote:
Hi and hrmmm,
What if user group was changed?
groupMapper.toEntity(user.getGroup(),
userDto.getGroup()); This would then operate on stale data unless
you
fetch
and send in correct group. And managing new groups is easy for me I
think,
I would call the method using
Re: [data] dto and isNew
Actually you want to override isNew and not save I think. This should be
doable (@Alternative maybe) but the mapper should be responsible of
ensuring collections are well mapped (find if needed) mainly because you
dont always map 1-1 (otherwise it is almost useless).
Le 16 juin 2014 09:22, Karl Kildén karl.kil...@gmail.com a écrit :
Hi,
On could argue that the real problem is that the algorithm that decides if
it should be a save or persist. It uses some portable way of deciding this
that requires the entity to be managed.
That algorithm could be improved in each project.
@Override
@RequiresTransaction
public E save(E entity)
{
if (context.isNew(entity))
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
I would say that overriding this method would solve EntityExistsException
in a cleaner way. For this project I have no natural keys only generated
long so it would be a cheap and easy way to fix it... This is fully
sufficient for me:
@Override
@RequiresTransaction
public E save(E entity)
{
BaseEntity e = (BaseEntity) entity;
if (e.getId == 0)
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
If not overridden then what happens if the group has changed in my example,
you are supposed to use entityManager and find it?
Maybe the API should provide something like the utility method I proposed
then... I will explain it a little better. All you need to do is inject the
groupMapper or whatever mapper you need. To get the group if it changed you
simply call fetch with a new Group instance, the DTO with the new
information and the groupMapper. Why send in new group instance? Well one
could also send in Group.class and use a reflection to create a new group
if needed. But new Group() vs Group.class I actually prefer the first
because it avoids reflection. Because the new Group() argument also allows
for getClass() the entityManager has all the info required except the id
but that is no problem since we also send in the mapper with the handy
#getPrimaryKey
method.
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity) entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
I have not tested this method at all, but something like it would work well
together with the default strategy for determine save or merge... But my
main wish now is to override the save logic actually.
On 16 June 2014 09:48, Thomas Hug thomas@gmail.com wrote:
Thanks Karl for the clarification!
If you assign a new group, I'd first make sure you have this one
persisted
as well (or we do too much in the mapper IMO). Then save the userDto with
something like
User toEntity(User user, UserDto dto) {
...
user.setGroup(groupMapper.toEntity(dto.getGroup()); // or check
before
if ID changed
}
I guess that would even work if the group is not persisted if you adapt
cascading.
Makes sense?
On Fri, Jun 13, 2014 at 5:56 PM, Karl Kildén karl.kil...@gmail.com
wrote:
Not sure I get myself ;)
Lets walk through how I see it:
1. User foo is created and is assigned to the preexisting group
Admin.
It goes like this in the flow: user = new UserDTO() -
*user*.setGroupDTO(selectedGroup)
- save
Some logic must make sure that we don't get EntityExistsException
trying
to
save that group.
2. Many sessions later user foo is edited. Flow:
*user*.setGroupDTO(newGroup)
- save
The variable *user *is a random object that happens to be the latest
version of that user that also has a new group set.
So *PK, user.getGroup()*
*should lazyload the group - right? *This will result in the previous
group
being loaded unless I am missing something. While it is technically
correct
since the new group relationship has not been persisted yet it is
actually
impossible to ever update group with this flow
On 13 June 2014 17:09, Thomas Hug thomas@gmail.com wrote:
Hi Karl
Sorry not sure if I get you right. Why would user.getGroup() be
stale?
As
in the update case user has just been fetched by the PK,
user.getGroup()
should lazyload the group - right?
On Fri, Jun 13, 2014 at 2:51 PM, Karl Kildén karl.kil...@gmail.com
wrote:
Hi and hrmmm,
What if user group was changed?
groupMapper.toEntity(user.getGroup(),
Re: [data] dto and isNew
Cause mapping can be done through several transactions (think jaxrs) so it
would be wrong. PersistenceUtil has some good things to gey id or null if
new but IIRC some impl are broken.
Le 16 juin 2014 09:31, Thomas Andraschko andraschko.tho...@gmail.com a
écrit :
Why don't we use entityManager#contains instead of checking the ID?
2014-06-16 10:22 GMT+02:00 Karl Kildén karl.kil...@gmail.com:
Hi,
On could argue that the real problem is that the algorithm that decides
if
it should be a save or persist. It uses some portable way of deciding
this
that requires the entity to be managed.
That algorithm could be improved in each project.
@Override
@RequiresTransaction
public E save(E entity)
{
if (context.isNew(entity))
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
I would say that overriding this method would solve EntityExistsException
in a cleaner way. For this project I have no natural keys only generated
long so it would be a cheap and easy way to fix it... This is fully
sufficient for me:
@Override
@RequiresTransaction
public E save(E entity)
{
BaseEntity e = (BaseEntity) entity;
if (e.getId == 0)
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
If not overridden then what happens if the group has changed in my
example,
you are supposed to use entityManager and find it?
Maybe the API should provide something like the utility method I proposed
then... I will explain it a little better. All you need to do is inject
the
groupMapper or whatever mapper you need. To get the group if it changed
you
simply call fetch with a new Group instance, the DTO with the new
information and the groupMapper. Why send in new group instance? Well one
could also send in Group.class and use a reflection to create a new group
if needed. But new Group() vs Group.class I actually prefer the first
because it avoids reflection. Because the new Group() argument also
allows
for getClass() the entityManager has all the info required except the id
but that is no problem since we also send in the mapper with the handy
#getPrimaryKey
method.
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity)
entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
I have not tested this method at all, but something like it would work
well
together with the default strategy for determine save or merge... But my
main wish now is to override the save logic actually.
On 16 June 2014 09:48, Thomas Hug thomas@gmail.com wrote:
Thanks Karl for the clarification!
If you assign a new group, I'd first make sure you have this one
persisted
as well (or we do too much in the mapper IMO). Then save the userDto
with
something like
User toEntity(User user, UserDto dto) {
...
user.setGroup(groupMapper.toEntity(dto.getGroup()); // or check
before
if ID changed
}
I guess that would even work if the group is not persisted if you adapt
cascading.
Makes sense?
On Fri, Jun 13, 2014 at 5:56 PM, Karl Kildén karl.kil...@gmail.com
wrote:
Not sure I get myself ;)
Lets walk through how I see it:
1. User foo is created and is assigned to the preexisting group
Admin.
It goes like this in the flow: user = new UserDTO() -
*user*.setGroupDTO(selectedGroup)
- save
Some logic must make sure that we don't get EntityExistsException
trying
to
save that group.
2. Many sessions later user foo is edited. Flow:
*user*.setGroupDTO(newGroup)
- save
The variable *user *is a random object that happens to be the latest
version of that user that also has a new group set.
So *PK, user.getGroup()*
*should lazyload the group - right? *This will result in the previous
group
being loaded unless I am missing something. While it is technically
correct
since the new group relationship has not been persisted yet it is
actually
impossible to ever update group with this flow
On 13 June 2014 17:09, Thomas Hug thomas@gmail.com wrote:
Hi Karl
Sorry not sure if I get you right. Why would user.getGroup() be
stale?
As
in the update case user has just been fetched by the PK,
user.getGroup()
Re: [data] dto and isNew
Hrmm maybe I mixed things up now.
If you have a relationship to another pre existing entity can it be
detached when you save? All I am really looking for is the groupId to be
updated but maybe JPA can't determine this in a good way. And updating the
entity itself is solved by including it as an argument to the mapper, all
though I am a wondering if that solution is optimal.
Romain and Thomas, your comments on the best way to handle relationships in
the Mapper? If the entity has not changed then you can just use
user.getGroup() but as I described previously this is wrong when the group
has changed.
On 16 June 2014 10:34, Romain Manni-Bucau rmannibu...@gmail.com wrote:
Cause mapping can be done through several transactions (think jaxrs) so it
would be wrong. PersistenceUtil has some good things to gey id or null if
new but IIRC some impl are broken.
Le 16 juin 2014 09:31, Thomas Andraschko andraschko.tho...@gmail.com a
écrit :
Why don't we use entityManager#contains instead of checking the ID?
2014-06-16 10:22 GMT+02:00 Karl Kildén karl.kil...@gmail.com:
Hi,
On could argue that the real problem is that the algorithm that decides
if
it should be a save or persist. It uses some portable way of deciding
this
that requires the entity to be managed.
That algorithm could be improved in each project.
@Override
@RequiresTransaction
public E save(E entity)
{
if (context.isNew(entity))
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
I would say that overriding this method would solve
EntityExistsException
in a cleaner way. For this project I have no natural keys only
generated
long so it would be a cheap and easy way to fix it... This is fully
sufficient for me:
@Override
@RequiresTransaction
public E save(E entity)
{
BaseEntity e = (BaseEntity) entity;
if (e.getId == 0)
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
If not overridden then what happens if the group has changed in my
example,
you are supposed to use entityManager and find it?
Maybe the API should provide something like the utility method I
proposed
then... I will explain it a little better. All you need to do is inject
the
groupMapper or whatever mapper you need. To get the group if it changed
you
simply call fetch with a new Group instance, the DTO with the new
information and the groupMapper. Why send in new group instance? Well
one
could also send in Group.class and use a reflection to create a new
group
if needed. But new Group() vs Group.class I actually prefer the first
because it avoids reflection. Because the new Group() argument also
allows
for getClass() the entityManager has all the info required except the
id
but that is no problem since we also send in the mapper with the handy
#getPrimaryKey
method.
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity)
entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
I have not tested this method at all, but something like it would work
well
together with the default strategy for determine save or merge... But
my
main wish now is to override the save logic actually.
On 16 June 2014 09:48, Thomas Hug thomas@gmail.com wrote:
Thanks Karl for the clarification!
If you assign a new group, I'd first make sure you have this one
persisted
as well (or we do too much in the mapper IMO). Then save the userDto
with
something like
User toEntity(User user, UserDto dto) {
...
user.setGroup(groupMapper.toEntity(dto.getGroup()); // or check
before
if ID changed
}
I guess that would even work if the group is not persisted if you
adapt
cascading.
Makes sense?
On Fri, Jun 13, 2014 at 5:56 PM, Karl Kildén karl.kil...@gmail.com
wrote:
Not sure I get myself ;)
Lets walk through how I see it:
1. User foo is created and is assigned to the preexisting group
Admin.
It goes like this in the flow: user = new UserDTO() -
*user*.setGroupDTO(selectedGroup)
- save
Some logic must make sure that we don't get EntityExistsException
trying
to
save that group.
2.
Re: [data] dto and isNew
Yes you need to merge it but the responsability is yours (user) IMO.
Le 16 juin 2014 09:56, Karl Kildén karl.kil...@gmail.com a écrit :
Hrmm maybe I mixed things up now.
If you have a relationship to another pre existing entity can it be
detached when you save? All I am really looking for is the groupId to be
updated but maybe JPA can't determine this in a good way. And updating the
entity itself is solved by including it as an argument to the mapper, all
though I am a wondering if that solution is optimal.
Romain and Thomas, your comments on the best way to handle relationships in
the Mapper? If the entity has not changed then you can just use
user.getGroup() but as I described previously this is wrong when the group
has changed.
On 16 June 2014 10:34, Romain Manni-Bucau rmannibu...@gmail.com wrote:
Cause mapping can be done through several transactions (think jaxrs) so
it
would be wrong. PersistenceUtil has some good things to gey id or null if
new but IIRC some impl are broken.
Le 16 juin 2014 09:31, Thomas Andraschko andraschko.tho...@gmail.com
a
écrit :
Why don't we use entityManager#contains instead of checking the ID?
2014-06-16 10:22 GMT+02:00 Karl Kildén karl.kil...@gmail.com:
Hi,
On could argue that the real problem is that the algorithm that
decides
if
it should be a save or persist. It uses some portable way of deciding
this
that requires the entity to be managed.
That algorithm could be improved in each project.
@Override
@RequiresTransaction
public E save(E entity)
{
if (context.isNew(entity))
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
I would say that overriding this method would solve
EntityExistsException
in a cleaner way. For this project I have no natural keys only
generated
long so it would be a cheap and easy way to fix it... This is fully
sufficient for me:
@Override
@RequiresTransaction
public E save(E entity)
{
BaseEntity e = (BaseEntity) entity;
if (e.getId == 0)
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
If not overridden then what happens if the group has changed in my
example,
you are supposed to use entityManager and find it?
Maybe the API should provide something like the utility method I
proposed
then... I will explain it a little better. All you need to do is
inject
the
groupMapper or whatever mapper you need. To get the group if it
changed
you
simply call fetch with a new Group instance, the DTO with the new
information and the groupMapper. Why send in new group instance? Well
one
could also send in Group.class and use a reflection to create a new
group
if needed. But new Group() vs Group.class I actually prefer the first
because it avoids reflection. Because the new Group() argument also
allows
for getClass() the entityManager has all the info required except the
id
but that is no problem since we also send in the mapper with the
handy
#getPrimaryKey
method.
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity)
entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
I have not tested this method at all, but something like it would
work
well
together with the default strategy for determine save or merge... But
my
main wish now is to override the save logic actually.
On 16 June 2014 09:48, Thomas Hug thomas@gmail.com wrote:
Thanks Karl for the clarification!
If you assign a new group, I'd first make sure you have this one
persisted
as well (or we do too much in the mapper IMO). Then save the
userDto
with
something like
User toEntity(User user, UserDto dto) {
...
user.setGroup(groupMapper.toEntity(dto.getGroup()); // or check
before
if ID changed
}
I guess that would even work if the group is not persisted if you
adapt
cascading.
Makes sense?
On Fri, Jun 13, 2014 at 5:56 PM, Karl Kildén
karl.kil...@gmail.com
wrote:
Not sure I get myself ;)
Lets walk through how I see it:
1. User foo is created
Re: [data] dto and isNew
But then I need to use the entityManager in the mapper or am I missing
something?
On 16 June 2014 11:17, Romain Manni-Bucau rmannibu...@gmail.com wrote:
Yes you need to merge it but the responsability is yours (user) IMO.
Le 16 juin 2014 09:56, Karl Kildén karl.kil...@gmail.com a écrit :
Hrmm maybe I mixed things up now.
If you have a relationship to another pre existing entity can it be
detached when you save? All I am really looking for is the groupId to be
updated but maybe JPA can't determine this in a good way. And updating
the
entity itself is solved by including it as an argument to the mapper, all
though I am a wondering if that solution is optimal.
Romain and Thomas, your comments on the best way to handle relationships
in
the Mapper? If the entity has not changed then you can just use
user.getGroup() but as I described previously this is wrong when the
group
has changed.
On 16 June 2014 10:34, Romain Manni-Bucau rmannibu...@gmail.com wrote:
Cause mapping can be done through several transactions (think jaxrs) so
it
would be wrong. PersistenceUtil has some good things to gey id or null
if
new but IIRC some impl are broken.
Le 16 juin 2014 09:31, Thomas Andraschko
andraschko.tho...@gmail.com
a
écrit :
Why don't we use entityManager#contains instead of checking the ID?
2014-06-16 10:22 GMT+02:00 Karl Kildén karl.kil...@gmail.com:
Hi,
On could argue that the real problem is that the algorithm that
decides
if
it should be a save or persist. It uses some portable way of
deciding
this
that requires the entity to be managed.
That algorithm could be improved in each project.
@Override
@RequiresTransaction
public E save(E entity)
{
if (context.isNew(entity))
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
I would say that overriding this method would solve
EntityExistsException
in a cleaner way. For this project I have no natural keys only
generated
long so it would be a cheap and easy way to fix it... This is fully
sufficient for me:
@Override
@RequiresTransaction
public E save(E entity)
{
BaseEntity e = (BaseEntity) entity;
if (e.getId == 0)
{
entityManager().persist(entity);
return entity;
}
return entityManager().merge(entity);
}
If not overridden then what happens if the group has changed in my
example,
you are supposed to use entityManager and find it?
Maybe the API should provide something like the utility method I
proposed
then... I will explain it a little better. All you need to do is
inject
the
groupMapper or whatever mapper you need. To get the group if it
changed
you
simply call fetch with a new Group instance, the DTO with the new
information and the groupMapper. Why send in new group instance?
Well
one
could also send in Group.class and use a reflection to create a new
group
if needed. But new Group() vs Group.class I actually prefer the
first
because it avoids reflection. Because the new Group() argument also
allows
for getClass() the entityManager has all the info required except
the
id
but that is no problem since we also send in the mapper with the
handy
#getPrimaryKey
method.
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity)
entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
I have not tested this method at all, but something like it would
work
well
together with the default strategy for determine save or merge...
But
my
main wish now is to override the save logic actually.
On 16 June 2014 09:48, Thomas Hug thomas@gmail.com wrote:
Thanks Karl for the clarification!
If you assign a new group, I'd first make sure you have this one
persisted
as well (or we do too much in the mapper IMO). Then save the
userDto
with
something like
User toEntity(User user, UserDto dto) {
...
user.setGroup(groupMapper.toEntity(dto.getGroup()); // or
check
before
if ID changed
}
Re: [data] dto and isNew
I wrote a response to the users list but not sure it came through. It kinda
belongs in this thread so here it goes.
So I ran into issues with the DTO mapper api and voiced my concerns in irc.
I saw this discussion and now I am trying the solution present in the
current SNAPSHOT. However I have one comment / question:
What if my Entity has a relationship to another Entity?
Like this:
public class User extends BaseAuditEntity {
@Column
private String name;
@Column
@ManyToOne
@JoinColumn(name=group_id)
private Group group;
This means my userDTO may come with a GroupDTO and how do I map that
relationship? Or to explain by code please fill in the question marks below
;) or if you feel my implementation is weird then I would gladly accept
comments on that too. But the way I see it I need to @Inject the
GroupMapper, use the EntityManager to find the Group then call
groupMapper.toEntity and then I think to myself that the API is worse now
because before I could call groupMapper.toEntity with only a dto argument.
At that point you had to use the entitymanager anyways to find yourself
and that feels clean compared to find your entities you form relationships
with.
@Override
protected User toEntity(final User user, final UserDTO userDTO) {
MapperUtil.toAuditEntity(user, userDTO);
user.setName(userDTO.getName());
user.setEmail(userDTO.getEmail());
user.setLocale(userDTO.getLocale());
user.setGroup(*?*);
return user;
}
Cheers / Karl
On 17 May 2014 16:40, Romain Manni-Bucau rmannibu...@gmail.com wrote:
Yep, missed it.
Works for me. Maybe the name should be closer to other methods,
mapKey? But whatever you choose as name this solution works :).
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-05-17 15:54 GMT+02:00 Thomas Hug thomas@gmail.com:
It's the PK, not the Entity ;) In SimpleQueryInOutMapperBase, it could be
something like
@Inject
private QueryInvocationContext context;
protected abstract Object getPrimaryKey(Dto dto);
protected E findEntity(Object pk)
{
return (E) context.getEntityManager().find(context.getEntityClass(),
pk);
}
@Override
public Object mapParameter(final Object parameter)
{
Object pk = getPrimaryKey((Dto) parameter);
if (pk != null)
{
E entity = findEntity(pk);
return toEntity(entity, (Dto) parameter);
}
return toEntity(newEntity(), (Dto) parameter);
}
On Sat, May 17, 2014 at 1:57 PM, Romain Manni-Bucau
rmannibu...@gmail.comwrote:
would work while return is E and not Object ;)
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-05-17 11:47 GMT+02:00 Thomas Hug thomas@gmail.com:
Or a protected abstract Object getPrimaryKey(Dto dto). We can get the
EM
over an injected QueryInvocationContext.
On Thu, May 15, 2014 at 9:06 PM, Romain Manni-Bucau
rmannibu...@gmail.comwrote:
I think a protected findEntity(id) in the mapper can be enough.
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-05-07 22:29 GMT+02:00 Thomas Hug thomas@gmail.com:
Hi Romain,
See your point. But if we only get the DTO - with what would we
call
the
find? Could even be that the PK is a DTO or encoded / encrypted and
needs
to go through the mapper first. Maybe we can provide some
convenience
methods in the base mapper?
On Tue, May 6, 2014 at 7:41 PM, Romain Manni-Bucau
rmannibu...@gmail.comwrote:
Hi guys,
DTO feature is awesome but doesn't work in update mode since isNew
doesn't use a managed entity.
When using a mapper we should call find and pass it to the mapper
(or
create a new unmanaged entity if not found). So mapper signature
should be Entity toEntity(Entity, DTO) no?
Otherwise users need to do the find in the mapper...almost
eveytime.
wdyt?
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
Re: [data] dto and isNew
Hey Karl
Sorry missed your mail indeed - it's probably time I subscribe to the user
mailing list too :-)
You can still chain those mappers together, but I agree the casting and
wiring is clunky. As options I see:
- we add a public E toEntity(Dto dto) back which basically does the same as
mapParameter now (just hides the casting) for new entities
- we make the E toEntity(Entity e, Dto dto) public as well, so it's quite
easy to chain mapper calls for updates
groupMapper.toEntity(user.getGroup(), userDto.getGroup());
You will still have to have some conditionals to see which one to call,
also depends on your data model (required relations, lazy or eager fetch).
How does that look? Better ideas?
On Fri, Jun 13, 2014 at 8:42 AM, Karl Kildén karl.kil...@gmail.com wrote:
I wrote a response to the users list but not sure it came through. It kinda
belongs in this thread so here it goes.
So I ran into issues with the DTO mapper api and voiced my concerns in irc.
I saw this discussion and now I am trying the solution present in the
current SNAPSHOT. However I have one comment / question:
What if my Entity has a relationship to another Entity?
Like this:
public class User extends BaseAuditEntity {
@Column
private String name;
@Column
@ManyToOne
@JoinColumn(name=group_id)
private Group group;
This means my userDTO may come with a GroupDTO and how do I map that
relationship? Or to explain by code please fill in the question marks below
;) or if you feel my implementation is weird then I would gladly accept
comments on that too. But the way I see it I need to @Inject the
GroupMapper, use the EntityManager to find the Group then call
groupMapper.toEntity and then I think to myself that the API is worse now
because before I could call groupMapper.toEntity with only a dto argument.
At that point you had to use the entitymanager anyways to find yourself
and that feels clean compared to find your entities you form relationships
with.
@Override
protected User toEntity(final User user, final UserDTO userDTO) {
MapperUtil.toAuditEntity(user, userDTO);
user.setName(userDTO.getName());
user.setEmail(userDTO.getEmail());
user.setLocale(userDTO.getLocale());
user.setGroup(*?*);
return user;
}
Cheers / Karl
On 17 May 2014 16:40, Romain Manni-Bucau rmannibu...@gmail.com wrote:
Yep, missed it.
Works for me. Maybe the name should be closer to other methods,
mapKey? But whatever you choose as name this solution works :).
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-05-17 15:54 GMT+02:00 Thomas Hug thomas@gmail.com:
It's the PK, not the Entity ;) In SimpleQueryInOutMapperBase, it could
be
something like
@Inject
private QueryInvocationContext context;
protected abstract Object getPrimaryKey(Dto dto);
protected E findEntity(Object pk)
{
return (E)
context.getEntityManager().find(context.getEntityClass(),
pk);
}
@Override
public Object mapParameter(final Object parameter)
{
Object pk = getPrimaryKey((Dto) parameter);
if (pk != null)
{
E entity = findEntity(pk);
return toEntity(entity, (Dto) parameter);
}
return toEntity(newEntity(), (Dto) parameter);
}
On Sat, May 17, 2014 at 1:57 PM, Romain Manni-Bucau
rmannibu...@gmail.comwrote:
would work while return is E and not Object ;)
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-05-17 11:47 GMT+02:00 Thomas Hug thomas@gmail.com:
Or a protected abstract Object getPrimaryKey(Dto dto). We can get
the
EM
over an injected QueryInvocationContext.
On Thu, May 15, 2014 at 9:06 PM, Romain Manni-Bucau
rmannibu...@gmail.comwrote:
I think a protected findEntity(id) in the mapper can be enough.
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-05-07 22:29 GMT+02:00 Thomas Hug thomas@gmail.com:
Hi Romain,
See your point. But if we only get the DTO - with what would we
call
the
find? Could even be that the PK is a DTO or encoded / encrypted
and
needs
to go through the mapper first. Maybe we can provide some
convenience
methods in the base mapper?
On Tue, May 6, 2014 at 7:41 PM, Romain Manni-Bucau
rmannibu...@gmail.comwrote:
Hi guys,
DTO feature is awesome but doesn't work in update mode since
isNew
doesn't use a managed entity.
When
Re: [data] dto and isNew
Hi and hrmmm,
What if user group was changed? groupMapper.toEntity(user.getGroup(),
userDto.getGroup()); This would then operate on stale data unless you fetch
and send in correct group. And managing new groups is easy for me I think,
I would call the method using groupMapper.toEntity(new Group(),
userDto.getGroup());
I would much prefer all three methods to be non protected. Then I could
create my helper something along the lines of this:
I did not test this very well but unless I thought wrong completely it
would be a one time thing to implement. But using mappers from mappers are
hard because if the relationship is declared in both ways you can get
circular references. Anyways here's my helper I theorycrafted together:
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity) entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
One could always create their own base class overriding the protected
methods and adding that fetch helper I guess...
cheers
On 13 June 2014 12:54, Thomas Hug thomas@gmail.com wrote:
Hey Karl
Sorry missed your mail indeed - it's probably time I subscribe to the user
mailing list too :-)
You can still chain those mappers together, but I agree the casting and
wiring is clunky. As options I see:
- we add a public E toEntity(Dto dto) back which basically does the same as
mapParameter now (just hides the casting) for new entities
- we make the E toEntity(Entity e, Dto dto) public as well, so it's quite
easy to chain mapper calls for updates
groupMapper.toEntity(user.getGroup(), userDto.getGroup());
You will still have to have some conditionals to see which one to call,
also depends on your data model (required relations, lazy or eager fetch).
How does that look? Better ideas?
On Fri, Jun 13, 2014 at 8:42 AM, Karl Kildén karl.kil...@gmail.com
wrote:
I wrote a response to the users list but not sure it came through. It
kinda
belongs in this thread so here it goes.
So I ran into issues with the DTO mapper api and voiced my concerns in
irc.
I saw this discussion and now I am trying the solution present in the
current SNAPSHOT. However I have one comment / question:
What if my Entity has a relationship to another Entity?
Like this:
public class User extends BaseAuditEntity {
@Column
private String name;
@Column
@ManyToOne
@JoinColumn(name=group_id)
private Group group;
This means my userDTO may come with a GroupDTO and how do I map that
relationship? Or to explain by code please fill in the question marks
below
;) or if you feel my implementation is weird then I would gladly accept
comments on that too. But the way I see it I need to @Inject the
GroupMapper, use the EntityManager to find the Group then call
groupMapper.toEntity and then I think to myself that the API is worse now
because before I could call groupMapper.toEntity with only a dto
argument.
At that point you had to use the entitymanager anyways to find yourself
and that feels clean compared to find your entities you form
relationships
with.
@Override
protected User toEntity(final User user, final UserDTO userDTO) {
MapperUtil.toAuditEntity(user, userDTO);
user.setName(userDTO.getName());
user.setEmail(userDTO.getEmail());
user.setLocale(userDTO.getLocale());
user.setGroup(*?*);
return user;
}
Cheers / Karl
On 17 May 2014 16:40, Romain Manni-Bucau rmannibu...@gmail.com wrote:
Yep, missed it.
Works for me. Maybe the name should be closer to other methods,
mapKey? But whatever you choose as name this solution works :).
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-05-17 15:54 GMT+02:00 Thomas Hug thomas@gmail.com:
It's the PK, not the Entity ;) In SimpleQueryInOutMapperBase, it
could
be
something like
@Inject
private QueryInvocationContext context;
protected abstract Object getPrimaryKey(Dto dto);
protected E findEntity(Object pk)
{
return (E)
context.getEntityManager().find(context.getEntityClass(),
pk);
}
@Override
public Object mapParameter(final Object parameter)
{
Object pk = getPrimaryKey((Dto) parameter);
if (pk != null)
{
E entity = findEntity(pk);
return toEntity(entity, (Dto) parameter);
}
return toEntity(newEntity(), (Dto) parameter);
}
Re: [data] dto and isNew
Hi Karl
Sorry not sure if I get you right. Why would user.getGroup() be stale? As
in the update case user has just been fetched by the PK, user.getGroup()
should lazyload the group - right?
On Fri, Jun 13, 2014 at 2:51 PM, Karl Kildén karl.kil...@gmail.com wrote:
Hi and hrmmm,
What if user group was changed? groupMapper.toEntity(user.getGroup(),
userDto.getGroup()); This would then operate on stale data unless you fetch
and send in correct group. And managing new groups is easy for me I think,
I would call the method using groupMapper.toEntity(new Group(),
userDto.getGroup());
I would much prefer all three methods to be non protected. Then I could
create my helper something along the lines of this:
I did not test this very well but unless I thought wrong completely it
would be a one time thing to implement. But using mappers from mappers are
hard because if the relationship is declared in both ways you can get
circular references. Anyways here's my helper I theorycrafted together:
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity) entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
One could always create their own base class overriding the protected
methods and adding that fetch helper I guess...
cheers
On 13 June 2014 12:54, Thomas Hug thomas@gmail.com wrote:
Hey Karl
Sorry missed your mail indeed - it's probably time I subscribe to the
user
mailing list too :-)
You can still chain those mappers together, but I agree the casting and
wiring is clunky. As options I see:
- we add a public E toEntity(Dto dto) back which basically does the same
as
mapParameter now (just hides the casting) for new entities
- we make the E toEntity(Entity e, Dto dto) public as well, so it's quite
easy to chain mapper calls for updates
groupMapper.toEntity(user.getGroup(), userDto.getGroup());
You will still have to have some conditionals to see which one to call,
also depends on your data model (required relations, lazy or eager
fetch).
How does that look? Better ideas?
On Fri, Jun 13, 2014 at 8:42 AM, Karl Kildén karl.kil...@gmail.com
wrote:
I wrote a response to the users list but not sure it came through. It
kinda
belongs in this thread so here it goes.
So I ran into issues with the DTO mapper api and voiced my concerns in
irc.
I saw this discussion and now I am trying the solution present in the
current SNAPSHOT. However I have one comment / question:
What if my Entity has a relationship to another Entity?
Like this:
public class User extends BaseAuditEntity {
@Column
private String name;
@Column
@ManyToOne
@JoinColumn(name=group_id)
private Group group;
This means my userDTO may come with a GroupDTO and how do I map that
relationship? Or to explain by code please fill in the question marks
below
;) or if you feel my implementation is weird then I would gladly accept
comments on that too. But the way I see it I need to @Inject the
GroupMapper, use the EntityManager to find the Group then call
groupMapper.toEntity and then I think to myself that the API is worse
now
because before I could call groupMapper.toEntity with only a dto
argument.
At that point you had to use the entitymanager anyways to find
yourself
and that feels clean compared to find your entities you form
relationships
with.
@Override
protected User toEntity(final User user, final UserDTO userDTO) {
MapperUtil.toAuditEntity(user, userDTO);
user.setName(userDTO.getName());
user.setEmail(userDTO.getEmail());
user.setLocale(userDTO.getLocale());
user.setGroup(*?*);
return user;
}
Cheers / Karl
On 17 May 2014 16:40, Romain Manni-Bucau rmannibu...@gmail.com
wrote:
Yep, missed it.
Works for me. Maybe the name should be closer to other methods,
mapKey? But whatever you choose as name this solution works :).
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-05-17 15:54 GMT+02:00 Thomas Hug thomas@gmail.com:
It's the PK, not the Entity ;) In SimpleQueryInOutMapperBase, it
could
be
something like
@Inject
private QueryInvocationContext context;
protected abstract Object getPrimaryKey(Dto dto);
protected E findEntity(Object pk)
{
return (E)
Re: [data] dto and isNew
Not sure I get myself ;)
Lets walk through how I see it:
1. User foo is created and is assigned to the preexisting group Admin.
It goes like this in the flow: user = new UserDTO() -
*user*.setGroupDTO(selectedGroup)
- save
Some logic must make sure that we don't get EntityExistsException trying to
save that group.
2. Many sessions later user foo is edited. Flow: *user*.setGroupDTO(newGroup)
- save
The variable *user *is a random object that happens to be the latest
version of that user that also has a new group set.
So *PK, user.getGroup()*
*should lazyload the group - right? *This will result in the previous group
being loaded unless I am missing something. While it is technically correct
since the new group relationship has not been persisted yet it is actually
impossible to ever update group with this flow
On 13 June 2014 17:09, Thomas Hug thomas@gmail.com wrote:
Hi Karl
Sorry not sure if I get you right. Why would user.getGroup() be stale? As
in the update case user has just been fetched by the PK, user.getGroup()
should lazyload the group - right?
On Fri, Jun 13, 2014 at 2:51 PM, Karl Kildén karl.kil...@gmail.com
wrote:
Hi and hrmmm,
What if user group was changed? groupMapper.toEntity(user.getGroup(),
userDto.getGroup()); This would then operate on stale data unless you
fetch
and send in correct group. And managing new groups is easy for me I
think,
I would call the method using groupMapper.toEntity(new Group(),
userDto.getGroup());
I would much prefer all three methods to be non protected. Then I could
create my helper something along the lines of this:
I did not test this very well but unless I thought wrong completely it
would be a one time thing to implement. But using mappers from mappers
are
hard because if the relationship is declared in both ways you can get
circular references. Anyways here's my helper I theorycrafted together:
Group g = fetch (new Group(), user.getGroup(), groupMapper);
public Entity, Dto Entity fetch(Entity entity, Dto dto,
SimpleQueryInOutMapperBaseEntity, Dto entityMapper){
Object pk = entityMapper.getPrimaryKey(dto);
Entity foundEntity = (Entity)
entityManager.find(entity.getClass(),
pk);
if (foundEntity == null) {
foundEntity = entity;
}
return entityMapper.toEntity(foundEntity, dto);
}
One could always create their own base class overriding the protected
methods and adding that fetch helper I guess...
cheers
On 13 June 2014 12:54, Thomas Hug thomas@gmail.com wrote:
Hey Karl
Sorry missed your mail indeed - it's probably time I subscribe to the
user
mailing list too :-)
You can still chain those mappers together, but I agree the casting and
wiring is clunky. As options I see:
- we add a public E toEntity(Dto dto) back which basically does the
same
as
mapParameter now (just hides the casting) for new entities
- we make the E toEntity(Entity e, Dto dto) public as well, so it's
quite
easy to chain mapper calls for updates
groupMapper.toEntity(user.getGroup(), userDto.getGroup());
You will still have to have some conditionals to see which one to call,
also depends on your data model (required relations, lazy or eager
fetch).
How does that look? Better ideas?
On Fri, Jun 13, 2014 at 8:42 AM, Karl Kildén karl.kil...@gmail.com
wrote:
I wrote a response to the users list but not sure it came through. It
kinda
belongs in this thread so here it goes.
So I ran into issues with the DTO mapper api and voiced my concerns
in
irc.
I saw this discussion and now I am trying the solution present in the
current SNAPSHOT. However I have one comment / question:
What if my Entity has a relationship to another Entity?
Like this:
public class User extends BaseAuditEntity {
@Column
private String name;
@Column
@ManyToOne
@JoinColumn(name=group_id)
private Group group;
This means my userDTO may come with a GroupDTO and how do I map that
relationship? Or to explain by code please fill in the question marks
below
;) or if you feel my implementation is weird then I would gladly
accept
comments on that too. But the way I see it I need to @Inject the
GroupMapper, use the EntityManager to find the Group then call
groupMapper.toEntity and then I think to myself that the API is worse
now
because before I could call groupMapper.toEntity with only a dto
argument.
At that point you had to use the entitymanager anyways to find
yourself
and that feels clean compared to find your entities you form
relationships
with.
@Override
protected User toEntity(final User user, final UserDTO userDTO) {
MapperUtil.toAuditEntity(user, userDTO);
Re: [data] dto and isNew
Or a protected abstract Object getPrimaryKey(Dto dto). We can get the EM over an injected QueryInvocationContext. On Thu, May 15, 2014 at 9:06 PM, Romain Manni-Bucau rmannibu...@gmail.comwrote: I think a protected findEntity(id) in the mapper can be enough. Romain Manni-Bucau Twitter: @rmannibucau Blog: http://rmannibucau.wordpress.com/ LinkedIn: http://fr.linkedin.com/in/rmannibucau Github: https://github.com/rmannibucau 2014-05-07 22:29 GMT+02:00 Thomas Hug thomas@gmail.com: Hi Romain, See your point. But if we only get the DTO - with what would we call the find? Could even be that the PK is a DTO or encoded / encrypted and needs to go through the mapper first. Maybe we can provide some convenience methods in the base mapper? On Tue, May 6, 2014 at 7:41 PM, Romain Manni-Bucau rmannibu...@gmail.comwrote: Hi guys, DTO feature is awesome but doesn't work in update mode since isNew doesn't use a managed entity. When using a mapper we should call find and pass it to the mapper (or create a new unmanaged entity if not found). So mapper signature should be Entity toEntity(Entity, DTO) no? Otherwise users need to do the find in the mapper...almost eveytime. wdyt? Romain Manni-Bucau Twitter: @rmannibucau Blog: http://rmannibucau.wordpress.com/ LinkedIn: http://fr.linkedin.com/in/rmannibucau Github: https://github.com/rmannibucau
Re: [data] dto and isNew
It's the PK, not the Entity ;) In SimpleQueryInOutMapperBase, it could be
something like
@Inject
private QueryInvocationContext context;
protected abstract Object getPrimaryKey(Dto dto);
protected E findEntity(Object pk)
{
return (E) context.getEntityManager().find(context.getEntityClass(),
pk);
}
@Override
public Object mapParameter(final Object parameter)
{
Object pk = getPrimaryKey((Dto) parameter);
if (pk != null)
{
E entity = findEntity(pk);
return toEntity(entity, (Dto) parameter);
}
return toEntity(newEntity(), (Dto) parameter);
}
On Sat, May 17, 2014 at 1:57 PM, Romain Manni-Bucau
rmannibu...@gmail.comwrote:
would work while return is E and not Object ;)
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-05-17 11:47 GMT+02:00 Thomas Hug thomas@gmail.com:
Or a protected abstract Object getPrimaryKey(Dto dto). We can get the EM
over an injected QueryInvocationContext.
On Thu, May 15, 2014 at 9:06 PM, Romain Manni-Bucau
rmannibu...@gmail.comwrote:
I think a protected findEntity(id) in the mapper can be enough.
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-05-07 22:29 GMT+02:00 Thomas Hug thomas@gmail.com:
Hi Romain,
See your point. But if we only get the DTO - with what would we call
the
find? Could even be that the PK is a DTO or encoded / encrypted and
needs
to go through the mapper first. Maybe we can provide some convenience
methods in the base mapper?
On Tue, May 6, 2014 at 7:41 PM, Romain Manni-Bucau
rmannibu...@gmail.comwrote:
Hi guys,
DTO feature is awesome but doesn't work in update mode since isNew
doesn't use a managed entity.
When using a mapper we should call find and pass it to the mapper (or
create a new unmanaged entity if not found). So mapper signature
should be Entity toEntity(Entity, DTO) no?
Otherwise users need to do the find in the mapper...almost eveytime.
wdyt?
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
[data] dto and isNew
Hi guys, DTO feature is awesome but doesn't work in update mode since isNew doesn't use a managed entity. When using a mapper we should call find and pass it to the mapper (or create a new unmanaged entity if not found). So mapper signature should be Entity toEntity(Entity, DTO) no? Otherwise users need to do the find in the mapper...almost eveytime. wdyt? Romain Manni-Bucau Twitter: @rmannibucau Blog: http://rmannibucau.wordpress.com/ LinkedIn: http://fr.linkedin.com/in/rmannibucau Github: https://github.com/rmannibucau
