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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> 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 <[email protected]> > > > 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 <[email protected]> > > 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 <[email protected]>: > > > > > > 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 > > > > > > <[email protected]>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 <[email protected]>: > > > > > >> > 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 > > > > > >> > <[email protected]>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 <[email protected] > >: > > > > > >> >> > 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 < > > > > > >> >> [email protected]>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 > > > > > >> >> >> > > > > > >> >> > > > > > >> > > > > > > > > > > > > > > >
