Skip to content

Commit 18aa69c

Browse files
tkleinketkleinke
committed
Correctly remove isPresentIn relations in resources targeting deleted resource
1 parent be03b24 commit 18aa69c

File tree

6 files changed

+57
-27
lines changed

6 files changed

+57
-27
lines changed

‎core/src/base-config.ts

+1
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,6 @@ export const basicIndexConfiguration = {
2323
'isRecordedIn:contain': { path: 'resource.relations.isRecordedIn', pathArray: ['resource', 'relations', 'isRecordedIn'], type: 'contain' },
2424
'isChildOf:contain': { path: 'resource.relations.isChildOf', pathArray: ['resource', 'relations', 'isChildOf'], type: 'contain', recursivelySearchable: true },
2525
'isChildOf:exist': { path: 'resource.relations.isChildOf', pathArray: ['resource', 'relations', 'isChildOf'], type: 'exist' },
26+
'isPresentIn:contain': { path: 'resource.relations.isPresentIn', pathArray: ['resource', 'relations', 'isPresentIn'], type: 'contain' },
2627
'missingRelationTargetIds:contain': { path: 'warnings.missingRelationTargets.targetIds', pathArray: ['warnings', 'missingRelationTargets', 'targetIds'], type: 'contain' }
2728
};

‎core/src/services/utilities/connected-docs.ts

