Skip to content

Commit 5deaf98

Browse files
ma-mirzaeima-mirzaei
authored
Merge pull request #224 from vitruv-tools/copilot/sub-pr-223
Fix view sync correctness, cascade delete, N+1 queries, and revert path for VSUM views
2 parents bf76813 + 9bbea6d commit 5deaf98

File tree

12 files changed

+249
-21
lines changed

12 files changed

+249
-21
lines changed

‎app/src/main/java/tools/vitruv/methodologist/user/service/UserService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ public User create(UserPostRequest userPostRequest) {
247247
* @param ttlMinutes time-to-live for the OTP (in minutes) shown in the email
248248
*/
249249
public void sendOtp(String to, String toName, String otp, int ttlMinutes) {
250-
// mailService.sendOtpMail(to, toName, otp, ttlMinutes);
250+
mailService.sendOtpMail(to, toName, otp, ttlMinutes);
251251
}
252252

253253
/**

‎app/src/main/java/tools/vitruv/methodologist/vsum/mapper/VsumHistoryMapper.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ default Set<VsumRepresentation.View> toView(Set<VsumView> views) {
8787
.map(
8888
vsumViewMetaModel ->
8989
vsumViewMetaModel.getMetaModel().getSource().getId())
90+
.sorted()
9091
.toList())
9192
.fileStorageId(view.getFileStorage().getId())
9293
.build())

‎app/src/main/java/tools/vitruv/methodologist/vsum/model/repository/VsumViewMetaModelRepository.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
package tools.vitruv.methodologist.vsum.model.repository;
22

3+
import java.util.Collection;
34
import java.util.List;
5+
import org.springframework.data.jpa.repository.Modifying;
6+
import org.springframework.data.jpa.repository.Query;
47
import org.springframework.data.repository.CrudRepository;
8+
import org.springframework.data.repository.query.Param;
59
import org.springframework.stereotype.Repository;
610
import tools.vitruv.methodologist.vsum.model.VsumView;
711
import tools.vitruv.methodologist.vsum.model.VsumViewMetaModel;
@@ -21,10 +25,29 @@ public interface VsumViewMetaModelRepository extends CrudRepository<VsumViewMeta
2125
*/
2226
List<VsumViewMetaModel> findAllByVsumView(VsumView vsumView);
2327

28+
/**
29+
* Retrieves all view-meta-model associations for the given collection of VSUM views in a single
30+
* query, avoiding N+1 patterns when processing multiple views.
31+
*
32+
* @param views the VSUM views whose associated view-meta-model entries are requested
33+
* @return a list of {@link VsumViewMetaModel} entities linked to any of the provided VSUM views
34+
*/
35+
List<VsumViewMetaModel> findAllByVsumViewIn(Collection<VsumView> views);
36+
2437
/**
2538
* Deletes all view-meta-model associations for the given VSUM view.
2639
*
2740
* @param vsumView the VSUM view whose associated view-meta-model entries should be removed
2841
*/
2942
void deleteAllByVsumView(VsumView vsumView);
43+
44+
/**
45+
* Deletes all view-meta-model associations for the given collection of VSUM views in a single
46+
* bulk query.
47+
*
48+
* @param views the VSUM views whose associated view-meta-model entries should be removed
49+
*/
50+
@Modifying
51+
@Query("DELETE FROM VsumViewMetaModel vm WHERE vm.vsumView IN :views")
52+
void deleteAllByVsumViewIn(@Param("views") Collection<VsumView> views);
3053
}

