Skip to content

Commit 4cdcdc3

Browse files
vlt1vlt1
committed
#4758: Resolve inconsistences with linkedResources and NODATA (#5760)
(cherry picked from commit f1b1326)
1 parent 6e4f85e commit 4cdcdc3

6 files changed

Lines changed: 126 additions & 19 deletions

File tree

‎web/client/components/resources/forms/Thumbnail.jsx‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ class Thumbnail extends React.Component {
100100
// with at least one error
101101
this.props.onError(errors, this.props.resource.id);
102102
this.files = files;
103-
this.props.onUpdate(null, null);
104103
}}
105104
onRemove={() => {
106105
this.files = null;

‎web/client/components/resources/modals/fragments/MainForm.jsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ module.exports = class MainForm extends React.Component {
5555
onRemove={() => onUpdateLinkedResource("thumbnail", "NODATA", "THUMBNAIL", {
5656
tail: `/raw?decode=datauri&v=${uuid()}`
5757
})}
58-
onUpdate={(data) => onUpdateLinkedResource("thumbnail", data, "THUMBNAIL", {
58+
onUpdate={(data) => onUpdateLinkedResource("thumbnail", data || "NODATA", "THUMBNAIL", {
5959
tail: `/raw?decode=datauri&v=${uuid()}`
6060
})} />
6161
</Col>

‎web/client/epics/__tests__/maps-test.js‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,7 @@ describe('Create and update flow using persistence api', () => {
692692
Persistence.setApi("geostore");
693693
});
694694
it('test create flow ', done => {
695-
testEpic(addTimeoutEpic(mapSaveMapResourceEpic), 6, saveMapResource( {}), actions => {
695+
testEpic(addTimeoutEpic(mapSaveMapResourceEpic), 6, saveMapResource({}), actions => {
696696
expect(actions.length).toBe(6);
697697
actions.map((action) => {
698698
switch (action.type) {
@@ -748,6 +748,36 @@ describe('Create and update flow using persistence api', () => {
748748
done();
749749
});
750750
});
751+
it('test thumbnail attribute is ignored', done => {
752+
const attributeTestApi = {
753+
...api,
754+
updateResourceAttribute: ({name}) => name === 'thumbnail' ? done(new Error('thumbnail update request was issued!')) : Rx.Observable.of(10)
755+
};
756+
Persistence.addApi('attributeTest', attributeTestApi);
757+
Persistence.setApi('attributeTest');
758+
759+
testEpic(addTimeoutEpic(mapSaveMapResourceEpic), 6, saveMapResource({id: 10, metadata: {name: 'resource'}, attributes: {thumbnail: 'thumb', someAttribute: 'value'}}), actions => {
760+
try {
761+
expect(actions.length).toBe(6);
762+
actions.map((action) => {
763+
switch (action.type) {
764+
case MAP_UPDATING:
765+
case TOGGLE_CONTROL:
766+
case MAP_SAVED:
767+
case SHOW_NOTIFICATION:
768+
case LOAD_MAP_INFO:
769+
case MAP_CONFIG_LOADED:
770+
break;
771+
default:
772+
expect(true).toBe(false);
773+
}
774+
});
775+
done();
776+
} catch (e) {
777+
done(e);
778+
}
779+
}, {});
780+
});
751781

752782
it('test reloadMaps', function(done) {
753783
const callback = actions => {

‎web/client/epics/maps.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,8 +490,9 @@ const mapSaveMapResourceEpic = (action$, store) =>
490490
action$.ofType(SAVE_MAP_RESOURCE)
491491
.exhaustMap(({resource}) => {
492492
// filter out invalid attributes
493+
// thumbnails are handled separately
493494
const validAttributesNames = keys(resource.attributes)
494-
.filter(attrName => resource.attributes[attrName] !== undefined && resource.attributes[attrName] !== null);
495+
.filter(attrName => attrName !== 'thumbnail' && resource.attributes[attrName] !== undefined && resource.attributes[attrName] !== null);
495496
return Rx.Observable.forkJoin(
496497
(() => {
497498
// get a context information using the id in the attribute

‎web/client/observables/__tests__/geostore-test.js‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
const expect = require('expect');
99

1010
const geoStoreMock = require('./geoStoreMock').default;
11-
const {createResource, deleteResource, getResourceIdByName} = require('../geostore');
11+
const {createResource, deleteResource, getResourceIdByName, updateResource} = require('../geostore');
1212
const testAndResolve = (test = () => {}, value) => (...args) => {
1313
test(...args);
1414
return Promise.resolve(value);
@@ -132,5 +132,76 @@ describe('geostore observables for resources management', () => {
132132

133133
});
134134
});
135+
136+
it('updateResource linked resource is not created if no thumbnail attribute for resource and data is NODATA', done => {
137+
const testResource = {
138+
id: 10,
139+
data: {},
140+
category: "TEST",
141+
metadata: {
142+
name: "RES2"
143+
},
144+
linkedResources: {
145+
thumbnail: {
146+
tail: '/raw?decode=datauri',
147+
data: "NODATA"
148+
}
149+
}
150+
};
151+
const DummyAPI = {
152+
getResourceAttribute: () => Promise.reject({status: 404}),
153+
putResourceMetadataAndAttributes: () => Promise.resolve(10),
154+
putResource: () => Promise.resolve(10),
155+
createResource: ({name}) => name.search(/10-thumbnail/) !== -1 ? done(new Error('createResource for thumbnail is called!')) : Promise.resolve(11),
156+
updateResourceAttribute: () => Promise.resolve(11)
157+
};
158+
updateResource(testResource, DummyAPI).subscribe(
159+
() => done(),
160+
e => done(e)
161+
);
162+
});
163+
164+
it('updateResource linked resource is created if data is valid', done => {
165+
const testResource = {
166+
id: 10,
167+
data: {},
168+
category: "TEST",
169+
metadata: {
170+
name: "RES2"
171+
},
172+
linkedResources: {
173+
thumbnail: {
174+
tail: '/raw?decode=datauri',
175+
data: "data"
176+
}
177+
}
178+
};
179+
180+
let createResourceThumbnail = false;
181+
182+
const DummyAPI = {
183+
getResourceAttribute: () => Promise.reject({status: 404}),
184+
putResourceMetadataAndAttributes: () => Promise.resolve(10),
185+
putResource: () => Promise.resolve(10),
186+
createResource: ({name}) => {
187+
if (name.search(/10-thumbnail/) !== -1) {
188+
createResourceThumbnail = true;
189+
}
190+
return Promise.resolve(11);
191+
},
192+
updateResourceAttribute: () => Promise.resolve(11)
193+
};
194+
updateResource(testResource, DummyAPI).subscribe(
195+
() => {
196+
try {
197+
expect(createResourceThumbnail).toBe(true);
198+
done();
199+
} catch (e) {
200+
done(e);
201+
}
202+
},
203+
e => done(e)
204+
);
205+
});
135206
});
136207
});

‎web/client/observables/geostore.js‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,14 @@ const updateOrDeleteLinkedResource = (id, attributeName, linkedResource = {}, re
7676

7777
/**
7878
* Creates a resource on GeoStore and link it to the current resource. Can be used for thumbnails, details and so on
79+
* If the data for the resource is "NODATA", it won't be created.
7980
* @param {number} id the id of the resource
8081
* @param {string} attributeName the name of the attribute to link
8182
* @param {object} linkedResource the resource object of the resource to link. This resource have a tail option that can be used to add options URL of the link
8283
* @param {object} API the API to use (default GeoStoreDAO)
8384
*/
8485
const createLinkedResource = (id, attributeName, linkedResource, permission, API = GeoStoreDAO) =>
85-
Observable.defer(() =>
86+
linkedResource.data !== 'NODATA' ? Observable.defer(() =>
8687
API.createResource({
8788
name: `${id}-${attributeName}-${uuid()}`
8889
},
@@ -98,7 +99,7 @@ const createLinkedResource = (id, attributeName, linkedResource, permission, API
9899
...(permission ? [updateResourcePermissions(linkedResourceId, permission, API)] : [])
99100

100101
]).map(() => linkedResourceId)
101-
);
102+
) : Observable.of(-1);
102103

103104
/**
104105
* Updates a linked resource. Check if the resource already exists as attribute.
@@ -121,15 +122,17 @@ const updateLinkedResource = (id, attributeName, linkedResource, permission, API
121122
: createLinkedResource(id, attributeName, linkedResource, permission, API)
122123
).catch(
123124
/* if the attribute doesn't exists or if the linked resource update gave an error
124-
* you have to create a new resource for the linked resource.
125+
* you have to create a new resource for the linked resource, provided it has valid data.
125126
* This error can occur if:
126127
* - The resource is new
127128
* - The resource URL is present as attribute of the main resource but the linked resource doesn't exist anymore.
128129
* ( for instance it may happen if the creation procedure gives an error )
129130
* - The resource is not writable by the user. It happens when a user changes the permission of a resource and doesn't update
130131
* the resource permission.
131132
*/
132-
(e) => createLinkedResource(id, attributeName, linkedResource, permission, API, e)
133+
(e) => linkedResource && linkedResource.data && linkedResource.data !== 'NODATA' ?
134+
createLinkedResource(id, attributeName, linkedResource, permission, API, e) :
135+
Observable.of(-1)
133136
);
134137
/**
135138
* Updates the permission of the linkedResources that are not modified.
@@ -291,9 +294,11 @@ export const createCategory = (category, API = GeoStoreDAO) =>
291294
* @return an observable that emits the id of the updated resource
292295
*/
293296

294-
export const updateResource = ({ id, data, permission, metadata, linkedResources = {} } = {}, API = GeoStoreDAO) =>
295-
// update metadata
296-
Observable.forkJoin([
297+
export const updateResource = ({ id, data, permission, metadata, linkedResources = {} } = {}, API = GeoStoreDAO) => {
298+
const linkedResourcesKeys = Object.keys(linkedResources);
299+
300+
// update metadata
301+
return Observable.forkJoin([
297302
// update data and permissions after data updated
298303
Observable.defer(
299304
() => API.putResourceMetadataAndAttributes(id, metadata)
@@ -304,17 +309,18 @@ export const updateResource = ({ id, data, permission, metadata, linkedResources
304309
() => API.putResource(id, data)
305310
)
306311
: Observable.of(res))
307-
.switchMap((res)=> permission ? Observable.defer(()=>updateResourcePermissions(id, permission, API)) : Observable.of(res)),
312+
.switchMap((res) => permission ? Observable.defer(() => updateResourcePermissions(id, permission, API)) : Observable.of(res)),
308313
// update linkedResources and permissions after linkedResources updated
309-
...(
310-
Object.keys(linkedResources).map(
314+
(linkedResourcesKeys.length > 0 ? Observable.forkJoin(
315+
...linkedResourcesKeys.map(
311316
attributeName => updateLinkedResource(id, attributeName, linkedResources[attributeName], permission, API)
312-
.switchMap((res)=>permission ?
313-
Observable.defer(()=> updateOtherLinkedResourcesPermissions(id, linkedResources, permission, API))
314-
: Observable.of(res))
315317
)
316-
)
318+
) : Observable.of([]))
319+
.switchMap(() => permission ?
320+
Observable.defer(() => updateOtherLinkedResourcesPermissions(id, linkedResources, permission, API)) :
321+
Observable.of(-1))
317322
]).map(() => id);
323+
};
318324

319325
/**
320326
* Deletes a resource and Its linked attributes

0 commit comments

Comments
 (0)