0

I would like to extend the requirements mentioned in the earlier post to support deletes. We have two data model object - Organization & Department sharing a one-to-many relationship. With the below mapping I am able to read the list of departments from the organization object. I have not added the cascade ALL property to restrict adding a department when creating an organization.

How should I modify the @OneToMany annotation (and possibly @ManyToOne) to restrict inserts of department but cascade the delete operation such that all associated departments are deleted when deleting an organization object?

@Entity
@Table(name="ORGANIZATIONS")
public class Organization{

    @Id
    @GeneratedValue
    Private long id;

    @Column(unique=true)
    Private String name;

    @OneToMany(mappedBy = "organization", fetch = FetchType.EAGER)
    private List<Department> departments;


}

@Entity
@Table(name="DEPARTMENTS")
Public class Department{

   @Id
   @GeneratedValue
   Private long id;

   @Column(unique=true)
   Private String name;


   @ManyToOne(fetch = FetchType.EAGER)
   private Organization organization;

}

The code to delete the organization is just a line

organizationRepository.deleteById(orgId);

The test case to validate this is as below

@RunWith(SpringJUnit4ClassRunner.class)
@DataJpaTest
@Transactional
public class OrganizationRepositoryTests {

    @Autowired
    private OrganizationRepository organizationRepository;

    @Autowired
    private DepartmentRepository departmentRepository;



    @Test
    public void testDeleteOrganization() {
        final organization organization = organizationRepository.findByName(organizationName).get(); //precondition

        Department d1 = new Department();
        d1.setName("d1");

        d1.setorganization(organization);

        Department d2 = new Department();
        d2.setName("d2");

        d2.setorganization(organization);

        departmentRepository.save(d1);
        departmentRepository.save(d2);

//        assertEquals(2, organizationRepository.getOne(organization.getId()).getDepartments().size()); //this assert is failing. For some reason organizations does not have a list of departments

        organizationRepository.deleteById(organization.getId());

        assertFalse(organizationRepository.findByName(organizationName).isPresent());
        assertEquals(0, departmentRepository.findAll().size()); //no departments should be found

    }

}
Andy Dufresne
  • 6,022
  • 7
  • 63
  • 113

3 Answers3

1

See code comments on why it fails:

@RunWith(SpringJUnit4ClassRunner.class)
@DataJpaTest
@Transactional
public class OrganizationRepositoryTests {

    @Autowired
    private OrganizationRepository organizationRepository;

    @Autowired
    private DepartmentRepository departmentRepository;

    @PersistenceContext
    private Entitymanager em;

    @Test
    public void testDeleteOrganization() {
        Organization organization = 
                organizationRepository.findByName(organizationName).get(); 

        Department d1 = new Department();
        d1.setName("d1");
        d1.setOrganization(organization);

        Department d2 = new Department();
        d2.setName("d2");
        d2.setOrganization(organization);

        departmentRepository.save(d1);
        departmentRepository.save(d2);

        // this fails because there is no trip to the database as Organization 
        // (the one loaded in the first line)
        // already exists in the current entityManager - and you have not 
        // updated its list of departments.
        // uncommenting the following line will trigger a reload and prove 
        // this to be the case: however it is not a fix for the issue.

        // em.clear();

         assertEquals(2,
             organizationRepository.getOne(
               organization.getId()).getDepartments().size()); 

        //similary this will execute without error with the em.clear() 
        //statement uncommented
        //however without that Hibernate knows nothing about the cascacding 
        //delete as there are no departments
        //associated with organisation as you have not added them to the list.
        organizationRepository.deleteById(organization.getId());

        assertFalse(organizationRepository.findByName(organizationName).isPresent());
        assertEquals(0, departmentRepository.findAll().size()); 
    }
}

The correct fix is to ensure that the in-memory model is always maintained correctly by encapsulating add/remove/set operations and preventing direct access to collections. e.g.

public class Department(){
    public void setOrganisation(Organisation organisation){
        this.organisation = organisation;

        if(! organisation.getDepartments().contains(department)){
            organisation.addDepartment(department);
        }
    }
}

public class Organisation(){

    public List<Department> getDepartments(){
        return Collections.unmodifiableList(departments);
    }