‎app/src/main/java/tools/vitruv/methodologist/vsum/service/VsumHistoryService.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import tools.vitruv.methodologist.user.model.repository.UserRepository;
1717
import tools.vitruv.methodologist.vsum.VsumRepresentation;
1818
import tools.vitruv.methodologist.vsum.controller.dto.request.MetaModelRelationRequest;
19+
import tools.vitruv.methodologist.vsum.controller.dto.request.ViewRequest;
1920
import tools.vitruv.methodologist.vsum.controller.dto.request.VsumSyncChangesPutRequest;
2021
import tools.vitruv.methodologist.vsum.controller.dto.response.VsumHistoryResponse;
2122
import tools.vitruv.methodologist.vsum.mapper.VsumHistoryMapper;
@@ -202,6 +203,25 @@ private VsumSyncChangesPutRequest toSyncRequest(VsumRepresentation representatio
202203
vsumSyncChangesPutRequest.setMetaModelRelationRequests(metaModelRelationRequests);
203204
}
204205

206+
if (representation.getViews() == null) {
207+
vsumSyncChangesPutRequest.setViewRequests(List.of());
208+
} else {
209+
List<ViewRequest> viewRequests =
210+
representation.getViews().stream()
211+
.filter(Objects::nonNull)
212+
.map(
213+
view ->
214+
ViewRequest.builder()
215+
.fileStorageId(view.getFileStorageId())
216+
.metaModelIds(
217+
view.getMetaModelIds() == null
218+
? List.of()
219+
: List.copyOf(view.getMetaModelIds()))
220+
.build())
221+
.toList();
222+
vsumSyncChangesPutRequest.setViewRequests(viewRequests);
223+
}
224+
205225
return vsumSyncChangesPutRequest;
206226
}
207227
}

‎app/src/main/java/tools/vitruv/methodologist/vsum/service/VsumService.java‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -832,9 +832,7 @@ private String toViewKey(ViewRequest request) {
832832
return request.getFileStorageId() + "|" + metaModelPart;
833833
}
834834

