4

I have two entities mapped in my application in order to model a Team and its Members. A Team can have many Members and a Member can belong to no more than one Team. Everything is fine about handling this concepts. The problem comes when I try to move a Member from one existing Team to another.

The entities are presented below, in simplified form. The very last method, transfer(), is the one that should perform the removal of a certain Member from its Team and send it to another one.

@Entity
public class Member extends Person {
    @ManyToOne
    private Team team;

    protected Member() {
        super();
    }

    public Member(Team team, String name) {
        super(name);
        this.team = team;
    }

    // Trivial getters and setters...

    public Team getTeam() {
        return team;
    }

    protected void setTeam(Team team) {
        this.team = team;
    }
}

@Entity
public class Team {
    @Id
    private long id;
    private String name;
    @OneToMany(mappedBy="team", cascade=CascadeType.ALL)
    private List<Member> members = new ArrayList<Member>();

    protected Team() {
    }

    public Team(String name) {
        this.name = name;
    }

    // trivial getters and setters...

    public Member addMember(String name) {
        Member member = new Member(this, name); 
        members.add(member);
        return member;
    }

    protected void addMember(Member member) {
        members.add(member);
        member.setTeam(this);
    }

    public void removeMember(Member member) {
        members.remove(member);
    }

    public Member memberByName(String memberName) {
        for(Member member : members)
          if(member.getName().equals(memberName))
              return member;
        return null;
    }

    public Collection<Members> getMembers() {
        return Collections.unmodifiableCollection(members);
    }

    public void transfer(Member member, Team destination) {
        members.remove(member);
        destination.addMember(member);
    }
}

I have this unit test code that is intended to validate the transfer service

Team teamA = teamRepository.teamById(idTeamA);
Team teamB = teamRepository.teamById(idTeamB);
Team teamC = teamRepository.teamById(idTeamC);

Member zaki = teamA.memberByName("Zaki");
Member denise = teamA.memberByName("Denise");

EntityTransaction t = teamRepository.transactionBegin();
teamA.transfer(zaki, teamB);
teamA.transferir(denise, teamC);
t.commit();

I have the following exception in the commit() line

javax.persistence.PersistenceException: org.hibernate.PersistentObjectException: detached entity passed to persist: application.domain.Member

Any ideas?

UPDATE 1:

I decided to perform a little test and changed the code of the transfer() method as follows

public void transfer(Member member, Team destination) {
    member.setTeam(this);
}

The result was curious: no error, but also no update on the tables. Hibernate couldn't track the update and the transfer simply didn't happen.

UPDATE 2:

I decided to give it a try on the suggestion from Steve K and changed the transfer() method to the following:

    public void transfer(Member member, Team destination) {
        destination.addMember(member);
        members.remove(member);
    }

Looking the addMember() and removeMember() methods (below) we see that the Team is begin updated too.

    protected void addMember(Member member) {
        members.add(member);
        member.setTeam(this);
    }

    public void removeMember(Member member) {
        members.remove(member);
    }

So, the member is being added to the destination collection, its Team is being set to the destination Team and then the member is being removed from the current collection (current Team).

The test case was changed to

EntityTransaction t = teamRepository.transactionBegin();
teamA.transfer(zaki, teamB);
teamA.getEntityManager().refresh(teamA); // I get an exception here!!!
t.commit();

In the refresh() line I have the following exception:

javax.persistence.PersistenceException: org.hibernate.PersistentObjectException: detached entity passed to persist: domain.Member
    at org.hibernate.jpa.spi.AbstractEntityManagerImpl.convert(AbstractEntityManagerImpl.java:1763)
        // ... many calls
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
Caused by: org.hibernate.PersistentObjectException: detached entity passed to persist: domain.Member
    at org.hibernate.event.internal.DefaultPersistEventListener.onPersist(DefaultPersistEventListener.java:139)
        // ... many calls
    at org.hibernate.jpa.spi.AbstractEntityManagerImpl.flush(AbstractEntityManagerImpl.java:1335)
    ... 28 more

It seems, after all, that transfering instances from one collection to another (that are implementing a simple aggregation) is not supported in Hibernate!

Vlad Mihalcea
  • 142,745
  • 71
  • 566
  • 911
