Yes, but that s not an issue since you can get it injected Le lundi 16 juin 2014, Karl Kildén <[email protected]> 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 <[email protected]> wrote: > >> Yes you need to merge it but the responsability is yours (user) IMO. >> Le 16 juin 2014 09:56, "Karl Kildén" <[email protected]> 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 <[email protected]> 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" < >> [email protected]> >> > 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 <[email protected]>: >> > > > >> > > > > 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 <[email protected]> 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 < >> > [email protected]> >> > > > > > 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 <[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 >> > > > > > > > > > > > >> >> >> >> > > > > > > > > > > > >> >> >> > > > > > > > > > > > >> >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >> >
-- Romain Manni-Bucau Twitter: @rmannibucau Blog: http://rmannibucau.wordpress.com/ LinkedIn: http://fr.linkedin.com/in/rmannibucau Github: https://github.com/rmannibucau