835-
private String toViewKey(VsumView vsumView) {
836-
List<VsumViewMetaModel> relations = vsumViewMetaModelRepository.findAllByVsumView(vsumView);
837-
835+
private String toViewKey(VsumView vsumView, List<VsumViewMetaModel> relations) {
838836
String metaModelPart =
839837
relations.stream()
840838
.map(VsumViewMetaModel::getMetaModel)
@@ -855,9 +853,22 @@ private String toViewKey(VsumView vsumView) {
855853
private ViewSyncPlan buildViewSyncPlan(Vsum vsum, List<ViewRequest> desiredViewRequests) {
856854
List<VsumView> existingViews = vsumViewRepository.findAllByVsum(vsum);
857855

856+
Map<Long, List<VsumViewMetaModel>> relationsByViewId = new HashMap<>();
857+
if (!existingViews.isEmpty()) {
858+
List<VsumViewMetaModel> allRelations =
859+
vsumViewMetaModelRepository.findAllByVsumViewIn(existingViews);
860+
for (VsumViewMetaModel relation : allRelations) {
861+
relationsByViewId
862+
.computeIfAbsent(relation.getVsumView().getId(), k -> new ArrayList<>())
863+
.add(relation);
864+
}
865+
}
866+
858867
Map<String, VsumView> existingByKey = new HashMap<>();
859868
for (VsumView existingView : existingViews) {
860-
existingByKey.put(toViewKey(existingView), existingView);
869+
List<VsumViewMetaModel> viewRelations =
870+
relationsByViewId.getOrDefault(existingView.getId(), List.of());
871+
existingByKey.put(toViewKey(existingView, viewRelations), existingView);
861872
}
862873

863874
Set<String> existingKeys = new HashSet<>(existingByKey.keySet());

‎app/src/main/java/tools/vitruv/methodologist/vsum/service/VsumViewMetaModelService.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.util.ArrayList;
44
import java.util.List;
55
import java.util.Set;
6+
import java.util.stream.StreamSupport;
67
import lombok.AccessLevel;
78
import lombok.AllArgsConstructor;
89
import lombok.experimental.FieldDefaults;
@@ -34,14 +35,19 @@ public class VsumViewMetaModelService {
3435
/**
3536
* Creates associations between a view and the provided metamodel ids.
3637
*
37-
* <p>Only source metamodels are considered valid for association.
38+
* <p>Only source metamodels are considered valid for association. A {@code null} or empty {@code
39+
* metaModelIds} set results in an empty list being returned without querying the database.
3840
*
3941
* @param vsumView target view
40-
* @param metaModelIds metamodel ids to associate
42+
* @param metaModelIds metamodel ids to associate; may be {@code null} (treated as empty)
4143
* @return persisted join entities
4244
*/
4345
@Transactional
4446
public List<VsumViewMetaModel> create(VsumView vsumView, Set<Long> metaModelIds) {
47+
if (metaModelIds == null || metaModelIds.isEmpty()) {
48+
return List.of();
49+
}
50+
4551
List<VsumMetaModel> vsumMetaModels =
4652
vsumMetaModelRepository.findAllByVsumAndMetaModel_source_idIn(
4753
vsumView.getVsum(), metaModelIds);
@@ -55,7 +61,9 @@ public List<VsumViewMetaModel> create(VsumView vsumView, Set<Long> metaModelIds)
5561
.build());
5662
}
5763

58-
return (List<VsumViewMetaModel>) vsumViewMetaModelRepository.saveAll(entities);
64+
return StreamSupport.stream(
65+
vsumViewMetaModelRepository.saveAll(entities).spliterator(), false)
66+
.toList();
5967
}
6068

6169
/**

‎app/src/main/java/tools/vitruv/methodologist/vsum/service/VsumViewService.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static tools.vitruv.methodologist.messages.Error.VIEW_FILE_ID_NOT_FOUND_ERROR;
44

5+
import java.util.List;
56
import lombok.AccessLevel;
67
import lombok.AllArgsConstructor;
78
import lombok.experimental.FieldDefaults;
@@ -14,6 +15,7 @@
1415
import tools.vitruv.methodologist.general.model.repository.FileStorageRepository;
1516
import tools.vitruv.methodologist.vsum.model.Vsum;
1617
import tools.vitruv.methodologist.vsum.model.VsumView;
18+
import tools.vitruv.methodologist.vsum.model.repository.VsumViewMetaModelRepository;
1719
import tools.vitruv.methodologist.vsum.model.repository.VsumViewRepository;
1820

1921
/**
@@ -30,6 +32,7 @@ public class VsumViewService {
3032

3133
VsumViewRepository vsumViewRepository;
3234
FileStorageRepository fileStorageRepository;
35+
VsumViewMetaModelRepository vsumViewMetaModelRepository;
3336

3437
/**
3538
* Creates and persists a new {@link VsumView} for the given VSUM and NeoJoin file.
@@ -60,11 +63,15 @@ public VsumView create(Vsum vsum, Long fileStorageId) {
6063
/**
6164
* Deletes the specified view and removes it from the in-memory VSUM collection if present.
6265
*
66+
* <p>Metamodel associations ({@code VsumViewMetaModel}) for the view are deleted before the view
67+
* itself to satisfy foreign key constraints.
68+
*
6369
* @param vsum target VSUM
6470
* @param vsumView view to delete
6571
*/
6672
@Transactional
6773
public void delete(Vsum vsum, VsumView vsumView) {
74+
vsumViewMetaModelRepository.deleteAllByVsumView(vsumView);
6875
vsumViewRepository.delete(vsumView);
6976

7077
if (vsum.getViews() != null) {
@@ -75,10 +82,17 @@ public void delete(Vsum vsum, VsumView vsumView) {
7582
/**
7683
* Deletes all views associated with the given VSUM.
7784
*
85+
* <p>Metamodel associations ({@code VsumViewMetaModel}) for all views are deleted before the
86+
* views themselves to satisfy foreign key constraints.
87+
*
7888
* @param vsum target VSUM
7989
*/
8090
@Transactional
8191
public void deleteByVsum(Vsum vsum) {
92+
List<VsumView> views = vsumViewRepository.findAllByVsum(vsum);
93+
if (!views.isEmpty()) {
94+
vsumViewMetaModelRepository.deleteAllByVsumViewIn(views);
95+
}
8296
vsumViewRepository.deleteAllByVsum(vsum);
8397

8498
if (vsum.getViews() != null) {

‎app/src/main/resources/db/migration/V9__VIEW_TYPE_CONSTRAINTS.sql‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
ALTER TABLE vsum_view_meta_model
2+
ADD CONSTRAINT uk_vsum_view_meta_model UNIQUE (vsum_view_id, meta_model_id);
3+
4+
CREATE INDEX idx_vsum_view_meta_model_vsum_view_id ON vsum_view_meta_model (vsum_view_id);
5+
6+
CREATE INDEX idx_vsum_view_meta_model_meta_model_id ON vsum_view_meta_model (meta_model_id);

‎app/src/test/java/tools/vitruv/methodologist/vsum/service/VsumHistoryServiceTest.java‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,4 +400,104 @@ void revert_filtersNullRelationsWhileMapping() {
400400

401401
assertThat(reqCap.getValue().getMetaModelRelationRequests()).hasSize(1);
402402
}
403+
404+
@Test
405+
void revert_mapsViewsToViewRequests_whenRepresentationContainsViews() {
406+
String callerEmail = "u@ex.com";
407+
408+
User user = new User();
409+
user.setEmail(callerEmail);
410+
when(userRepository.findByEmailIgnoreCaseAndRemovedAtIsNull(callerEmail))
411+
.thenReturn(Optional.of(user));
412+
413+
Vsum vsum = new Vsum();
414+
vsum.setId(104L);
415+
416+
VsumRepresentation.View view1 =
417+
VsumRepresentation.View.builder()
418+
.fileStorageId(10L)
419+
.metaModelIds(List.of(1L, 2L))
420+
.build();
421+
VsumRepresentation.View view2 =
422+
VsumRepresentation.View.builder()
423+
.fileStorageId(20L)
424+
.metaModelIds(List.of(3L))
425+
.build();
426+
427+
VsumRepresentation rep =
428+
VsumRepresentation.builder()
429+
.metaModels(Set.of())
430+
.metaModelsRealation(Set.of())
431+
.views(new HashSet<>(Set.of(view1, view2)))
432+
.build();
433+
434+
Long historyId = 16L;
435+
VsumHistory history =
436+
VsumHistory.builder().id(historyId).vsum(vsum).representation(rep).build();
437+
when(vsumHistoryRepository.findById(historyId)).thenReturn(Optional.of(history));
438+
439+
when(vsumUserRepository
440+
.findByVsum_IdAndUser_EmailAndUser_RemovedAtIsNullAndVsum_RemovedAtIsNull(
441+
vsum.getId(), callerEmail))
442+
.thenReturn(Optional.of(new VsumUser()));
443+
444+
service.revert(callerEmail, historyId);
445+
446+
ArgumentCaptor<VsumSyncChangesPutRequest> reqCap =
447+
ArgumentCaptor.forClass(VsumSyncChangesPutRequest.class);
448+
verify(vsumService).applySyncChanges(eq(vsum), eq(user), reqCap.capture(), eq(false));
449+
450+
VsumSyncChangesPutRequest applied = reqCap.getValue();
451+
assertThat(applied.getViewRequests()).hasSize(2);
452+
assertThat(applied.getViewRequests())
453+
.anySatisfy(
454+
v -> {
455+
assertThat(v.getFileStorageId()).isEqualTo(10L);
456+
assertThat(v.getMetaModelIds()).containsExactlyInAnyOrder(1L, 2L);
457+
});
458+
assertThat(applied.getViewRequests())
459+
.anySatisfy(
460+
v -> {
461+
assertThat(v.getFileStorageId()).isEqualTo(20L);
462+
assertThat(v.getMetaModelIds()).containsExactly(3L);
463+
});
464+
}
465+
466+
@Test
467+
void revert_mapsNullViewsToEmptyViewRequests() {
468+
String callerEmail = "u@ex.com";
469+
470+
User user = new User();
471+
user.setEmail(callerEmail);
472+
when(userRepository.findByEmailIgnoreCaseAndRemovedAtIsNull(callerEmail))
473+
.thenReturn(Optional.of(user));
474+
475+
Vsum vsum = new Vsum();
476+
vsum.setId(105L);
477+
478+
VsumRepresentation rep =
479+
VsumRepresentation.builder()
480+
.metaModels(null)
481+
.metaModelsRealation(null)
482+
.views(null)
483+
.build();
484+
485+
Long historyId = 17L;
486+
VsumHistory history =
487+
VsumHistory.builder().id(historyId).vsum(vsum).representation(rep).build();
488+
when(vsumHistoryRepository.findById(historyId)).thenReturn(Optional.of(history));
489+
490+
when(vsumUserRepository
491+
.findByVsum_IdAndUser_EmailAndUser_RemovedAtIsNullAndVsum_RemovedAtIsNull(
492+
vsum.getId(), callerEmail))
493+
.thenReturn(Optional.of(new VsumUser()));
494+
495+
service.revert(callerEmail, historyId);
496+
497+
ArgumentCaptor<VsumSyncChangesPutRequest> reqCap =
498+
ArgumentCaptor.forClass(VsumSyncChangesPutRequest.class);
499+
verify(vsumService).applySyncChanges(eq(vsum), eq(user), reqCap.capture(), eq(false));
500+
501+
assertThat(reqCap.getValue().getViewRequests()).isNotNull().isEmpty();
502+
}
403503
}

‎app/src/test/java/tools/vitruv/methodologist/vsum/service/VsumServiceTest.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ void setUp() {
193193

194194
lenient().when(vsumViewRepository.findAllByVsum(any(Vsum.class))).thenReturn(List.of());
195195
lenient()
196-
.when(vsumViewMetaModelRepository.findAllByVsumView(any(VsumView.class)))
196+
.when(vsumViewMetaModelRepository.findAllByVsumViewIn(anyList()))
197197
.thenReturn(List.of());
198198
}
199199

@@ -765,9 +765,12 @@ void update_deletesViews_whenExistingViewIsRemoved_andWritesHistory() {
765765

766766
VsumViewMetaModel vm1 = new VsumViewMetaModel();
767767
vm1.setMetaModel(clonedMetaModel(1001L, 1L));
768+
vm1.setVsumView(existingView);
768769
VsumViewMetaModel vm2 = new VsumViewMetaModel();
769770
vm2.setMetaModel(clonedMetaModel(1002L, 2L));
770-
when(vsumViewMetaModelRepository.findAllByVsumView(existingView)).thenReturn(List.of(vm1, vm2));
771+
vm2.setVsumView(existingView);
772+
when(vsumViewMetaModelRepository.findAllByVsumViewIn(List.of(existingView)))
773+
.thenReturn(List.of(vm1, vm2));
771774

772775
VsumSyncChangesPutRequest put = new VsumSyncChangesPutRequest();
773776
put.setViewRequests(List.of());
@@ -806,9 +809,12 @@ void update_doesNotTouchViews_whenDesiredMatchesExistingAfterNormalization() {
806809

807810
VsumViewMetaModel vm1 = new VsumViewMetaModel();
808811
vm1.setMetaModel(clonedMetaModel(2001L, 1L));
812+
vm1.setVsumView(existingView);
809813
VsumViewMetaModel vm2 = new VsumViewMetaModel();
810814
vm2.setMetaModel(clonedMetaModel(2002L, 2L));
811-
when(vsumViewMetaModelRepository.findAllByVsumView(existingView)).thenReturn(List.of(vm1, vm2));
815+
vm2.setVsumView(existingView);
816+
when(vsumViewMetaModelRepository.findAllByVsumViewIn(List.of(existingView)))
817+
.thenReturn(List.of(vm1, vm2));
812818

813819
VsumSyncChangesPutRequest put = new VsumSyncChangesPutRequest();
814820
put.setViewRequests(

0 commit comments

Comments
 (0)