From f1b1326e41bde2ff29c7383aefa4c81fc58df76f Mon Sep 17 00:00:00 2001 From: vlt1 <54526151+vlt1@users.noreply.github.com> Date: Mon, 10 Aug 2020 12:57:31 +0300 Subject: [PATCH] #4758: Resolve inconsistences with linkedResources and NODATA (#5760) --- .../components/resources/forms/Thumbnail.jsx | 1 - .../resources/modals/fragments/MainForm.jsx | 2 +- web/client/epics/__tests__/maps-test.js | 32 +++++++- web/client/epics/maps.js | 3 +- .../observables/__tests__/geostore-test.js | 73 ++++++++++++++++++- web/client/observables/geostore.js | 34 +++++---- 6 files changed, 126 insertions(+), 19 deletions(-) diff --git a/web/client/components/resources/forms/Thumbnail.jsx b/web/client/components/resources/forms/Thumbnail.jsx index 44ab67eeb0..e04100f86c 100644 --- a/web/client/components/resources/forms/Thumbnail.jsx +++ b/web/client/components/resources/forms/Thumbnail.jsx @@ -100,7 +100,6 @@ class Thumbnail extends React.Component { // with at least one error this.props.onError(errors, this.props.resource.id); this.files = files; - this.props.onUpdate(null, null); }} onRemove={() => { this.files = null; diff --git a/web/client/components/resources/modals/fragments/MainForm.jsx b/web/client/components/resources/modals/fragments/MainForm.jsx index d4bc85a89f..2ec2a6ace2 100644 --- a/web/client/components/resources/modals/fragments/MainForm.jsx +++ b/web/client/components/resources/modals/fragments/MainForm.jsx @@ -55,7 +55,7 @@ module.exports = class MainForm extends React.Component { onRemove={() => onUpdateLinkedResource("thumbnail", "NODATA", "THUMBNAIL", { tail: `/raw?decode=datauri&v=${uuid()}` })} - onUpdate={(data) => onUpdateLinkedResource("thumbnail", data, "THUMBNAIL", { + onUpdate={(data) => onUpdateLinkedResource("thumbnail", data || "NODATA", "THUMBNAIL", { tail: `/raw?decode=datauri&v=${uuid()}` })} /> diff --git a/web/client/epics/__tests__/maps-test.js b/web/client/epics/__tests__/maps-test.js index af66a0e624..638392f901 100644 --- a/web/client/epics/__tests__/maps-test.js +++ b/web/client/epics/__tests__/maps-test.js @@ -692,7 +692,7 @@ describe('Create and update flow using persistence api', () => { Persistence.setApi("geostore"); }); it('test create flow ', done => { - testEpic(addTimeoutEpic(mapSaveMapResourceEpic), 6, saveMapResource( {}), actions => { + testEpic(addTimeoutEpic(mapSaveMapResourceEpic), 6, saveMapResource({}), actions => { expect(actions.length).toBe(6); actions.map((action) => { switch (action.type) { @@ -748,6 +748,36 @@ describe('Create and update flow using persistence api', () => { done(); }); }); + it('test thumbnail attribute is ignored', done => { + const attributeTestApi = { + ...api, + updateResourceAttribute: ({name}) => name === 'thumbnail' ? done(new Error('thumbnail update request was issued!')) : Rx.Observable.of(10) + }; + Persistence.addApi('attributeTest', attributeTestApi); + Persistence.setApi('attributeTest'); + + testEpic(addTimeoutEpic(mapSaveMapResourceEpic), 6, saveMapResource({id: 10, metadata: {name: 'resource'}, attributes: {thumbnail: 'thumb', someAttribute: 'value'}}), actions => { + try { + expect(actions.length).toBe(6); + actions.map((action) => { + switch (action.type) { + case MAP_UPDATING: + case TOGGLE_CONTROL: + case MAP_SAVED: + case SHOW_NOTIFICATION: + case LOAD_MAP_INFO: + case MAP_CONFIG_LOADED: + break; + default: + expect(true).toBe(false); + } + }); + done(); + } catch (e) { + done(e); + } + }, {}); + }); it('test reloadMaps', function(done) { const callback = actions => { diff --git a/web/client/epics/maps.js b/web/client/epics/maps.js index 5a1fc8dc93..a77e002292 100644 --- a/web/client/epics/maps.js +++ b/web/client/epics/maps.js @@ -490,8 +490,9 @@ const mapSaveMapResourceEpic = (action$, store) => action$.ofType(SAVE_MAP_RESOURCE) .exhaustMap(({resource}) => { // filter out invalid attributes + // thumbnails are handled separately const validAttributesNames = keys(resource.attributes) - .filter(attrName => resource.attributes[attrName] !== undefined && resource.attributes[attrName] !== null); + .filter(attrName => attrName !== 'thumbnail' && resource.attributes[attrName] !== undefined && resource.attributes[attrName] !== null); return Rx.Observable.forkJoin( (() => { // get a context information using the id in the attribute diff --git a/web/client/observables/__tests__/geostore-test.js b/web/client/observables/__tests__/geostore-test.js index fb488656ad..d8d5fd8c77 100644 --- a/web/client/observables/__tests__/geostore-test.js +++ b/web/client/observables/__tests__/geostore-test.js @@ -8,7 +8,7 @@ const expect = require('expect'); const geoStoreMock = require('./geoStoreMock').default; -const {createResource, deleteResource, getResourceIdByName} = require('../geostore'); +const {createResource, deleteResource, getResourceIdByName, updateResource} = require('../geostore'); const testAndResolve = (test = () => {}, value) => (...args) => { test(...args); return Promise.resolve(value); @@ -132,5 +132,76 @@ describe('geostore observables for resources management', () => { }); }); + + it('updateResource linked resource is not created if no thumbnail attribute for resource and data is NODATA', done => { + const testResource = { + id: 10, + data: {}, + category: "TEST", + metadata: { + name: "RES2" + }, + linkedResources: { + thumbnail: { + tail: '/raw?decode=datauri', + data: "NODATA" + } + } + }; + const DummyAPI = { + getResourceAttribute: () => Promise.reject({status: 404}), + putResourceMetadataAndAttributes: () => Promise.resolve(10), + putResource: () => Promise.resolve(10), + createResource: ({name}) => name.search(/10-thumbnail/) !== -1 ? done(new Error('createResource for thumbnail is called!')) : Promise.resolve(11), + updateResourceAttribute: () => Promise.resolve(11) + }; + updateResource(testResource, DummyAPI).subscribe( + () => done(), + e => done(e) + ); + }); + + it('updateResource linked resource is created if data is valid', done => { + const testResource = { + id: 10, + data: {}, + category: "TEST", + metadata: { + name: "RES2" + }, + linkedResources: { + thumbnail: { + tail: '/raw?decode=datauri', + data: "data" + } + } + }; + + let createResourceThumbnail = false; + + const DummyAPI = { + getResourceAttribute: () => Promise.reject({status: 404}), + putResourceMetadataAndAttributes: () => Promise.resolve(10), + putResource: () => Promise.resolve(10), + createResource: ({name}) => { + if (name.search(/10-thumbnail/) !== -1) { + createResourceThumbnail = true; + } + return Promise.resolve(11); + }, + updateResourceAttribute: () => Promise.resolve(11) + }; + updateResource(testResource, DummyAPI).subscribe( + () => { + try { + expect(createResourceThumbnail).toBe(true); + done(); + } catch (e) { + done(e); + } + }, + e => done(e) + ); + }); }); }); diff --git a/web/client/observables/geostore.js b/web/client/observables/geostore.js index da91f1e19f..ef9a64ab61 100644 --- a/web/client/observables/geostore.js +++ b/web/client/observables/geostore.js @@ -76,13 +76,14 @@ const updateOrDeleteLinkedResource = (id, attributeName, linkedResource = {}, re /** * Creates a resource on GeoStore and link it to the current resource. Can be used for thumbnails, details and so on + * If the data for the resource is "NODATA", it won't be created. * @param {number} id the id of the resource * @param {string} attributeName the name of the attribute to link * @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 * @param {object} API the API to use (default GeoStoreDAO) */ const createLinkedResource = (id, attributeName, linkedResource, permission, API = GeoStoreDAO) => - Observable.defer(() => + linkedResource.data !== 'NODATA' ? Observable.defer(() => API.createResource({ name: `${id}-${attributeName}-${uuid()}` }, @@ -98,7 +99,7 @@ const createLinkedResource = (id, attributeName, linkedResource, permission, API ...(permission ? [updateResourcePermissions(linkedResourceId, permission, API)] : []) ]).map(() => linkedResourceId) - ); + ) : Observable.of(-1); /** * Updates a linked resource. Check if the resource already exists as attribute. @@ -121,7 +122,7 @@ const updateLinkedResource = (id, attributeName, linkedResource, permission, API : createLinkedResource(id, attributeName, linkedResource, permission, API) ).catch( /* if the attribute doesn't exists or if the linked resource update gave an error - * you have to create a new resource for the linked resource. + * you have to create a new resource for the linked resource, provided it has valid data. * This error can occur if: * - The resource is new * - The resource URL is present as attribute of the main resource but the linked resource doesn't exist anymore. @@ -129,7 +130,9 @@ const updateLinkedResource = (id, attributeName, linkedResource, permission, API * - The resource is not writable by the user. It happens when a user changes the permission of a resource and doesn't update * the resource permission. */ - (e) => createLinkedResource(id, attributeName, linkedResource, permission, API, e) + (e) => linkedResource && linkedResource.data && linkedResource.data !== 'NODATA' ? + createLinkedResource(id, attributeName, linkedResource, permission, API, e) : + Observable.of(-1) ); /** * Updates the permission of the linkedResources that are not modified. @@ -291,9 +294,11 @@ export const createCategory = (category, API = GeoStoreDAO) => * @return an observable that emits the id of the updated resource */ -export const updateResource = ({ id, data, permission, metadata, linkedResources = {} } = {}, API = GeoStoreDAO) => -// update metadata - Observable.forkJoin([ +export const updateResource = ({ id, data, permission, metadata, linkedResources = {} } = {}, API = GeoStoreDAO) => { + const linkedResourcesKeys = Object.keys(linkedResources); + + // update metadata + return Observable.forkJoin([ // update data and permissions after data updated Observable.defer( () => API.putResourceMetadataAndAttributes(id, metadata) @@ -304,17 +309,18 @@ export const updateResource = ({ id, data, permission, metadata, linkedResources () => API.putResource(id, data) ) : Observable.of(res)) - .switchMap((res)=> permission ? Observable.defer(()=>updateResourcePermissions(id, permission, API)) : Observable.of(res)), + .switchMap((res) => permission ? Observable.defer(() => updateResourcePermissions(id, permission, API)) : Observable.of(res)), // update linkedResources and permissions after linkedResources updated - ...( - Object.keys(linkedResources).map( + (linkedResourcesKeys.length > 0 ? Observable.forkJoin( + ...linkedResourcesKeys.map( attributeName => updateLinkedResource(id, attributeName, linkedResources[attributeName], permission, API) - .switchMap((res)=>permission ? - Observable.defer(()=> updateOtherLinkedResourcesPermissions(id, linkedResources, permission, API)) - : Observable.of(res)) ) - ) + ) : Observable.of([])) + .switchMap(() => permission ? + Observable.defer(() => updateOtherLinkedResourcesPermissions(id, linkedResources, permission, API)) : + Observable.of(-1)) ]).map(() => id); +}; /** * Deletes a resource and Its linked attributes