+29-22
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export namespace ConnectedDocs {
2727
otherVersions: Array<Document>) {
2828

2929
const connectedDocs = await getExistingConnectedDocs(
30-
datastore.get, relationNames, [document].concat(otherVersions)
30+
datastore, relationNames, [document].concat(otherVersions)
3131
);
3232

3333
const docsToUpdate = updateRelations(
@@ -39,7 +39,7 @@ export namespace ConnectedDocs {
3939

4040
for (const doc of connectedDocs) datastore.convert(doc);
4141

42-
await updateDocs(datastore.update, docsToUpdate);
42+
await updateDocs(datastore, docsToUpdate);
4343
}
4444

4545

@@ -48,7 +48,7 @@ export namespace ConnectedDocs {
4848
inverseRelationsMap: Relation.InverseRelationsMap,
4949
document: Document) {
5050

51-
const connectedDocs = await getExistingConnectedDocsForRemove(datastore.get, relationNames, document);
51+
const connectedDocs = await getExistingConnectedDocsForRemove(datastore, relationNames, document);
5252

5353
const docsToUpdate = updateRelations(
5454
document,
@@ -57,41 +57,39 @@ export namespace ConnectedDocs {
5757
false
5858
);
5959

60-
await updateDocs(datastore.update, docsToUpdate);
60+
await updateDocs(datastore, docsToUpdate);
6161
}
6262

6363

64-
async function updateDocs(update: Datastore.Update, docsToUpdate: Array<Document>) {
64+
async function updateDocs(datastore: Datastore, docsToUpdate: Array<Document>) {
6565

6666
// Note that this does not update a document for being target of isRecordedIn
6767
for (let docToUpdate of docsToUpdate) {
68-
await update(docToUpdate, undefined);
68+
await datastore.update(docToUpdate, undefined);
6969
}
7070
}
7171

7272

73-
async function getExistingConnectedDocs(get: Datastore.Get,
74-
relationNames: Array<Name>,
73+
async function getExistingConnectedDocs(datastore: Datastore, relationNames: Array<Name>,
7574
documents: Array<Document>) {
7675

77-
const uniqueConnectedDocIds = getUniqueConnectedDocumentsIds(
78-
documents, relationNames);
76+
const uniqueConnectedDocIds = getUniqueConnectedDocumentsIds(documents, relationNames, datastore);
7977

80-
return getDocumentsForIds(get, uniqueConnectedDocIds, id => {
78+
return getDocumentsForIds(datastore, uniqueConnectedDocIds, id => {
8179
console.warn('connected document not found', id);
8280
})
8381
}
8482

8583

86-
async function getExistingConnectedDocsForRemove(get: Datastore.Get, relationNames: Array<Name>, document: Document) {
84+
async function getExistingConnectedDocsForRemove(datastore: Datastore, relationNames: Array<Name>,
85+
document: Document) {
8786

88-
const uniqueConnectedDocIds = getUniqueConnectedDocumentsIds(
89-
[document], relationNames);
87+
const uniqueConnectedDocIds = getUniqueConnectedDocumentsIds([document], relationNames, datastore);
9088

9189
const liesWithinTargets = Resource.getRelationTargets(document.resource, ['liesWithin']);
9290
const recordedInTargets = Resource.getRelationTargets(document.resource, ['isRecordedIn']);
9391

94-
return getDocumentsForIds(get, uniqueConnectedDocIds, id => {
92+
return getDocumentsForIds(datastore, uniqueConnectedDocIds, id => {
9593
if (liesWithinTargets.includes(id) || recordedInTargets.includes(id)) {
9694
// this can happen due to deletion order during deletion with descendants
9795
} else {
@@ -101,15 +99,13 @@ export namespace ConnectedDocs {
10199
}
102100

103101

104-
async function getDocumentsForIds(get: Datastore.Get,
105-
ids: string[],
106-
handleError: (id: string) => void) {
102+
async function getDocumentsForIds(datastore: Datastore, ids: string[], handleError: (id: string) => void) {
107103

108104
const connectedDocuments: Array<Document> = [];
109105
for (let id of ids) {
110106

111107
try {
112-
connectedDocuments.push(await get(id));
108+
connectedDocuments.push(await datastore.get(id));
113109
} catch {
114110
handleError(id);
115111
}
@@ -118,15 +114,26 @@ export namespace ConnectedDocs {
118114
}
119115

120116

121-
function getUniqueConnectedDocumentsIds(documents: Array<Document>,
122-
allowedRelations: string[]) {
117+
function getUniqueConnectedDocumentsIds(documents: Array<Document>, allowedRelations: string[],
118+
datastore: Datastore) {
123119

124120
const getAllRelationTargetsForDoc = (doc: Document): string[] =>
125-
Resource.getRelationTargets(doc.resource, allowedRelations);
121+
Resource.getRelationTargets(doc.resource, allowedRelations)
122+
.concat(getPresentInDocumentIds(doc, datastore));
126123

127124
return flow(
128125
documents,
129126
flatMap(getAllRelationTargetsForDoc),
130127
subtract(documents.map(toResourceId)));
131128
}
129+
130+
131+
function getPresentInDocumentIds(document: Document, datastore: Datastore): string[] {
132+
133+
return datastore.findIds({
134+
constraints: {
135+
'isPresentIn:contain': document.resource.id
136+
}
137+
}).ids;
138+
}
132139
}

‎core/test/index/perform-query.spec.ts

-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ describe('performQuery', () => {
1919

2020
projectConfiguration = createMockProjectConfiguration();
2121

22-
2322
const createdConstraintIndex = ConstraintIndex.make({
2423
... basicIndexConfiguration,
2524
'someField:exist': { path: 'resource.someField', pathArray: ['resource', 'someField'], type: 'exist' }

‎core/test/services/relations-manager.spec.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,10 @@ describe('RelationsManager', () => {
6969

7070
spyOn(console, 'warn');
7171

72-
mockDatastore = jasmine.createSpyObj('mockDatastore',
73-
['get', 'getMultiple', 'putCache', 'find', 'create', 'update', 'convert', 'refresh', 'remove']);
72+
mockDatastore = jasmine.createSpyObj(
73+
'mockDatastore',
74+
['get', 'getMultiple', 'putCache', 'find', 'findIds', 'create', 'update', 'convert', 'refresh', 'remove']
75+
);
7476

7577
const mockSettingsProvider = jasmine.createSpyObj('settingsProvider', ['getSettings']);
7678
mockSettingsProvider.getSettings.and.returnValue({ username: 'u' });
@@ -79,6 +81,7 @@ describe('RelationsManager', () => {
7981

8082
mockDatastore.get.and.callFake(getFunction);
8183
mockDatastore.find.and.callFake(findFunction);
84+
mockDatastore.findIds.and.returnValue({ ids: [] });
8285
mockDatastore.update.and.returnValue(Promise.resolve(doc));
8386
mockDatastore.create.and.returnValue(Promise.resolve(doc));
8487
mockDatastore.remove.and.returnValue(Promise.resolve('ok'));

‎core/test/services/utilities/connected-docs.spec.ts

+22-1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@ describe('ConnectedDocs', () => {
3737
range: [],
3838
editable: false,
3939
inputType: 'relation'
40+
},
41+
{
42+
name: 'isPresentIn',
43+
domain: [],
44+
range: [],
45+
editable: false,
46+
inputType: 'relation'
4047
}
4148
],
4249
commonFields: {},
@@ -57,14 +64,15 @@ describe('ConnectedDocs', () => {
5764

5865
spyOn(console, 'warn');
5966

60-
mockDatastore = jasmine.createSpyObj('mockDatastore', ['get', 'update', 'convert']);
67+
mockDatastore = jasmine.createSpyObj('mockDatastore', ['get', 'update', 'convert', 'findIds']);
6168
inverseRelationsMap = Relation.makeInverseRelationsMap(projectConfiguration.getRelations());
6269
relationNames = projectConfiguration.getRelations().map(Named.toName);
6370

6471
mockDatastore.update.and.returnValue(Promise.resolve(doc));
6572
mockDatastore.get.and.callFake(async id => {
6673
return id === relatedDoc['resource']['id'] ? relatedDoc : anotherRelatedDoc;
6774
});
75+
mockDatastore.findIds.and.returnValue({ ids: [] });
6876

6977
doc = { 'resource' : {
7078
'id' :'1', 'identifier': 'ob1',
@@ -181,4 +189,17 @@ describe('ConnectedDocs', () => {
181189
expect(mockDatastore.update).not.toHaveBeenCalled();
182190
done();
183191
});
192+
193+
194+
it('remove: delete isPresentIn relation pointing to deleted document', async done => {
195+
196+
relatedDoc.resource.relations['isPresentIn'] = ['1'];
197+
mockDatastore.findIds.and.returnValue({ ids: ['2'] });
198+
199+
await ConnectedDocs.updateForRemove(mockDatastore, relationNames, inverseRelationsMap, doc);
200+
201+
expect(mockDatastore.update).toHaveBeenCalledWith(relatedDoc, undefined);
202+
expect(relatedDoc.resource.relations['isPresentIn']).toBe(undefined);
203+
done();
204+
});
184205
});

‎desktop/src/app/indexer-configuration.ts

-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ export module IndexerConfiguration {
2020
'isInstanceOf:exist': { path: 'resource.relations.isInstanceOf', pathArray: ['resource', 'relations', 'isInstanceOf'], type: 'exist' },
2121
'hasInstance:exist': { path: 'resource.relations.hasInstance', pathArray: ['resource', 'relations', 'hasInstance'], type: 'exist' },
2222
'isSameAs:exist': { path: 'resource.relations.isSameAs', pathArray: ['resource', 'relations', 'isSameAs'], type: 'exist' },
23-
'isPresentIn:contain': { path: 'resource.relations.isPresentIn', pathArray: ['resource', 'relations', 'isPresentIn'], type: 'contain' },
2423
'geometry:exist': { path: 'resource.geometry', pathArray: ['resource', 'geometry'], type: 'exist' },
2524
'georeference:exist': { path: 'resource.georeference', pathArray: ['resource', 'georeference'], type: 'exist' },
2625
'conflicts:exist': { path: '_conflicts', pathArray: ['_conflicts'], type: 'exist' },

0 commit comments

Comments
 (0)