Skip to content

Commit

Permalink
#4758: Resolve inconsistences with linkedResources and NODATA (#5760)
Browse files Browse the repository at this point in the history
  • Loading branch information
vlt1 authored Aug 10, 2020
1 parent 24d6766 commit f1b1326
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 19 deletions.
1 change: 0 additions & 1 deletion web/client/components/resources/forms/Thumbnail.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()}`
})} />
</Col>
Expand Down
32 changes: 31 additions & 1 deletion web/client/epics/__tests__/maps-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 => {
Expand Down
3 changes: 2 additions & 1 deletion web/client/epics/maps.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
73 changes: 72 additions & 1 deletion web/client/observables/__tests__/geostore-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
);
});
});
});
34 changes: 20 additions & 14 deletions web/client/observables/geostore.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()}`
},
Expand All @@ -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.
Expand All @@ -121,15 +122,17 @@ 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.
* ( for instance it may happen if the creation procedure gives an error )
* - 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.
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down

0 comments on commit f1b1326

Please sign in to comment.