From 390d94c9763b22ce85f4f2f2169aca40ad422554 Mon Sep 17 00:00:00 2001 From: Marie Grosjean Date: Thu, 15 Apr 2021 10:31:36 +0200 Subject: [PATCH 1/4] Update roadmap-grscicoll.md --- roadmap-grscicoll.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roadmap-grscicoll.md b/roadmap-grscicoll.md index 44e24167ce..ae26e21dbf 100644 --- a/roadmap-grscicoll.md +++ b/roadmap-grscicoll.md @@ -14,7 +14,7 @@ The connection with Index Herbariorum and import of iDigBio enriched the catalog The current processes are weak, and don’t capture the proposed change in a structured manner. -* Develop an interface allowing anyone to propose a change to any/all fields and state whether they have authority to approve it. Changes are then to be reviewed and applied by the editorial team [[REG-299](https://github.com/gbif/registry/issues/299)]. +* Develop an interface allowing anyone to propose a change to any/all fields and state whether they have authority to approve it. Changes are then to be reviewed and applied by the editorial team [[REG-CONSOLE-376]https://github.com/gbif/registry-console/issues/376)]. ## Improve documentation From 00e86810d3e5dcb67544b58db0922fad246d4d3f Mon Sep 17 00:00:00 2001 From: Marie Grosjean Date: Thu, 15 Apr 2021 10:32:09 +0200 Subject: [PATCH 2/4] Update roadmap-grscicoll.md --- roadmap-grscicoll.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roadmap-grscicoll.md b/roadmap-grscicoll.md index ae26e21dbf..d09662075c 100644 --- a/roadmap-grscicoll.md +++ b/roadmap-grscicoll.md @@ -14,7 +14,7 @@ The connection with Index Herbariorum and import of iDigBio enriched the catalog The current processes are weak, and don’t capture the proposed change in a structured manner. -* Develop an interface allowing anyone to propose a change to any/all fields and state whether they have authority to approve it. Changes are then to be reviewed and applied by the editorial team [[REG-CONSOLE-376]https://github.com/gbif/registry-console/issues/376)]. +* Develop an interface allowing anyone to propose a change to any/all fields and state whether they have authority to approve it. Changes are then to be reviewed and applied by the editorial team [[REG-CONSOLE-376](https://github.com/gbif/registry-console/issues/376)]. ## Improve documentation From 927f0a81161c7501d56b043cec0b68b41a0b880a Mon Sep 17 00:00:00 2001 From: fmendezh Date: Thu, 29 Apr 2021 12:20:59 +0200 Subject: [PATCH 3/4] adding missing checks and using Object.equals which is null-aware --- .../gbif/registry/search/DatasetIndexUpdateListener.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/registry-events/src/main/java/org/gbif/registry/search/DatasetIndexUpdateListener.java b/registry-events/src/main/java/org/gbif/registry/search/DatasetIndexUpdateListener.java index 7225432d37..5cd72d39fd 100644 --- a/registry-events/src/main/java/org/gbif/registry/search/DatasetIndexUpdateListener.java +++ b/registry-events/src/main/java/org/gbif/registry/search/DatasetIndexUpdateListener.java @@ -29,6 +29,7 @@ import org.gbif.registry.search.dataset.indexing.DatasetRealtimeIndexer; import org.gbif.ws.NotFoundException; +import java.util.Objects; import java.util.UUID; import org.slf4j.Logger; @@ -72,7 +73,8 @@ public final void updated(UpdateEvent event) { // we only care about title & country changes Organization org1 = (Organization) event.getOldObject(); Organization org2 = (Organization) event.getNewObject(); - if (!org1.getTitle().equals(org2.getTitle()) || org1.getCountry() != org2.getCountry()) { + if (!Objects.equals(org1.getTitle(), org2.getTitle()) || + !Objects.equals(org1.getCountry(), org2.getCountry())) { indexService.index(org2); } @@ -80,14 +82,15 @@ public final void updated(UpdateEvent event) { // we only care about the hosting organization Installation i1 = (Installation) event.getOldObject(); Installation i2 = (Installation) event.getNewObject(); - if (!i1.getOrganizationKey().equals(i2.getOrganizationKey())) { + if (!Objects.equals(i1.getOrganizationKey(), (i2.getOrganizationKey())) || + !Objects.equals(i1.getTitle(), i2.getTitle())) { indexService.index(i2); } } else if (event.getObjectClass().equals(Network.class)) { // we only care about title changes Network network1 = (Network)event.getOldObject(); Network network2 = (Network)event.getNewObject(); - if (!network1.getTitle().equals(network2.getTitle())) { + if (!Objects.equals(network1.getTitle(), network2.getTitle())) { indexService.index(network2); } } From 447408e82a3c8fb4c1b4fcf1741ce59fd42d9422 Mon Sep 17 00:00:00 2001 From: Marcos Lopez Gonzalez Date: Thu, 29 Apr 2021 12:59:03 +0200 Subject: [PATCH 4/4] removed PUT without key endpoint + checking key mismatches in PUTs for all entities --- .../ws/it/collections/BaseCollectionEntityIT.java | 2 +- .../registry/ws/client/NetworkEntityClient.java | 11 +++++++++-- .../registry/ws/client/collections/CrudClient.java | 4 ++-- .../ws/resources/BaseNetworkEntityResource.java | 13 +++++++++---- .../gbif/registry/ws/resources/DatasetResource.java | 9 +-------- .../ExtendedCollectionEntityResource.java | 6 ++++-- .../ws/resources/collections/PersonResource.java | 5 ++++- 7 files changed, 30 insertions(+), 20 deletions(-) diff --git a/registry-integration-tests/src/test/java/org/gbif/registry/ws/it/collections/BaseCollectionEntityIT.java b/registry-integration-tests/src/test/java/org/gbif/registry/ws/it/collections/BaseCollectionEntityIT.java index 7c98c9c623..4a136c6306 100644 --- a/registry-integration-tests/src/test/java/org/gbif/registry/ws/it/collections/BaseCollectionEntityIT.java +++ b/registry-integration-tests/src/test/java/org/gbif/registry/ws/it/collections/BaseCollectionEntityIT.java @@ -186,7 +186,7 @@ public void updateInvalidEntityTest(ServiceType serviceType) { T newEntity = newInvalidEntity(); newEntity.setKey(key); - assertThrows(Exception.class, () -> service.update(newEntity)); + assertThrows(ValidationException.class, () -> service.update(newEntity)); } @ParameterizedTest diff --git a/registry-ws-client/src/main/java/org/gbif/registry/ws/client/NetworkEntityClient.java b/registry-ws-client/src/main/java/org/gbif/registry/ws/client/NetworkEntityClient.java index 81d92c801f..03742e1fd0 100644 --- a/registry-ws-client/src/main/java/org/gbif/registry/ws/client/NetworkEntityClient.java +++ b/registry-ws-client/src/main/java/org/gbif/registry/ws/client/NetworkEntityClient.java @@ -58,9 +58,16 @@ public interface NetworkEntityClient extends NetworkEnt @Override PagingResponse list(@SpringQueryMap Pageable page); - @RequestMapping(method = RequestMethod.PUT, consumes = MediaType.APPLICATION_JSON_VALUE) @Override - void update(@RequestBody T entity); + default void update(@RequestBody T entity) { + updateResource(entity.getKey(), entity); + } + + @RequestMapping( + method = RequestMethod.PUT, + value = "{key}", + consumes = MediaType.APPLICATION_JSON_VALUE) + void updateResource(@PathVariable("key") UUID key, @RequestBody T entity); @RequestMapping( method = RequestMethod.GET, diff --git a/registry-ws-client/src/main/java/org/gbif/registry/ws/client/collections/CrudClient.java b/registry-ws-client/src/main/java/org/gbif/registry/ws/client/collections/CrudClient.java index 10427f7e74..773631f098 100644 --- a/registry-ws-client/src/main/java/org/gbif/registry/ws/client/collections/CrudClient.java +++ b/registry-ws-client/src/main/java/org/gbif/registry/ws/client/collections/CrudClient.java @@ -47,12 +47,12 @@ public interface CrudClient extends CrudService { @Override default void update(@RequestBody T entity) { - updateEntity(entity.getKey(), entity); + updateResource(entity.getKey(), entity); } @RequestMapping( method = RequestMethod.PUT, value = "{key}", consumes = MediaType.APPLICATION_JSON_VALUE) - void updateEntity(@PathVariable("key") UUID key, @RequestBody T entity); + void updateResource(@PathVariable("key") UUID key, @RequestBody T entity); } diff --git a/registry-ws/src/main/java/org/gbif/registry/ws/resources/BaseNetworkEntityResource.java b/registry-ws/src/main/java/org/gbif/registry/ws/resources/BaseNetworkEntityResource.java index 0c78e897ea..60125d2ccc 100644 --- a/registry-ws/src/main/java/org/gbif/registry/ws/resources/BaseNetworkEntityResource.java +++ b/registry-ws/src/main/java/org/gbif/registry/ws/resources/BaseNetworkEntityResource.java @@ -53,6 +53,7 @@ import java.util.UUID; import javax.annotation.Nullable; +import javax.validation.Valid; import javax.validation.groups.Default; import org.slf4j.Logger; @@ -76,6 +77,7 @@ import com.google.common.base.Strings; import com.google.common.collect.Maps; +import static com.google.common.base.Preconditions.checkArgument; import static org.gbif.registry.security.UserRoles.ADMIN_ROLE; import static org.gbif.registry.security.UserRoles.APP_ROLE; import static org.gbif.registry.security.UserRoles.EDITOR_ROLE; @@ -229,15 +231,18 @@ public PagingResponse listByIdentifier(String identifier, Pageable page) { * * @param entity entity that extends NetworkEntity */ - @PutMapping( - value = {"", "{key}"}, - consumes = MediaType.APPLICATION_JSON_VALUE) + @PutMapping(value = "{key}", consumes = MediaType.APPLICATION_JSON_VALUE) @Validated({PostPersist.class, Default.class}) @Trim @Transactional @Secured({ADMIN_ROLE, EDITOR_ROLE, IPT_ROLE}) + public void update(@PathVariable("key") UUID key, @Valid @RequestBody @Trim T entity) { + checkArgument(key.equals(entity.getKey())); + update(entity); + } + @Override - public void update(@RequestBody @Trim T entity) { + public void update(T entity) { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); if (authentication != null) { entity.setModifiedBy(authentication.getName()); diff --git a/registry-ws/src/main/java/org/gbif/registry/ws/resources/DatasetResource.java b/registry-ws/src/main/java/org/gbif/registry/ws/resources/DatasetResource.java index 71c0d5337a..9bd477ddf0 100644 --- a/registry-ws/src/main/java/org/gbif/registry/ws/resources/DatasetResource.java +++ b/registry-ws/src/main/java/org/gbif/registry/ws/resources/DatasetResource.java @@ -31,7 +31,6 @@ import org.gbif.api.model.registry.LenientEquals; import org.gbif.api.model.registry.Metadata; import org.gbif.api.model.registry.Network; -import org.gbif.api.model.registry.PostPersist; import org.gbif.api.model.registry.PrePersist; import org.gbif.api.model.registry.Tag; import org.gbif.api.model.registry.search.DatasetSearchParameter; @@ -551,14 +550,8 @@ public UUID create(@RequestBody @Trim Dataset dataset) { return key; } - @PutMapping( - value = {"", "{key}"}, - consumes = MediaType.APPLICATION_JSON_VALUE) - @Validated({PostPersist.class, Default.class}) - @Trim - @Secured({ADMIN_ROLE, EDITOR_ROLE, IPT_ROLE}) @Override - public void update(@RequestBody @Trim Dataset dataset) { + public void update(Dataset dataset) { Dataset old = super.get(dataset.getKey()); if (old == null) { throw new IllegalArgumentException("Dataset " + dataset.getKey() + " not existing"); diff --git a/registry-ws/src/main/java/org/gbif/registry/ws/resources/collections/ExtendedCollectionEntityResource.java b/registry-ws/src/main/java/org/gbif/registry/ws/resources/collections/ExtendedCollectionEntityResource.java index c25276e842..79bc2d93c6 100644 --- a/registry-ws/src/main/java/org/gbif/registry/ws/resources/collections/ExtendedCollectionEntityResource.java +++ b/registry-ws/src/main/java/org/gbif/registry/ws/resources/collections/ExtendedCollectionEntityResource.java @@ -43,6 +43,7 @@ import java.util.UUID; import javax.annotation.Nullable; +import javax.validation.Valid; import javax.validation.constraints.NotNull; import javax.validation.groups.Default; @@ -172,10 +173,11 @@ public UUID create(@RequestBody @Trim T entity) { } @PutMapping(value = "{key}", consumes = MediaType.APPLICATION_JSON_VALUE) - @Trim + @Validated({PostPersist.class, Default.class}) @Transactional + @Trim @Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE}) - public void update(@PathVariable("key") UUID key, @RequestBody @Trim T entity) { + public void update(@PathVariable("key") UUID key, @Valid @RequestBody @Trim T entity) { checkArgument(key.equals(entity.getKey())); update(entity); } diff --git a/registry-ws/src/main/java/org/gbif/registry/ws/resources/collections/PersonResource.java b/registry-ws/src/main/java/org/gbif/registry/ws/resources/collections/PersonResource.java index f467ffdfef..fdeb83e8ca 100644 --- a/registry-ws/src/main/java/org/gbif/registry/ws/resources/collections/PersonResource.java +++ b/registry-ws/src/main/java/org/gbif/registry/ws/resources/collections/PersonResource.java @@ -23,6 +23,7 @@ import org.gbif.api.model.common.paging.PagingResponse; import org.gbif.api.model.registry.Identifier; import org.gbif.api.model.registry.MachineTag; +import org.gbif.api.model.registry.PostPersist; import org.gbif.api.model.registry.PrePersist; import org.gbif.api.model.registry.Tag; import org.gbif.api.model.registry.search.collections.PersonSuggestResult; @@ -42,6 +43,7 @@ import java.util.UUID; import javax.annotation.Nullable; +import javax.validation.Valid; import javax.validation.groups.Default; import org.springframework.http.MediaType; @@ -155,10 +157,11 @@ public UUID create(@RequestBody @Trim Person person) { } @PutMapping(value = "{key}", consumes = MediaType.APPLICATION_JSON_VALUE) + @Validated({PostPersist.class, Default.class}) @Trim @Transactional @Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE}) - public void update(@PathVariable("key") UUID key, @RequestBody @Trim Person entity) { + public void update(@PathVariable("key") UUID key, @Valid @RequestBody @Trim Person entity) { checkArgument(key.equals(entity.getKey())); update(entity); }