    public void addDepartment(Department departmenmt){
        departments.add(department);

        if(department.getOrganisation() != this){
            department.setOrganisation(this);
        }
    }
}
Alan Hay
  • 22,665
  • 4
  • 56
  • 110
  • The delete fails with an error - "ERROR 23503: DELETE on table 'ORGANIZATIONS' caused a violation of foreign key constraint 'FKP6W2NOGXIEXHGMUXN1G4MWVHP' for key (2)". This is the constraint added on the department table because it has organization id as the foreign key. Also we cannot remove the setDepartments() method or return an unmodifiableList from getProjects() since when creating a new Department we fetch the organization object in it and set the list of departments so that the chained calls work - department.getOrganization().getDepartments() – Andy Dufresne Jan 24 '19 at 09:51
  • Regarding you 2nd point I don't really understand what you mean. For the first issue is this correct `mappedBy = "organization"` because the field in the posted code is named `org` – Alan Hay Jan 24 '19 at 09:59
  • Yes, I corrected the mappedBy attribute. It was a typo. The delete operation still fails with the "violation of foreign key constraint" error. About #2, the one-to-many relationship is bidirectional so organization.getDepartments() should work and this should work too - department.getOrganization().getDepartments() . – Andy Dufresne Jan 24 '19 at 10:05
  • Ensuring the correctness of the in-memory model should be done by means of encapsulation rather than by allowing client code to randomly modify collections. https://stackoverflow.com/a/23648953/1356423. For the SQL error you should turn on SQL logging to see what is going on. – Alan Hay Jan 24 '19 at 10:10
  • The delete is failing with the same error. Turning on sql logging just shows the last statement being fired as "delete from organizations where id=?". o.h.engine.jdbc.spi.SqlExceptionHelper : SQL Error: 20000, SQLState: 23503 I am testing this using a @DataJpaTest which tries to save the organization first, then add departments through the departments repository and then executes orgRepository.deleteById(). I have removed the setter for department too though that's a separate change. – Andy Dufresne Jan 24 '19 at 12:54
  • I believe your in-memory model is broken - the departments do not exist in the list associated with the organisation so Hibernate knows nothing about them when you call delete. You will need to post the test class in full. – Alan Hay Jan 24 '19 at 13:06
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/187259/discussion-between-andy-dufresne-and-alan-hay). – Andy Dufresne Jan 24 '19 at 13:25
  • See the updated answer clearly explaining what you are doing wrong. – Alan Hay Jan 24 '19 at 13:57
  • Maybe you have fixed this already: check the setter for the organization in the test class. It carries a typo and the organization might not be set correctly (... .setorganization(...). ... should be ... .setOrganization(...). ... – benji2505 Jan 24 '19 at 13:58
  • I tried both of these changes - em.clear & setters in respective classes. It seems organizationRepository.getOne() isn't fetching the list of Projects even after em.clear. If I comment that, the delete still fails with constraint violation error. I verified that both the instances (before em.clear and after) of organization have different object ids. Hence em.clear() is working in clearing the objects from the sessions cache. – Andy Dufresne Jan 24 '19 at 14:32
  • Sorry. I can't help you any more. The answer I have given is broadly correct and would suggest reading up on some JPA basics is likely to be of more use than discussing any further here. Looking at the SQL logging output should help you understand what is going on. You can also try `departmentRepository.saveAndFlush(d1);` However as I say your code is badly broken. – Alan Hay Jan 24 '19 at 14:37
  • 1
    You can also try using `departmentRepository.saveAndFlush(d1);` or removing `@Transactional` from the test class to actually trigger a database write. Flushing in the context of a transaction is something you need to understand. – Alan Hay Jan 24 '19 at 14:43
  • Calling flush() resolved the issue. Thanks for helping out and appreciate your patience while troubleshooting the issue – Andy Dufresne Jan 25 '19 at 05:35
0

You can try to add to limit the cascade to delete operations only from Organization to department:

@OneToMany(mappedBy = "organization", fetch = FetchType.EAGER, cascade = CascadeType.REMOVE, orphanRemoval = true)
private List<Department> departments;

Please note that if you have dependents/foreign key constraints on the department entity, then you would need to cascade the delete operations to these dependent entities as well.

You can read this guide, it explains the cascade operations nicely: https://vladmihalcea.com/a-beginners-guide-to-jpa-and-hibernate-cascade-types/

Nullbeans
  • 309
  • 1
  • 8
  • The delete fails with an error - "ERROR 23503: DELETE on table 'ORGANIZATIONS' caused a violation of foreign key constraint 'FKP6W2NOGXIEXHGMUXN1G4MWVHP' for key (2)". This is the constraint added on the department table because it has organization id as the foreign key. – Andy Dufresne Jan 24 '19 at 09:52
  • Hello Andy, how did you generate your database? Was it manually scripted or did you use the hibernate.ddl property to autoamatically generate the schema? In the mean time, can you please also try to add "orphanRemoval = true" to your annotation. I will update my answer as well. – Nullbeans Jan 24 '19 at 15:23
0

Try this code,

    @OneToMany( fetch = FetchType.EAGER, cascade = CascadeType.ALL)
    @JoinColumn(name = "organisation_id", referencedColumnName = "id")
    private List<Department> departments;

    @ManyToOne(fetch = FetchType.EAGER,ascade = CascadeType.REFRESH,mappedBy = "departments")
    private Organization organization;

if any issue inform

avishka eranga
  • 105
  • 1
  • 11