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,
> > 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 <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
> > > > > > 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....@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);
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On Sat, May 17, 2014 at 1:57 PM, Romain Manni-Bucau
> > > > > > > > > > <rmannibu...@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....@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.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....@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.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
> > > > > > > > > >> >> >>
> > > > > > > > > >> >>
> > > > > > > > > >>
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
