You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltaspike.apache.org by Karl Kildén <ka...@gmail.com> on 2014/08/18 18:08:51 UTC

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 <th...@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 <rmannibucau@gmail.com
> >
> wrote:
>
> > Yes, but that s not an issue since you can get it injected
> >
> > Le lundi 16 juin 2014, Karl Kildén <ka...@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 <rm...@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" <ka...@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 <rm...@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.thomas@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 <ka...@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,
> > >> > > > > SimpleQueryInOutMapperBase<Entity, 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 <th...@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.kilden@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 <th...@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.kilden@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,
> > >> > > > > > > > > SimpleQueryInOutMapperBase<Entity, 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.hug@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.kilden@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 <
> > >> > > > > rmannibucau@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.hug@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
> > >> > > > > > > > > > > > > <rm...@gmail.com>wrote:
> > >> > > > > > > > > > > > >
> > >> > > > > > > > > > > > >> 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.hug@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
> > >> > > > > > > > > > > > >> > <rm...@gmail.com>wrote:
> > >> > > > > > > > > > > > >> >
> > >> > > > > > > > > > > > >> >> 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.hug@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
> > >> > > > <
> > >> > > > > > > > > > > > >> >> rmannibucau@gmail.com>wrote:
> > >> > > > > > > > > > > > >> >> >
> > >> > > > > > > > > > > > >> >> >> 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
> > >> > > > > > > > > > > > >> >> >>
> > >> > > > > > > > > > > > >> >>
> > >> > > > > > > > > > > > >>
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > >
> > >> > > > > > > > > >
> > >> > > > > > > > >
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> >
> > --
> >
> >
> > 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

Posted by Karl Kildén <ka...@gmail.com>.
Yes please! :-)


On 18 August 2014 20:22, Romain Manni-Bucau <rm...@gmail.com> wrote:

> 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 <ka...@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 <th...@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 <
> rmannibucau@gmail.com
> >> >
> >> wrote:
> >>
> >> > Yes, but that s not an issue since you can get it injected
> >> >
> >> > Le lundi 16 juin 2014, Karl Kildén <ka...@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 <rm...@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" <ka...@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 <rmannibucau@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.thomas@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.kilden@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,
> >> > >> > > > > SimpleQueryInOutMapperBase<Entity, 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 <th...@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.kilden@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.hug@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.kilden@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,
> >> > >> > > > > > > > > SimpleQueryInOutMapperBase<Entity, 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.hug@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.kilden@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 <
> >> > >> > > > > rmannibucau@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.hug@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
> >> > >> > > > > > > > > > > > > <rm...@gmail.com>wrote:
> >> > >> > > > > > > > > > > > >
> >> > >> > > > > > > > > > > > >> 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.hug@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
> >> > >> > > > > > > > > > > > >> > <rm...@gmail.com>wrote:
> >> > >> > > > > > > > > > > > >> >
> >> > >> > > > > > > > > > > > >> >> 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.hug@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
> >> > >> > > > <
> >> > >> > > > > > > > > > > > >> >> rmannibucau@gmail.com>wrote:
> >> > >> > > > > > > > > > > > >> >> >
> >> > >> > > > > > > > > > > > >> >> >> 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
> >> > >> > > > > > > > > > > > >> >> >>
> >> > >> > > > > > > > > > > > >> >>
> >> > >> > > > > > > > > > > > >>
> >> > >> > > > > > > > > > > >
> >> > >> > > > > > > > > > >
> >> > >> > > > > > > > > >
> >> > >> > > > > > > > >
> >> > >> > > > > > > >
> >> > >> > > > > > >
> >> > >> > > > > >
> >> > >> > > > >
> >> > >> > > >
> >> > >> > >
> >> > >> >
> >> > >>
> >> > >
> >> >
> >> > --
> >> >
> >> >
> >> > 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

Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <ka...@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 <th...@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 <rmannibucau@gmail.com
>> >
>> wrote:
>>
>> > Yes, but that s not an issue since you can get it injected
>> >
>> > Le lundi 16 juin 2014, Karl Kildén <ka...@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 <rm...@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" <ka...@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 <rm...@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.thomas@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 <ka...@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,
>> > >> > > > > SimpleQueryInOutMapperBase<Entity, 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 <th...@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.kilden@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 <th...@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.kilden@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,
>> > >> > > > > > > > > SimpleQueryInOutMapperBase<Entity, 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.hug@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.kilden@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 <
>> > >> > > > > rmannibucau@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.hug@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
>> > >> > > > > > > > > > > > > <rm...@gmail.com>wrote:
>> > >> > > > > > > > > > > > >
>> > >> > > > > > > > > > > > >> 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.hug@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
>> > >> > > > > > > > > > > > >> > <rm...@gmail.com>wrote:
>> > >> > > > > > > > > > > > >> >
>> > >> > > > > > > > > > > > >> >> 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.hug@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
>> > >> > > > <
>> > >> > > > > > > > > > > > >> >> rmannibucau@gmail.com>wrote:
>> > >> > > > > > > > > > > > >> >> >
>> > >> > > > > > > > > > > > >> >> >> 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
>> > >> > > > > > > > > > > > >> >> >>
>> > >> > > > > > > > > > > > >> >>
>> > >> > > > > > > > > > > > >>
>> > >> > > > > > > > > > > >
>> > >> > > > > > > > > > >
>> > >> > > > > > > > > >
>> > >> > > > > > > > >
>> > >> > > > > > > >
>> > >> > > > > > >
>> > >> > > > > >
>> > >> > > > >
>> > >> > > >
>> > >> > >
>> > >> >
>> > >>
>> > >
>> >
>> > --
>> >
>> >
>> > Romain Manni-Bucau
>> > Twitter: @rmannibucau
>> > Blog: http://rmannibucau.wordpress.com/
>> > LinkedIn: http://fr.linkedin.com/in/rmannibucau
>> > Github: https://github.com/rmannibucau
>> >
>>