Allow DynamicUpdate making the (erreneous) updatePartial method and PATCH interface redundant

dnlkoch · dnlkoch · commit ea2d9235e8a2 · 2022-05-13T16:50:47.000+02:00
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/controller/BaseController.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/controller/BaseController.java
@@ -33,7 +33,6 @@
 
 import java.time.OffsetDateTime;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.UUID;
 
@@ -597,80 +596,6 @@ public S update(@RequestBody S entity, @PathVariable("id") Long entityId) {
         }
     }
 
-    @PatchMapping("/{id}")
-    @ResponseStatus(HttpStatus.OK)
-    public S updatePartial(@RequestBody Map<String, Object> values, @PathVariable("id") Long entityId) {
-        log.trace("Requested to partially update entity of type {} with ID {} ({})", getGenericClassName(), entityId, values);
-
-        try {
-            Object idFromValues = values.get("id");
-            if (idFromValues == null) {
-                log.error("Field 'id' (entity {})is missing in the passed values: {}.", entityId, values);
-                throw new ResponseStatusException(HttpStatus.BAD_REQUEST);
-            }
-            Long id = Long.valueOf((Integer) idFromValues);
-            if (!entityId.equals(id)) {
-                log.error("IDs of update candidate (ID: {}) and update data ({}) don't match.", entityId, values);
-                throw new ResponseStatusException(HttpStatus.BAD_REQUEST);
-            }
-
-            S persistedEntity = service.findOne(entityId).orElseThrow();
-            if (persistedEntity != null) {
-                S updatedEntity = service.updatePartial(entityId, persistedEntity, values);
-
-                log.trace("Successfully updated values for entity of type {} with ID {}",
-                    getGenericClassName(), entityId);
-
-                return updatedEntity;
-            } else {
-                log.error("Could not find entity of type {} with ID {}",
-                    getGenericClassName(), entityId);
-
-                throw new ResponseStatusException(
-                        HttpStatus.NOT_FOUND,
-                        messageSource.getMessage(
-                                "BaseController.NOT_FOUND",
-                                null,
-                                LocaleContextHolder.getLocale()
-                        )
-                );
-            }
-        } catch (AccessDeniedException ade) {
-            log.warn("Updating values of type {} with ID {} is denied",
-                getGenericClassName(), entityId);
-
-            throw new ResponseStatusException(
-                    HttpStatus.NOT_FOUND,
-                    messageSource.getMessage(
-                            "BaseController.NOT_FOUND",
-                            null,
-                            LocaleContextHolder.getLocale()
-                    ),
-                    ade
-            );
-        } catch (NumberFormatException nfe) {
-            log.error("Can't parse 'id' field ({}) from values ({}). It has to be an Integer.: {}",
-                values, entityId, nfe.getMessage());
-            throw new ResponseStatusException(HttpStatus.BAD_REQUEST);
-        } catch (ResponseStatusException rse) {
-            throw rse;
-        } catch (Exception e) {
-            log.error("Error while updating values for entity of type {} with ID {}: \n {}",
-                    getGenericClassName(), entityId, e.getMessage());
-            log.trace("Full stack trace: ", e);
-
-            throw new ResponseStatusException(
-                    HttpStatus.INTERNAL_SERVER_ERROR,
-                    messageSource.getMessage(
-                            "BaseController.INTERNAL_SERVER_ERROR",
-                            null,
-                            LocaleContextHolder.getLocale()
-                    ),
-                    e
-            );
-        }
-    }
-
     @DeleteMapping("/{id}")
     @ResponseStatus(HttpStatus.NO_CONTENT)
     public void delete(@PathVariable("id") Long entityId) {
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/Application.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/Application.java
@@ -24,6 +24,7 @@
 import lombok.*;
 import org.hibernate.annotations.Cache;
 import org.hibernate.annotations.CacheConcurrencyStrategy;
+import org.hibernate.annotations.DynamicUpdate;
 import org.hibernate.annotations.Type;
 import org.hibernate.envers.AuditTable;
 import org.hibernate.envers.Audited;
@@ -33,6 +34,7 @@
 
 @Entity(name = "applications")
 @Table(schema = "shogun")
+@DynamicUpdate
 @Audited
 @AuditTable(value = "applications_rev", schema = "shogun_rev")
 @Cacheable
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/File.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/File.java
@@ -21,13 +21,13 @@
 import lombok.*;
 import org.hibernate.annotations.Cache;
 import org.hibernate.annotations.CacheConcurrencyStrategy;
-import org.hibernate.annotations.Type;
+import org.hibernate.annotations.DynamicUpdate;
 
 import javax.persistence.*;
-import java.util.UUID;
 
 @Entity(name = "files")
 @Table(schema = "shogun")
+@DynamicUpdate
 @Cacheable
 @Cache(usage = CacheConcurrencyStrategy.READ_WRITE, region = "files")
 @AllArgsConstructor
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/Group.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/Group.java
@@ -20,13 +20,15 @@
 import lombok.*;
 import org.hibernate.annotations.Cache;
 import org.hibernate.annotations.CacheConcurrencyStrategy;
+import org.hibernate.annotations.DynamicUpdate;
 import org.hibernate.envers.AuditTable;
 import org.hibernate.envers.Audited;
 
 import javax.persistence.*;
 
 @Entity(name = "groups")
 @Table(schema = "shogun")
+@DynamicUpdate
 @Audited
 @AuditTable(value = "groups_rev", schema = "shogun_rev")
 @Cacheable
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/ImageFile.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/ImageFile.java
@@ -17,20 +17,18 @@
 package de.terrestris.shogun.lib.model;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
+import io.swagger.v3.oas.annotations.media.Schema;
+import lombok.*;
+import org.hibernate.annotations.DynamicUpdate;
+
 import javax.persistence.Cacheable;
 import javax.persistence.Column;
 import javax.persistence.Entity;
 import javax.persistence.Table;
 
-import io.swagger.v3.oas.annotations.media.Schema;
-import lombok.AllArgsConstructor;
-import lombok.Data;
-import lombok.EqualsAndHashCode;
-import lombok.NoArgsConstructor;
-import lombok.ToString;
-
 @Entity(name = "imagefiles")
 @Table(schema = "shogun")
+@DynamicUpdate
 @Cacheable
 @Data
 @AllArgsConstructor
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/Layer.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/Layer.java
@@ -24,6 +24,7 @@
 import org.geojson.GeoJsonObject;
 import org.hibernate.annotations.Cache;
 import org.hibernate.annotations.CacheConcurrencyStrategy;
+import org.hibernate.annotations.DynamicUpdate;
 import org.hibernate.annotations.Type;
 import org.hibernate.envers.AuditTable;
 import org.hibernate.envers.Audited;
@@ -32,6 +33,7 @@
 
 @Entity(name = "layers")
 @Table(schema = "shogun")
+@DynamicUpdate
 @Audited
 @AuditTable(value = "layers_rev", schema = "shogun_rev")
 @Cacheable
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/User.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/User.java
@@ -22,6 +22,7 @@
 import lombok.*;
 import org.hibernate.annotations.Cache;
 import org.hibernate.annotations.CacheConcurrencyStrategy;
+import org.hibernate.annotations.DynamicUpdate;
 import org.hibernate.annotations.Type;
 import org.hibernate.envers.AuditTable;
 import org.hibernate.envers.Audited;
@@ -30,6 +31,7 @@
 
 @Entity(name = "users")
 @Table(schema = "shogun")
+@DynamicUpdate
 @Audited
 @AuditTable(value = "users_rev", schema = "shogun_rev")
 @Cacheable
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/security/permission/GroupClassPermission.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/security/permission/GroupClassPermission.java
@@ -17,22 +17,21 @@
 package de.terrestris.shogun.lib.model.security.permission;
 
 import de.terrestris.shogun.lib.model.Group;
-import javax.persistence.Cacheable;
-import javax.persistence.Entity;
-import javax.persistence.ManyToOne;
-import javax.persistence.Table;
-import lombok.AllArgsConstructor;
-import lombok.Data;
-import lombok.EqualsAndHashCode;
-import lombok.NoArgsConstructor;
-import lombok.ToString;
+import lombok.*;
 import org.hibernate.annotations.Cache;
 import org.hibernate.annotations.CacheConcurrencyStrategy;
+import org.hibernate.annotations.DynamicUpdate;
 import org.hibernate.envers.AuditTable;
 import org.hibernate.envers.Audited;
 
+import javax.persistence.Cacheable;
+import javax.persistence.Entity;
+import javax.persistence.ManyToOne;
+import javax.persistence.Table;
+
 @Entity(name = "groupclasspermissions")
 @Table(schema = "shogun")
+@DynamicUpdate
 @Audited
 @AuditTable(value = "groupclasspermissions_rev", schema = "shogun_rev")
 @Cacheable
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/security/permission/GroupInstancePermission.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/security/permission/GroupInstancePermission.java
@@ -17,22 +17,21 @@
 package de.terrestris.shogun.lib.model.security.permission;
 
 import de.terrestris.shogun.lib.model.Group;
-import javax.persistence.Cacheable;
-import javax.persistence.Entity;
-import javax.persistence.ManyToOne;
-import javax.persistence.Table;
-import lombok.AllArgsConstructor;
-import lombok.Data;
-import lombok.EqualsAndHashCode;
-import lombok.NoArgsConstructor;
-import lombok.ToString;
+import lombok.*;
 import org.hibernate.annotations.Cache;
 import org.hibernate.annotations.CacheConcurrencyStrategy;
+import org.hibernate.annotations.DynamicUpdate;
 import org.hibernate.envers.AuditTable;
 import org.hibernate.envers.Audited;
 
+import javax.persistence.Cacheable;
+import javax.persistence.Entity;
+import javax.persistence.ManyToOne;
+import javax.persistence.Table;
+
 @Entity(name = "groupinstancepermissions")
 @Table(schema = "shogun")
+@DynamicUpdate
 @Audited
 @AuditTable(value = "groupinstancepermissions_rev", schema = "shogun_rev")
 @Cacheable
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/security/permission/PermissionCollection.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/security/permission/PermissionCollection.java
@@ -19,27 +19,22 @@
 import de.terrestris.shogun.lib.enumeration.PermissionCollectionType;
 import de.terrestris.shogun.lib.enumeration.PermissionType;
 import de.terrestris.shogun.lib.model.BaseEntity;
-import java.util.HashSet;
-import java.util.Set;
-import javax.persistence.Cacheable;
-import javax.persistence.CollectionTable;
-import javax.persistence.Column;
-import javax.persistence.ElementCollection;
-import javax.persistence.Entity;
-import javax.persistence.EnumType;
-import javax.persistence.Enumerated;
-import javax.persistence.Table;
 import lombok.AllArgsConstructor;
 import lombok.Data;
 import lombok.EqualsAndHashCode;
 import lombok.NoArgsConstructor;
+import org.hibernate.annotations.*;
 import org.hibernate.annotations.Cache;
-import org.hibernate.annotations.CacheConcurrencyStrategy;
-import org.hibernate.annotations.Fetch;
-import org.hibernate.annotations.FetchMode;
+
+import javax.persistence.*;
+import javax.persistence.Entity;
+import javax.persistence.Table;
+import java.util.HashSet;
+import java.util.Set;
 
 @Entity(name = "permissions")
 @Table(schema = "shogun")
+@DynamicUpdate
 @Cacheable
 @Cache(usage = CacheConcurrencyStrategy.READ_WRITE, region = "permissions")
 @Data
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/security/permission/UserClassPermission.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/security/permission/UserClassPermission.java
@@ -17,22 +17,21 @@
 package de.terrestris.shogun.lib.model.security.permission;
 
 import de.terrestris.shogun.lib.model.User;
-import javax.persistence.Cacheable;
-import javax.persistence.Entity;
-import javax.persistence.ManyToOne;
-import javax.persistence.Table;
-import lombok.AllArgsConstructor;
-import lombok.Data;
-import lombok.EqualsAndHashCode;
-import lombok.NoArgsConstructor;
-import lombok.ToString;
+import lombok.*;
 import org.hibernate.annotations.Cache;
 import org.hibernate.annotations.CacheConcurrencyStrategy;
+import org.hibernate.annotations.DynamicUpdate;
 import org.hibernate.envers.AuditTable;
 import org.hibernate.envers.Audited;
 
+import javax.persistence.Cacheable;
+import javax.persistence.Entity;
+import javax.persistence.ManyToOne;
+import javax.persistence.Table;
+
 @Entity(name = "userclasspermissions")
 @Table(schema = "shogun")
+@DynamicUpdate
 @Audited
 @AuditTable(value = "userclasspermissions_rev", schema = "shogun_rev")
 @Cacheable
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/security/permission/UserInstancePermission.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/model/security/permission/UserInstancePermission.java
@@ -17,22 +17,21 @@
 package de.terrestris.shogun.lib.model.security.permission;
 
 import de.terrestris.shogun.lib.model.User;
-import javax.persistence.Cacheable;
-import javax.persistence.Entity;
-import javax.persistence.ManyToOne;
-import javax.persistence.Table;
-import lombok.AllArgsConstructor;
-import lombok.Data;
-import lombok.EqualsAndHashCode;
-import lombok.NoArgsConstructor;
-import lombok.ToString;
+import lombok.*;
 import org.hibernate.annotations.Cache;
 import org.hibernate.annotations.CacheConcurrencyStrategy;
+import org.hibernate.annotations.DynamicUpdate;
 import org.hibernate.envers.AuditTable;
 import org.hibernate.envers.Audited;
 
+import javax.persistence.Cacheable;
+import javax.persistence.Entity;
+import javax.persistence.ManyToOne;
+import javax.persistence.Table;
+
 @Entity(name = "userinstancepermissions")
 @Table(schema = "shogun")
+@DynamicUpdate
 @Audited
 @AuditTable(value = "userinstancepermissions_rev", schema = "shogun_rev")
 @Cacheable
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/service/BaseService.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/service/BaseService.java
@@ -140,28 +140,11 @@ public S create(S entity) {
     public S update(Long id, S entity) throws IOException {
         Optional<S> persistedEntityOpt = repository.findById(id);
 
-        ObjectNode jsonObject = objectMapper.valueToTree(entity);
-
         // Ensure the created timestamp will not be overridden.
         S persistedEntity = persistedEntityOpt.orElseThrow();
-        OffsetDateTime createdTimestamp = persistedEntity.getCreated();
-        String serialized = createdTimestamp != null ? createdTimestamp.toInstant().toString() : null;
-        jsonObject.put("created", serialized);
-
-        S updatedEntity = objectMapper.readerForUpdating(persistedEntity).readValue(jsonObject);
-
-        return repository.save(updatedEntity);
-    }
+        entity.setCreated(persistedEntity.getCreated());
 
-    @PreAuthorize("hasRole('ROLE_ADMIN') or hasPermission(#entity, 'UPDATE')")
-    @Transactional(isolation = Isolation.SERIALIZABLE)
-    public S updatePartial(Long entityId, S entity, Map<String, Object> values) throws IOException {
-        if (ObjectUtils.notEqual(entityId, entity.getId())) {
-            throw new IOException("ID's of passed entity and parameter do not match. No partial update possible");
-        }
-        JsonNode jsonObject = objectMapper.valueToTree(values);
-        S updatedEntity = objectMapper.readerForUpdating(entity).readValue(jsonObject);
-        return repository.save(updatedEntity);
+        return repository.save(entity);
     }
 
     @PreAuthorize("hasRole('ROLE_ADMIN') or hasPermission(#entity, 'DELETE')")
diff --git a/shogun-lib/src/test/java/de/terrestris/shogun/lib/service/BaseServiceTest.java b/shogun-lib/src/test/java/de/terrestris/shogun/lib/service/BaseServiceTest.java
@@ -178,10 +178,6 @@ public void update_ShouldCallCorrectRepositoryMethodAndShouldReturnTheCreatedEnt
         when(baseCrudRepositoryMock.findById(1909L)).thenReturn(Optional.of(mockEntity));
         when(baseCrudRepositoryMock.save(mockEntity)).thenReturn(mockEntity);
 
-        when(objectMapperMock.valueToTree(mockEntity)).thenReturn(returnNode);
-        when(objectMapperMock.readerForUpdating(mockEntity)).thenReturn(objectReaderMock);
-        when(objectReaderMock.readValue(returnNode)).thenReturn(mockEntity);
-
         S returnValue = (S) service.update(1909L, mockEntity);
 
         verify(baseCrudRepositoryMock, times(1)).findById(1909L);