AlexSC
  • 1,823
  • 3
  • 28
  • 54
  • You don't show us your `Team.members()` method, so I need to guess: it returns a `new Member` that isn't part of your persistence context. This would also explain the behavior you describe in your update. – mabi Nov 19 '14 at 13:41
  • I just updated the code to show the method `memberByName()` (formerly `member()`). It just search for a certain member in the team and returns it. – AlexSC Nov 19 '14 at 13:52
  • What happens if you move the `EntityTransaction` before your `teamById` calls? Also @dpawel is correct that modifying anything with a `mappedBy` attribute will not get picked up by hibernate. – mabi Nov 19 '14 at 15:29
  • @mabi: nothing changes if I change the code to start the transaction before loading the teams. You said that the `mappedBy` attribute is problem here. Why is that? Do you have any advices in order to fix the problem? Should I understand that Hibernate can't support the operation of moving an item from a collection to another? – AlexSC Nov 20 '14 at 09:40
  • How are you doing the transfer call? Why your transfer method receive the Team destination ? better set that argument into the member? – Guilherme Dec 16 '14 at 11:50
  • @Forcecoder: The transfer method receives the item to be transfered and the transfer destination, which means, another Tem instance. Them member class does not have a `setTeam()` method, since I prefer to make that by using a specific method (the transfer method). However, inside the transfer method what I do is to remove the member being transfered from the current collection and to add it to the destination collection. In the process I set the new `Team` as well. – AlexSC Dec 16 '14 at 13:03
  • Did you use inverse=true for members set in mapping? – M.G.E Dec 18 '14 at 10:41
  • @TommyMoore: I used `mappedBy` and `cascade`, as the code shows. The mapping was made by JPA annotations, not in Hibernate (XML or fluent). Since I didn't deeply study Hibernate specific configurations, I don't know if `mappedBy` and `inverse=true` are the same. – AlexSC Dec 18 '14 at 11:59
  • @AlexSC according to [this](http://stackoverflow.com/a/10095033/3903430) they're the same. so give it a try and let's see what happen in both parent or children's add or remove scenarios. – M.G.E Dec 18 '14 at 12:16
  • @TommyMoore: thanks, but if you look closely the code I posted in the question, you will see the presence of `mappedBy` in `Team`, which means that the FK must be in `Member`, what is the fact. Notice that all the other persistence operations work correctly, so the mapping is fine. The problem is specifically the operation of removing one `Member` from one `Team` and adding it to another `Team`, what I call a *member transfering*. The problem is that when I remove a `Member` from the members collection, Hibernate considers it an orphan and deletes it instead changing the member's `Team`. – AlexSC Dec 18 '14 at 13:21
  • You probably need to change your addMember method to set the member's team first, and then add the (now updated) member to this team's list of members. – Steve K Dec 21 '14 at 21:53

4 Answers4

3

Your mappings and data access logic are just fine.

You need to move the transaction boundary at the beginning of the code:

EntityTransaction t = teamRepository.transactionBegin();

Team teamA = teamRepository.teamById(idTeamA);
...
teamA.transferir(denise, teamC);
t.commit();

But the exception you got it's not from the code you've shown us:

javax.persistence.PersistenceException: org.hibernate.PersistentObjectException: detached entity passed to persist: application.domain.Member

There might be a pending persist action being queued up and only translated during flush time (commit time in your case). So you need to activate the SQL logs being executed and try finding out the detached entity.

Vlad Mihalcea
  • 142,745
  • 71
  • 566
  • 911
1

Moving Member from one list to other is not going to change the team value. Better idea would be to directly change team from A to B in a Member entity.

dpawel09
  • 116
  • 3
  • Thanks for your answer! However, if you look closely the `transfer()` method, you will see that it calls the `addMember()` method, that sets the team of the member being transfered, by calling its `setTeam()` method. – AlexSC Nov 19 '14 at 12:57
  • You need to rethink your implementation :) Now, you are removing `Member` from list (which isn't going to remove member from db anyway) and then create copy of this member and assign to other list. Additionally - it is not a good idea to put business logic methods to entities. – dpawel09 Nov 19 '14 at 13:21
  • Thanks, but recreating the instance is not an option, there are a number of references to it from other entities (business rules, not mine).Particularly I consider the entities the best place to put business rules, since they represent the business entities, but this is not the subject at this time. Besides, if you have a better idea, why don't you present it as a suggestion? Don't get me wrong, but if you really want to help, improve your answer with an actual solution, instead a vague criticism. – AlexSC Nov 19 '14 at 13:39
0

There are a couple of things that are not clear. First one is how do you handle hibernate session in this test? Is it active throughout the whole test? Second, does teamRepository.teamById(idTeamA) create also members? Because Member zaki = teamA.memberByName("Zaki") will return null if Zaki isn't created.

My setup would be something like this:

  • In @Before method, initial data is created (teamA (with Zaki and Denise), teamB and teamC)
  • In test method, begin transaction and get those entities by id

    teamRepository.transactionBegin();
    Team teamA = teamRepository.teamById(idTeamA); // does this create a team, or returns an existing one? here, it should only return existing team
    ... // get all teams
    Member zaki = teamA.memberByName("Zaki");
    ... // get all members

  • Transfer members

    teamA.transfer(zaki, teamB);
    teamA.transfer(denise, teamC);

  • Commit transaction

    t.commit();

Predrag Maric
  • 23,938
  • 5
  • 52
  • 68
  • Thanks for your answer. I use the Repositoty pattern, so `teamRepository.teamById(xxx)` returns an already saved instance. In my test I assumed that those instances were saved because the problem is how to move from one collection to another and have that correctly persisted (one collection loosing an instance and the other receiving it, without loosing the instance in the process). The problem here is not the test case, it was there just as an example of what I want to do. – AlexSC Dec 12 '14 at 14:08
  • My guess is that it has to do with session management in the test. Because entity code looks fine, and as long as the session is active during entity modifications it should work. – Predrag Maric Dec 12 '14 at 14:15
0

The transfer() method is completely redundant. All you should need to do is call member.setTeam(destination) and Hibernate will update the collections accordingly.

Your Update's code:

member.setTeam(this);

does nothing. It should be

member.setTeam(destination);

You're seeing no updates because you're not changing the value. The member's already a member of this Team.

UPDATE:

Try just changing your transfer() method to this:

public void transfer(Member member, Team destination){
    member.setTeam(destination);
}

After you've moved around all the team members you want to move, you can commit or flush the transaction before moving on to the next unit of work. Moving the items around in the local lists is not necessary unless your units of work are badly defined. Transfer your people, then commit. Then you're ready for the next bit of processing. If you need to do so in the middle of a transaction, then call entityManager.flush() which will execute the currently queued updates but still allow you to later commit or rollback. Flushing in this manner is an anti-pattern, though. If you need to know which team a member is a part of during your processing, ask the member, not the team (which is much more efficient anyway).

Steve K
  • 4,863
  • 2
  • 32
  • 41
  • Thanks for your answear, but I have to disagree. The method is not redundant. Think on this simple scenario: I have two instances of `Team` both have a collection of `Member`. When I transfer one member from one `Team` to another, I want the transfered `Member` to disapear from the origin collection and become visible in the destination collection. Just changing a member's team will not do that! – AlexSC Dec 18 '14 at 09:37
  • AlexSC, it WILL, because you have defined the relationship as one-to-many. A member can only have one team. Try it. If it fails, then feel free to disagree with me. – Steve K Dec 18 '14 at 22:52
  • Well, I guess we see things differently. If I follow your suggestion, in fact the transfered member will disappear from the original collection and be listed in the destination collection, but **only next time I load those two teams**. However, what I want is to have both collections showing the transfer just after it happens, without having evict/detach and reload all those instances. – AlexSC Dec 19 '14 at 01:25
  • You can add the member to the other team, and THEN remove it from the local collection - but the reason you're getting your error is that you're removing it locally, which makes it a disconnected entity, before you add to the other team. If these are database entities, you need to let the database manage them. And you don't need to evict/detach entities to reload them, just call `session.refresh(TeamA)` on your currently active session after the merge. – Steve K Dec 19 '14 at 03:38
  • Good try, but still not working. After calling `transfer()` I call `teamA.getEntityManager().Refresh(teamA)` to refresh the team that just lost one member and I got `javax.persistence.PersistenceException: org.hibernate.PersistentObjectException: detached entity passed to persist: domain.Member`. I'll add the new code and more details in the question. – AlexSC Dec 19 '14 at 13:19