Add patch endpoint for global config (#1629)

Co-authored-by: Sebastian Sdorra <sebastian.sdorra@cloudogu.com>

gradle/changelog/config_patch_endpoint.yaml

@@ -0,0 +1,2 @@
+- type: added
+  description: Patch endpoint for global configuration ([#1629](https://github.com/scm-manager/scm-manager/pull/1629))

gradle/dependencies.gradle

@@ -1,7 +1,7 @@
 ext {
   slf4jVersion = '1.7.30'
   guiceVersion = '4.2.3'
-  resteasyVersion = '4.5.8.Final'
+  resteasyVersion = '4.6.0.Final'
 
   // TODO upgrade to 2.12.0, but this breaks openapi spec generation
   jacksonVersion = '2.11.3'
@@ -101,6 +101,7 @@ ext {
     ssp: "com.github.sdorra:ssp-lib:${sspVersion}",
     sspProcessor: "com.github.sdorra:ssp-processor:${sspVersion}",
     shiroUnit: 'com.github.sdorra:shiro-unit:1.0.1',
+    shiroExtension: 'com.github.sdorra:junit-shiro-extension:1.0.1',
 
     // jwt
     jjwtApi: "io.jsonwebtoken:jjwt-api:${jjwtVersion}",

scm-core/src/main/java/sonia/scm/util/JsonMerger.java

@@ -0,0 +1,214 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2020-present Cloudogu GmbH and Contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package sonia.scm.util;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import sonia.scm.web.api.DtoValidator;
+
+import javax.inject.Inject;
+import java.util.Iterator;
+
+/**
+ * This json merger can be used to apply various new fields to an existing object without overwriting
+ * the whole data or simply merge some json nodes into one.
+ *
+ * @since 2.18.0
+ */
+public class JsonMerger {
+
+  private final ObjectMapper objectMapper;
+
+  @Inject
+  public JsonMerger(ObjectMapper objectMapper) {
+    this.objectMapper = objectMapper;
+  }
+
+  /**
+   * Creates a MergerBuilder for the object.
+   *
+   * @param object object which should be updated
+   * @return merger builder
+   */
+  public MergeStage fromObject(Object object) {
+    JsonNode mainNode = objectMapper.valueToTree(object);
+    return new MergeStage(objectMapper, mainNode);
+  }
+
+  /**
+   * Creates a MergerBuilder for the json node.
+   *
+   * @param mainNode json node which should be updated
+   * @return merger builder
+   */
+  public MergeStage fromJson(JsonNode mainNode) {
+    return new MergeStage(objectMapper, mainNode);
+  }
+
+  public static class MergeStage {
+    private final ObjectMapper objectMapper;
+    private final JsonNode mainNode;
+
+    private MergeStage(ObjectMapper objectMapper, JsonNode node) {
+      this.objectMapper = objectMapper;
+      this.mainNode = node;
+    }
+
+    /**
+     * Merge object with main node
+     *
+     * @param object object which will be transformed to a json node in order to be merged with the main node
+     * @return this merge builder
+     */
+    public MergeStage mergeWithObject(Object object) {
+      JsonNode updateNode = objectMapper.valueToTree(object);
+      merge(mainNode, updateNode);
+      return this;
+    }
+
+    /**
+     * Merge json node with main node
+     *
+     * @param updateNode json node with data which should be applied to main node
+     * @return this merge builder
+     */
+    public MergeStage mergeWithJson(JsonNode updateNode) {
+      merge(mainNode, updateNode);
+      return this;
+    }
+
+    /**
+     * Returns the merged json node
+     *
+     * @return merged json node
+     */
+    public JsonNode toJsonNode() {
+      return mainNode;
+    }
+
+    /**
+     * Creates a specific object merger which can apply logic like validation after the actual merge.
+     *
+     * @param clazz class to which the output should be transformed
+     * @return object merger
+     */
+    public <T> ToObjectStage<T> toObject(Class<T> clazz) {
+      return new ToObjectStage<>(objectMapper, mainNode, clazz);
+    }
+
+    /**
+     * Updates the {@param mainNode} with the provided fields from the {@param updateNode}.
+     *
+     * @param mainNode   base nodes which will be updated
+     * @param updateNode updateNode that contains the new data
+     * @return updated json node
+     */
+    private JsonNode merge(JsonNode mainNode, JsonNode updateNode) {
+      Iterator<String> fieldNames = updateNode.fieldNames();
+      while (fieldNames.hasNext()) {
+        String fieldName = fieldNames.next();
+        if (mainNode.has(fieldName)) {
+          mergeNodes(mainNode, updateNode, fieldName);
+        } else {
+          mergeField(mainNode, updateNode, fieldName);
+        }
+      }
+      return mainNode;
+    }
+
+    private void mergeField(JsonNode mainNode, JsonNode updateNode, String fieldName) {
+      if (mainNode instanceof ObjectNode) {
+        JsonNode value = updateNode.get(fieldName);
+        if (value.isNull()) {
+          return;
+        }
+        if (value.isIntegralNumber() && value.toString().equals("0")) {
+          return;
+        }
+        if (value.isFloatingPointNumber() && value.toString().equals("0.0")) {
+          return;
+        }
+        ((ObjectNode) mainNode).set(fieldName, value);
+      }
+    }
+
+    private void mergeNodes(JsonNode mainNode, JsonNode updateNode, String fieldName) {
+      JsonNode jsonNode = updateNode.get(fieldName);
+      if (jsonNode.isObject()) {
+        ((ObjectNode) mainNode).set(fieldName, merge(mainNode.get(fieldName), updateNode.get(fieldName)));
+      } else if (jsonNode.isArray()) {
+        mergeArray((ObjectNode) mainNode, updateNode, fieldName, jsonNode);
+      } else {
+        ((ObjectNode) mainNode).set(fieldName, jsonNode);
+      }
+    }
+
+    private void mergeArray(ObjectNode mainNode, JsonNode updateNode, String fieldName, JsonNode jsonNode) {
+      ArrayNode arrayNode = objectMapper.createArrayNode();
+      for (int i = 0; i < jsonNode.size(); i++) {
+        arrayNode.add(merge(jsonNode.get(i), updateNode.get(fieldName).get(i)));
+      }
+      mainNode.set(fieldName, arrayNode);
+    }
+  }
+
+  public static class ToObjectStage<T> {
+    private final ObjectMapper objectMapper;
+    private final JsonNode node;
+    private final Class<T> clazz;
+    private boolean shouldValidate = false;
+
+    private ToObjectStage(ObjectMapper objectMapper, JsonNode node, Class<T> clazz) {
+      this.objectMapper = objectMapper;
+      this.node = node;
+      this.clazz = clazz;
+    }
+
+    /**
+     * Enables validation for dto object
+     *
+     * @return this builder instance
+     */
+    public ToObjectStage<T> withValidation() {
+      this.shouldValidate = true;
+      return this;
+    }
+
+    /**
+     * Returns the merged object as the provided class
+     *
+     * @return merged object
+     */
+    public T build() {
+      T updatedObject = objectMapper.convertValue(node, clazz);
+      if (shouldValidate) {
+        DtoValidator.validate(updatedObject);
+      }
+      return updatedObject;
+    }
+  }
+}

scm-core/src/test/java/sonia/scm/util/JsonMergerTest.java

@@ -0,0 +1,236 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2020-present Cloudogu GmbH and Contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package sonia.scm.util;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableList;
+import org.junit.jupiter.api.Test;
+
+import javax.validation.ConstraintViolationException;
+import javax.validation.constraints.NotNull;
+import java.util.List;
+
+import static java.util.Collections.emptyList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+class JsonMergerTest {
+
+  private final ObjectMapper objectMapper = new ObjectMapper();
+  private final JsonMerger jsonMerger = new JsonMerger(objectMapper);
+
+  @Test
+  void shouldMergeMultipleFieldsWithDto() throws JsonProcessingException {
+    JsonNode updateNode = objectMapper.readValue("{ \"name\":\"zorbot\", \"value\":\"42\"}", JsonNode.class);
+    TestDto testDto = new TestDto("trillian", "blue", 21, new TestUser(), emptyList());
+
+    TestDto updatedDto = jsonMerger.fromObject(testDto).mergeWithJson(updateNode).toObject(TestDto.class).withValidation().build();
+
+    assertThat(updatedDto.name).isEqualTo("zorbot");
+    assertThat(updatedDto.color).isEqualTo("blue");
+    assertThat(updatedDto.value).isEqualTo(42);
+  }
+
+  @Test
+  void shouldMergeNestedObjects() throws JsonProcessingException {
+    JsonNode updateNode = objectMapper.readValue("{ \"user\": { \"username\":\"zorbot\" } }", JsonNode.class);
+    TestDto testDto =
+      new TestDto("trillian", "blue", 21, new TestUser("trillian", "trillian@hitchhiker.org"), emptyList());
+
+    TestDto updatedDto = jsonMerger.fromObject(testDto).mergeWithJson(updateNode).toObject(TestDto.class).withValidation().build();
+
+    assertThat(updatedDto.user.username).isEqualTo("zorbot");
+    assertThat(updatedDto.user.mail).isEqualTo("trillian@hitchhiker.org");
+    assertThat(updatedDto.color).isEqualTo("blue");
+    assertThat(updatedDto.value).isEqualTo(21);
+  }
+
+  @Test
+  void shouldMergeArrays() throws JsonProcessingException {
+    JsonNode updateNode = objectMapper.readValue("{ \"fruits\": [\"banana\",\"apple\",\"orange\"] }", JsonNode.class);
+    TestDto testDto =
+      new TestDto("trillian", "blue", 21, new TestUser(), ImmutableList.of("strawberries"));
+
+    TestDto updatedDto = jsonMerger.fromObject(testDto).mergeWithJson(updateNode).toObject(TestDto.class).withValidation().build();
+
+    assertThat(updatedDto.fruits).contains("banana", "apple", "orange");
+    assertThat(updatedDto.color).isEqualTo("blue");
+    assertThat(updatedDto.value).isEqualTo(21);
+  }
+
+  @Test
+  void shouldRemoveFieldOnNull() throws JsonProcessingException {
+    JsonNode updateNode = objectMapper.readValue("{ \"fruits\": null, \"color\":null }", JsonNode.class);
+
+    TestDto testDto =
+      new TestDto("trillian", "blue", 21, new TestUser(), ImmutableList.of("strawberries"));
+    TestDto updatedDto = jsonMerger.fromObject(testDto).mergeWithJson(updateNode).toObject(TestDto.class).withValidation().build();
+
+    assertThat(updatedDto.fruits).isNull();
+    assertThat(updatedDto.color).isNull();
+    assertThat(updatedDto.value).isEqualTo(21);
+    assertThat(updatedDto.name).isEqualTo("trillian");
+  }
+
+  @Test
+  void shouldMergeJsonNodes() throws JsonProcessingException {
+    JsonNode main = objectMapper.readValue(
+      "{ \"fruits\": [\"banana\", \"pineapple\"], \"name\":\"main\" }",
+      JsonNode.class);
+    JsonNode update = objectMapper.readValue("{ \"fruits\": [\"apple\"], \"color\":\"blue\"}", JsonNode.class);
+
+    JsonNode jsonNode = jsonMerger.fromJson(main).mergeWithJson(update).toJsonNode();
+
+    assertThat(jsonNode.get("fruits").toPrettyString()).isEqualTo("[ \"apple\" ]");
+    assertThat(jsonNode.get("color").asText()).isEqualTo("blue");
+    assertThat(jsonNode.get("name").asText()).isEqualTo("main");
+  }
+
+  @Test
+  void shouldMergeMultipleTimes() throws JsonProcessingException {
+    TestDto mainDto = new TestDto("main", null, 0, new TestUser("trillian", ""), emptyList());
+    TestDto firstUpdate = new TestDto("main", "blue", 42, new TestUser("trillian", "trillian@hitchhiker.org"), emptyList());
+    JsonNode secondUpdate = objectMapper.readValue("{ \"fruits\": [\"banana\", \"pineapple\"], \"color\":\"red\" }", JsonNode.class);
+
+    TestDto resultDto = jsonMerger
+      .fromObject(mainDto)
+      .mergeWithObject(firstUpdate)
+      .mergeWithJson(secondUpdate)
+      .toObject(TestDto.class)
+      .withValidation()
+      .build();
+
+    assertThat(resultDto.name).isEqualTo("main");
+    assertThat(resultDto.color).isEqualTo("red");
+    assertThat(resultDto.value).isEqualTo(42);
+    assertThat(resultDto.user.username).isEqualTo("trillian");
+    assertThat(resultDto.user.mail).isEqualTo("trillian@hitchhiker.org");
+    assertThat(resultDto.fruits).contains("banana", "pineapple");
+  }
+
+  @Test
+  void shouldFailForInvalidDto() {
+    TestDto mainDto = new TestDto("main", null, 0, new TestUser("trillian", ""), emptyList());
+    TestDto firstUpdate = new TestDto(null, "blue", 42, new TestUser("trillian", "trillian@hitchhiker.org"), emptyList());
+
+    JsonMerger.ToObjectStage<TestDto> objectStage = jsonMerger.fromObject(mainDto)
+      .mergeWithObject(firstUpdate)
+      .toObject(TestDto.class)
+      .withValidation();
+
+    assertThrows(ConstraintViolationException.class, objectStage::build);
+  }
+
+  private static class TestDto {
+    @NotNull
+    String name;
+    String color;
+    int value;
+    TestUser user;
+    List<String> fruits;
+
+    public TestDto() {
+    }
+
+    public TestDto(String name, String color, int value, TestUser user, List<String> fruits) {
+      this.name = name;
+      this.color = color;
+      this.value = value;
+      this.user = user;
+      this.fruits = fruits;
+    }
+
+    public String getName() {
+      return name;
+    }
+
+    public void setName(String name) {
+      this.name = name;
+    }
+
+    public String getColor() {
+      return color;
+    }
+
+    public void setColor(String color) {
+      this.color = color;
+    }
+
+    public int getValue() {
+      return value;
+    }
+
+    public void setValue(int value) {
+      this.value = value;
+    }
+
+    public TestUser getUser() {
+      return user;
+    }
+
+    public void setUser(TestUser user) {
+      this.user = user;
+    }
+
+    public List<String> getFruits() {
+      return fruits;
+    }
+
+    public void setFruits(List<String> fruits) {
+      this.fruits = fruits;
+    }
+  }
+
+  private static class TestUser {
+    private String username;
+    private String mail;
+
+    public TestUser() {
+    }
+
+    public TestUser(String username, String mail) {
+      this.username = username;
+      this.mail = mail;
+    }
+
+    public String getUsername() {
+      return username;
+    }
+
+    public void setUsername(String username) {
+      this.username = username;
+    }
+
+    public String getMail() {
+      return mail;
+    }
+
+    public void setMail(String mail) {
+      this.mail = mail;
+    }
+  }
+}

scm-test/build.gradle

@@ -36,6 +36,7 @@ dependencies {
   api libraries.junitJupiterParams
   api libraries.junitJupiterEngine
   api libraries.shiroUnit
+  api libraries.shiroExtension
 
   // junit 4 support
   api libraries.junitVintageEngine

scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigResource.java

@@ -24,6 +24,7 @@
 
 package sonia.scm.api.v2.resources;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import com.google.common.annotations.VisibleForTesting;
 import io.swagger.v3.oas.annotations.OpenAPIDefinition;
 import io.swagger.v3.oas.annotations.Operation;
@@ -36,6 +37,7 @@ import io.swagger.v3.oas.annotations.tags.Tag;
 import sonia.scm.config.ConfigurationPermissions;
 import sonia.scm.config.ScmConfiguration;
 import sonia.scm.repository.NamespaceStrategyValidator;
+import sonia.scm.util.JsonMerger;
 import sonia.scm.util.ScmConfigurationUtil;
 import sonia.scm.web.VndMediaType;
 
@@ -43,6 +45,7 @@ import javax.inject.Inject;
 import javax.validation.Valid;
 import javax.ws.rs.Consumes;
 import javax.ws.rs.GET;
+import javax.ws.rs.PATCH;
 import javax.ws.rs.PUT;
 import javax.ws.rs.Path;
 import javax.ws.rs.Produces;
@@ -63,17 +66,21 @@ public class ConfigResource {
   private final ScmConfigurationToConfigDtoMapper configToDtoMapper;
   private final ScmConfiguration configuration;
   private final NamespaceStrategyValidator namespaceStrategyValidator;
+  private final JsonMerger jsonMerger;
 
-  private Consumer<ScmConfiguration> store = (config) -> ScmConfigurationUtil.getInstance().store(config);
+  private Consumer<ScmConfiguration> store = config -> ScmConfigurationUtil.getInstance().store(config);
 
   @Inject
   public ConfigResource(ConfigDtoToScmConfigurationMapper dtoToConfigMapper,
                         ScmConfigurationToConfigDtoMapper configToDtoMapper,
-                        ScmConfiguration configuration, NamespaceStrategyValidator namespaceStrategyValidator) {
+                        ScmConfiguration configuration,
+                        NamespaceStrategyValidator namespaceStrategyValidator,
+                        JsonMerger jsonMerger) {
     this.dtoToConfigMapper = dtoToConfigMapper;
     this.configToDtoMapper = configToDtoMapper;
     this.configuration = configuration;
     this.namespaceStrategyValidator = namespaceStrategyValidator;
+    this.jsonMerger = jsonMerger;
   }
 
   @VisibleForTesting
@@ -107,7 +114,6 @@ public class ConfigResource {
     )
   )
   public Response get() {
-
     // We do this permission check in Resource and not in ScmConfiguration, because it must be available for reading
     // from within the code (plugins, etc.), but not for the whole anonymous world outside.
     ConfigurationPermissions.read(configuration).check();
@@ -151,21 +157,73 @@ public class ConfigResource {
     )
   )
   public Response update(@Valid ConfigDto configDto) {
-
-    // This *could* be moved to ScmConfiguration or ScmConfigurationUtil classes.
-    // But to where to check? load() or store()? Leave it for now, SCMv1 legacy that can be cleaned up later.
     ConfigurationPermissions.write(configuration).check();
-
-    // ensure the namespace strategy is valid
-    namespaceStrategyValidator.check(configDto.getNamespaceStrategy());
-
-    ScmConfiguration config = dtoToConfigMapper.map(configDto);
-    synchronized (ScmConfiguration.class) {
-      configuration.load(config);
-      store.accept(configuration);
-    }
+    updateConfig(configDto);
 
     return Response.noContent().build();
   }
 
+  /**
+   * Modifies the global scm config partially.
+   *
+   * @param updateNode json object which contains changed fields
+   */
+  @PATCH
+  @Path("")
+  @Consumes(VndMediaType.CONFIG)
+  @Operation(
+    summary = "Update instance configuration partially",
+    description = "Modifies the instance configuration partially.",
+    tags = "Instance configuration",
+    requestBody = @RequestBody(
+      content = @Content(
+        mediaType = VndMediaType.CONFIG,
+        schema = @Schema(implementation = UpdateConfigDto.class),
+        examples = @ExampleObject(
+          name = "Overwrites the provided fields of the current configuration.",
+          value = "{\n  \"realmDescription\":\"SCM-Manager Realm\" \n}",
+          summary = "Update configuration partially"
+        )
+      )
+    )
+  )
+  @ApiResponse(responseCode = "204", description = "update success")
+  @ApiResponse(responseCode = "401", description = "not authenticated / invalid credentials")
+  @ApiResponse(responseCode = "403", description = "not authorized, the current user does not have the \"configuration:write\" privilege")
+  @ApiResponse(
+    responseCode = "500",
+    description = "internal server error",
+    content = @Content(
+      mediaType = VndMediaType.ERROR_TYPE,
+      schema = @Schema(implementation = ErrorDto.class)
+    )
+  )
+  public Response updatePartially(JsonNode updateNode) {
+    ConfigurationPermissions.write(configuration).check();
+
+    ConfigDto updatedConfigDto = jsonMerger
+      .fromObject(configToDtoMapper.map(configuration))
+      .mergeWithJson(updateNode)
+      .toObject(ConfigDto.class)
+      .withValidation()
+      .build();
+    updateConfig(updatedConfigDto);
+
+    return Response.noContent().build();
+  }
+
+  private void updateConfig(ConfigDto updatedConfigDto) {
+    // This *could* be moved to ScmConfiguration or ScmConfigurationUtil classes.
+    // But to where to check? load() or store()? Leave it for now, SCMv1 legacy that can be cleaned up later.
+
+    // ensure the namespace strategy is valid
+    namespaceStrategyValidator.check(updatedConfigDto.getNamespaceStrategy());
+
+    ScmConfiguration config = dtoToConfigMapper.map(updatedConfigDto);
+    synchronized (ScmConfiguration.class) {
+      configuration.load(config);
+      store.accept(configuration);
+    }
+  }
+
 }

scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java

@@ -27,7 +27,6 @@ package sonia.scm.web.i18n;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.github.legman.Subscribe;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.inject.Singleton;
@@ -40,6 +39,7 @@ import sonia.scm.cache.CacheManager;
 import sonia.scm.filter.WebElement;
 import sonia.scm.lifecycle.RestartEvent;
 import sonia.scm.plugin.PluginLoader;
+import sonia.scm.util.JsonMerger;
 
 import javax.inject.Inject;
 import javax.servlet.http.HttpServlet;
@@ -49,7 +49,6 @@ import java.io.IOException;
 import java.io.PrintWriter;
 import java.net.URL;
 import java.util.Enumeration;
-import java.util.Iterator;
 import java.util.Optional;
 
 
@@ -70,13 +69,15 @@ public class I18nServlet extends HttpServlet {
   private final ClassLoader classLoader;
   private final Cache<String, JsonNode> cache;
   private final ObjectMapper objectMapper = new ObjectMapper();
+  private final JsonMerger jsonMerger;
 
 
   @Inject
-  public I18nServlet(SCMContextProvider context, PluginLoader pluginLoader, CacheManager cacheManager) {
+  public I18nServlet(SCMContextProvider context, PluginLoader pluginLoader, CacheManager cacheManager, JsonMerger jsonMerger) {
     this.context = context;
     this.classLoader = pluginLoader.getUberClassLoader();
     this.cache = cacheManager.getCache(CACHE_NAME);
+    this.jsonMerger = jsonMerger;
   }
 
   @Subscribe(async = false)
@@ -146,7 +147,7 @@ public class I18nServlet extends HttpServlet {
       URL url = resources.nextElement();
       JsonNode jsonNode = objectMapper.readTree(url);
       if (mergedJsonNode != null) {
-        merge(mergedJsonNode, jsonNode);
+        mergedJsonNode = jsonMerger.fromJson(mergedJsonNode).mergeWithJson(jsonNode).toJsonNode();
       } else {
         mergedJsonNode = jsonNode;
       }
@@ -155,44 +156,4 @@ public class I18nServlet extends HttpServlet {
     return Optional.ofNullable(mergedJsonNode);
   }
 
-  private JsonNode merge(JsonNode mainNode, JsonNode updateNode) {
-    Iterator<String> fieldNames = updateNode.fieldNames();
-    while (fieldNames.hasNext()) {
-      String fieldName = fieldNames.next();
-      JsonNode jsonNode = mainNode.get(fieldName);
-      if (jsonNode != null) {
-        mergeNode(updateNode, fieldName, jsonNode);
-      } else {
-        mergeField(mainNode, updateNode, fieldName);
-      }
-    }
-    return mainNode;
-  }
-
-  private void mergeField(JsonNode mainNode, JsonNode updateNode, String fieldName) {
-    if (mainNode instanceof ObjectNode) {
-      JsonNode value = updateNode.get(fieldName);
-      if (value.isNull()) {
-        return;
-      }
-      if (value.isIntegralNumber() && value.toString().equals("0")) {
-        return;
-      }
-      if (value.isFloatingPointNumber() && value.toString().equals("0.0")) {
-        return;
-      }
-      ((ObjectNode) mainNode).set(fieldName, value);
-    }
-  }
-
-  private void mergeNode(JsonNode updateNode, String fieldName, JsonNode jsonNode) {
-    if (jsonNode.isObject()) {
-      merge(jsonNode, updateNode.get(fieldName));
-    } else if (jsonNode.isArray()) {
-      for (int i = 0; i < jsonNode.size(); i++) {
-        merge(jsonNode.get(i), updateNode.get(fieldName).get(i));
-      }
-    }
-  }
-
 }

scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigResourceTest.java

@@ -24,19 +24,21 @@
 
 package sonia.scm.api.v2.resources;
 
-import com.github.sdorra.shiro.ShiroRule;
-import com.github.sdorra.shiro.SubjectAware;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.io.Resources;
-import org.apache.shiro.util.ThreadContext;
+import org.github.sdorra.jse.ShiroExtension;
+import org.github.sdorra.jse.SubjectAware;
 import org.jboss.resteasy.mock.MockHttpRequest;
 import org.jboss.resteasy.mock.MockHttpResponse;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.config.ScmConfiguration;
 import sonia.scm.repository.NamespaceStrategyValidator;
+import sonia.scm.util.JsonMerger;
 import sonia.scm.web.RestDispatcher;
 import sonia.scm.web.VndMediaType;
 
@@ -48,110 +50,104 @@ import java.net.URISyntaxException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.verify;
-import static org.mockito.MockitoAnnotations.initMocks;
 
+@ExtendWith(ShiroExtension.class)
+@ExtendWith(MockitoExtension.class)
 @SubjectAware(
-  configuration = "classpath:sonia/scm/configuration/shiro.ini",
-  password = "secret"
+  value = "trillian"
 )
-public class ConfigResourceTest {
+class ConfigResourceTest {
 
-  @Rule
-  public ShiroRule shiro = new ShiroRule();
-
-  private RestDispatcher dispatcher = new RestDispatcher();
+  private final RestDispatcher dispatcher = new RestDispatcher();
 
   private final URI baseUri = URI.create("/");
   @SuppressWarnings("unused") // Is injected
-  private ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri);
+  private final ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri);
 
   @Mock
   private NamespaceStrategyValidator namespaceStrategyValidator;
 
+  private final JsonMerger jsonMerger = new JsonMerger(new ObjectMapper());
+
   @InjectMocks
   private ConfigDtoToScmConfigurationMapperImpl dtoToConfigMapper;
   @InjectMocks
   private ScmConfigurationToConfigDtoMapperImpl configToDtoMapper;
 
-  public ConfigResourceTest() {
-      // cleanup state that might have been left by other tests
-      ThreadContext.unbindSecurityManager();
-      ThreadContext.unbindSubject();
-      ThreadContext.remove();
-  }
-
-  @Before
-  public void prepareEnvironment() {
-    initMocks(this);
-
-    ConfigResource configResource = new ConfigResource(dtoToConfigMapper, configToDtoMapper, createConfiguration(), namespaceStrategyValidator);
-    configResource.setStore(config -> {});
+  @BeforeEach
+  void prepareEnvironment() {
+    ConfigResource configResource = new ConfigResource(dtoToConfigMapper, configToDtoMapper, createConfiguration(), namespaceStrategyValidator, jsonMerger);
+    configResource.setStore(config -> {
+    });
 
     dispatcher.addSingletonResource(configResource);
   }
 
   @Test
-  @SubjectAware(username = "readOnly")
-  public void shouldGetGlobalConfig() throws URISyntaxException, UnsupportedEncodingException {
+  @SubjectAware(
+    permissions = "configuration:read:global"
+  )
+  void shouldGetGlobalConfig() throws URISyntaxException, UnsupportedEncodingException {
     MockHttpRequest request = MockHttpRequest.get("/" + ConfigResource.CONFIG_PATH_V2);
     MockHttpResponse response = new MockHttpResponse();
     dispatcher.invoke(request, response);
-    assertEquals(HttpServletResponse.SC_OK, response.getStatus());
-    assertTrue(response.getContentAsString().contains("\"proxyPassword\":\"heartOfGold\""));
-    assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/config"));
-    assertFalse("Update link present", response.getContentAsString().contains("\"update\":{\"href\":\"/v2/config"));
+
+    assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
+    assertThat(response.getContentAsString()).contains("\"proxyPassword\":\"heartOfGold\"");
+    assertThat(response.getContentAsString()).contains("\"self\":{\"href\":\"/v2/config");
+    assertThat(response.getContentAsString()).doesNotContain("\"update\":{\"href\":\"/v2/config");
   }
 
   @Test
-  @SubjectAware(username = "writeOnly")
-  public void shouldNotGetConfigWhenNotAuthorized() throws URISyntaxException, UnsupportedEncodingException {
+  void shouldNotGetConfigWhenNotAuthorized() throws URISyntaxException {
     MockHttpRequest request = MockHttpRequest.get("/" + ConfigResource.CONFIG_PATH_V2);
     MockHttpResponse response = new MockHttpResponse();
 
     dispatcher.invoke(request, response);
 
-    assertEquals("Subject does not have permission [configuration:read:global]", response.getContentAsString());
-    assertEquals(HttpServletResponse.SC_FORBIDDEN, response.getStatus());
+    assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
   }
 
   @Test
-  @SubjectAware(username = "readWrite")
-  public void shouldUpdateConfig() throws URISyntaxException, IOException {
-    MockHttpRequest request = post("sonia/scm/api/v2/config-test-update.json");
+  @SubjectAware(
+    permissions = "configuration:read,write:global"
+  )
+  void shouldUpdateConfig() throws URISyntaxException, IOException {
+    MockHttpRequest request = put("sonia/scm/api/v2/config-test-update.json");
 
     MockHttpResponse response = new MockHttpResponse();
     dispatcher.invoke(request, response);
-    assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus());
+    assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_NO_CONTENT);
 
     request = MockHttpRequest.get("/" + ConfigResource.CONFIG_PATH_V2);
     response = new MockHttpResponse();
     dispatcher.invoke(request, response);
 
-    assertEquals(HttpServletResponse.SC_OK, response.getStatus());
-    assertTrue(response.getContentAsString().contains("\"proxyPassword\":\"newPassword\""));
-    assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/config"));
-    assertTrue("link not found", response.getContentAsString().contains("\"update\":{\"href\":\"/v2/config"));
+    assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
+    // Should overwrite old realm description with null
+    assertThat(response.getContentAsString()).contains("\"realmDescription\":null");
+    assertThat(response.getContentAsString()).contains("\"proxyPassword\":\"newPassword\"");
+    assertThat(response.getContentAsString()).contains("\"self\":{\"href\":\"/v2/config");
+    assertThat(response.getContentAsString()).contains("\"update\":{\"href\":\"/v2/config");
   }
 
   @Test
-  @SubjectAware(username = "readOnly")
-  public void shouldNotUpdateConfigWhenNotAuthorized() throws URISyntaxException, IOException {
-    MockHttpRequest request = post("sonia/scm/api/v2/config-test-update.json");
+  void shouldNotUpdateConfigWhenNotAuthorized() throws URISyntaxException, IOException {
+    MockHttpRequest request = put("sonia/scm/api/v2/config-test-update.json");
     MockHttpResponse response = new MockHttpResponse();
 
     dispatcher.invoke(request, response);
 
-    assertEquals("Subject does not have permission [configuration:write:global]", response.getContentAsString());
-    assertEquals(HttpServletResponse.SC_FORBIDDEN, response.getStatus());
+    assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
   }
 
   @Test
-  @SubjectAware(username = "readWrite")
-  public void shouldValidateNamespaceStrategy() throws URISyntaxException {
+  @SubjectAware(
+    permissions = "configuration:write:global"
+  )
+  void shouldValidateNamespaceStrategy() throws URISyntaxException {
     MockHttpRequest request = MockHttpRequest.put("/" + ConfigResource.CONFIG_PATH_V2)
       .contentType(VndMediaType.CONFIG)
       .content("{ \"namespaceStrategy\": \"AwesomeStrategy\" }".getBytes(StandardCharsets.UTF_8));
@@ -159,11 +155,44 @@ public class ConfigResourceTest {
     MockHttpResponse response = new MockHttpResponse();
     dispatcher.invoke(request, response);
 
-    assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus());
+    assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_NO_CONTENT);
     verify(namespaceStrategyValidator).check("AwesomeStrategy");
   }
 
-  private MockHttpRequest post(String resourceName) throws IOException, URISyntaxException {
+  @Test
+  @SubjectAware(
+    permissions = "configuration:read,write:global"
+  )
+  void shouldUpdateConfigPartially() throws URISyntaxException, IOException {
+    MockHttpRequest request = patch("{ \"proxyPort\":\"1337\", \"proxyPassword\":null }");
+
+    MockHttpResponse response = new MockHttpResponse();
+    dispatcher.invoke(request, response);
+    assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_NO_CONTENT);
+
+    request = MockHttpRequest.get("/" + ConfigResource.CONFIG_PATH_V2);
+    response = new MockHttpResponse();
+    dispatcher.invoke(request, response);
+
+    assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
+    // Should not change old realm description
+    assertThat(response.getContentAsString()).contains("\"realmDescription\":\"SONIA :: SCM Manager\"");
+    assertThat(response.getContentAsString()).contains("\"proxyPassword\":null");
+    assertThat(response.getContentAsString()).contains("\"proxyPort\":1337");
+    assertThat(response.getContentAsString()).contains("\"self\":{\"href\":\"/v2/config");
+    assertThat(response.getContentAsString()).contains("\"update\":{\"href\":\"/v2/config");
+  }
+
+  @Test
+  void shouldNotUpdateConfigPartiallyIfNotAuthorized() throws URISyntaxException {
+    MockHttpRequest request = patch("{ \"proxyPort\":\"1337\", \"proxyPassword\":\"hitchhiker\" }");
+
+    MockHttpResponse response = new MockHttpResponse();
+    dispatcher.invoke(request, response);
+    assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
+  }
+
+  private MockHttpRequest put(String resourceName) throws IOException, URISyntaxException {
     URL url = Resources.getResource(resourceName);
     byte[] configJson = Resources.toByteArray(url);
     return MockHttpRequest.put("/" + ConfigResource.CONFIG_PATH_V2)
@@ -171,6 +200,12 @@ public class ConfigResourceTest {
       .content(configJson);
   }
 
+  private MockHttpRequest patch(String json) throws URISyntaxException {
+    return MockHttpRequest.patch("/" + ConfigResource.CONFIG_PATH_V2)
+      .contentType(VndMediaType.CONFIG)
+      .content(json.getBytes());
+  }
+
   private static ScmConfiguration createConfiguration() {
     ScmConfiguration scmConfiguration = new ScmConfiguration();
     scmConfiguration.setProxyPassword("heartOfGold");

scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java

@@ -41,6 +41,7 @@ import sonia.scm.cache.Cache;
 import sonia.scm.cache.CacheManager;
 import sonia.scm.lifecycle.RestartEventFactory;
 import sonia.scm.plugin.PluginLoader;
+import sonia.scm.util.JsonMerger;
 
 import javax.annotation.Nonnull;
 import javax.servlet.http.HttpServletRequest;
@@ -62,7 +63,11 @@ import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.fail;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 @ExtendWith(MockitoExtension.class)
 class I18nServletTest {
@@ -223,7 +228,7 @@ class I18nServletTest {
 
     @Nonnull
     private I18nServlet createServlet() {
-      return new I18nServlet(context, pluginLoader, cacheManager);
+      return new I18nServlet(context, pluginLoader, cacheManager, new JsonMerger(new ObjectMapper()));
     }
 
     private String doGetString(I18nServlet servlet, HttpServletRequest request, HttpServletResponse response) throws IOException {