Resolve hibernate database crap
The lab and assignment tables are both declared as the owner of a many to many relationship. This caused two join tables to be created one for both entities. Neither of the two tables contained the other, so this migration inserts their union into a third new table.
The composite key needs to be done through jpa compliant annotations for spring repositories to work correctly. Note that this differs from the hibernate documentation.
Spring repositories apparently code to the jpa spec not to the hibernate implementation. The way to create a composite key using JPA is to create an embeddable that you use as an @EmbeddedId. The associations then need a corresponding @MapsId to link it to the id stored in the embedded id.
There is still a lot of work to do before this lab <-> assignment dataflow is anywhere near sane. There's lots of bullshit happening with cloning of labs, updating labs through shallow clones of json and copying things over from assignments and courses.
Steps to take:
- frontend Lab updates should be mapped to a different type then the concrete entity.
- Tests should be made for the weird cases that are left over with
todos in the code.
These include:
- The resetting of labs through removing and re-adding the links
- The contains check on addings labs and assignments
- Anything that does a getter on labs or assignments that might have mutations trigger any state changes.
Geez man this shit was fucked up....
Merge request reports
Activity
added bug-bounty priohigh typefeature-request labels
- Resolved by Liam Clark
- Resolved by Liam Clark
- Resolved by Liam Clark
- Resolved by Liam Clark
- Resolved by Liam Clark
353 353 lab.setDirection(labData.getDirection()); 354 354 lab.setSlot(labData.getSlot()); 355 355 lab.setRooms(labData.getRooms()); 356 lab.setAssignments(labData.getAssignments()); 356 //Why does a method called add room reset assignments on a lab? 357 358 // This update is slightly problematic to completely reset the entity we should do the following: 359 // Remove all assignments from the lab 360 lab.getAssignmentsFiltered().forEach(assignment -> lab.removeAssignmentLink(assignment)); 126 128 public String toString() { 127 129 return name; 128 130 } 131 132 public LocalDateTime getDeletedAt() { 133 return deletedAt; 134 } 135 136 137 @Override 138 public boolean equals(Object o) { Given that the system will hold records of whether people passed a certain assignment, we need to work towards full accountability. Who did what when and why. If a teacher decides to delete an assignment (by accident perhaps), we do not want to loose the approvals on said assignment. You could argue that you could prevent this with foreign key constraints, but then you won't be able to delete it from the currently available assignments. So the usual way this is fixed is by this deletedAt timestamp (and actually: what is missing is a pointer to the user that did this) and use the @where annotation to prevent these "deleted" assignments from showing up.
75 75 @ManyToOne 76 76 private Course course = new Course(); 77 77 78 @ManyToMany 79 @Where(clause = "deleted_at IS NULL") This where is currently replaced with a filter in !212 (diffs).
We might want to remove this method in the future and just have a proper query for this
503 public void addAssignment(Assignment assignment) { 504 assignments.add(assignment); 504 public void removeAssignmentLink(Assignment assignment) { 505 LabAssignment labAssignment = new LabAssignment(this, assignment); 506 assignment.getLabAssignments().remove(labAssignment); 507 labAssignments.remove(labAssignment); 508 509 labAssignment.setAssignment(null); 510 labAssignment.setLab(null); 505 511 506 if (!assignment.getLabs().contains(this)) { 507 assignment.addLab(this); 508 } 509 512 } 513 //TODO figure out whether this contains check is really needed. 514 // public void addAssignment(Assignment assignment) { changed this line in version 3 of the diff
- Resolved by Chris Lemaire
20 @DirtiesContext(classMode = DirtiesContext.ClassMode.BEFORE_CLASS) 21 public class LabAssignmentTest { 22 @Autowired 23 private LabRepository labRepository; 24 25 @Autowired 26 private CourseRepository courseRepository; 27 28 @Autowired 29 private RoomRepository roomRepository; 30 31 @Autowired 32 private AssignmentRepository assignmentRepository; 33 34 @Test 35 public void testStuffRepositoriesStyle() { changed this line in version 3 of the diff
- Resolved by Liam Clark
added 2 commits
- faed4571 - DB migration for #191 (closed)
- 2ff7eb72 - Update thymeleaf templates
added 2 commits
- 6854834d - DB migration for #191 (closed)
- b10f96fe - Update thymeleaf templates