Fixes NullPointerException

There may be v1 properties with key/value pairs, where the value is not set. Such "pairs" led to a null pointer exception in the `map` function, because no Optional could be created for the null value. To solve this, we filter such properties without value in the beginning, because these properties would create a null result both ways.
scm-manager · Jul 8, 2020 · f8da4ce · f8da4ce
1 parent 3a3eaae
commit f8da4ce
Show file tree

Hide file tree

Showing 3 changed files with 169 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 - Fixed file extension detection with new spotter version
 - Fixed wrong cache directory location ([#1236](https://github.com/scm-manager/scm-manager/issues/1236) and [#1242](https://github.com/scm-manager/scm-manager/issues/1242))
+- Fixed error in update step ([#1237](https://github.com/scm-manager/scm-manager/issues/1237) and [#1244](https://github.com/scm-manager/scm-manager/issues/1244))
 
 ## [2.2.0] - 2020-07-03
 ### Added

diff --git a/scm-core/src/main/java/sonia/scm/update/V1Properties.java b/scm-core/src/main/java/sonia/scm/update/V1Properties.java
@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.update;
 
 import javax.xml.bind.annotation.XmlAccessType;
@@ -58,7 +58,11 @@ public String get(String key) {
   }
 
   public Optional<String> getOptional(String key) {
-    return streamProps().filter(p -> key.equals(p.getKey())).map(V1Property::getValue).findFirst();
+    return streamProps()
+      .filter(prop -> prop.getValue() != null)
+      .filter(p -> key.equals(p.getKey()))
+      .map(V1Property::getValue)
+      .findFirst();
   }
 
   public Optional<Boolean> getBoolean(String key) {

diff --git a/scm-dao-xml/src/test/java/sonia/scm/update/xml/XmlV1PropertyDAOTest.java b/scm-dao-xml/src/test/java/sonia/scm/update/xml/XmlV1PropertyDAOTest.java
@@ -0,0 +1,162 @@
+/*
+ * 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.update.xml;
+
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import sonia.scm.SCMContextProvider;
+import sonia.scm.Stage;
+import sonia.scm.security.KeyGenerator;
+import sonia.scm.store.JAXBConfigurationEntryStoreFactory;
+import sonia.scm.update.RepositoryV1PropertyReader;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+
+class XmlV1PropertyDAOTest {
+
+  public static final String PROPERTIES = "<?xml version=\"1.0\" ?>\n" +
+    "<configuration>\n" +
+    "  <entry>\n" +
+    "    <key>9ZQKlvI401</key>\n" +
+    "    <value>\n" +
+    "      <item></item>\n" +
+    "      <item>\n" +
+    "        <key>redmine.url</key>\n" +
+    "        <value>https://redmine.example.net/projects/r6</value>\n" +
+    "      </item>\n" +
+    "      <item>\n" +
+    "        <key>redmine.auto-close-username-transformer</key>\n" +
+    "        <value>{0}</value>\n" +
+    "      </item>\n" +
+    "      <item>\n" +
+    "        <key>redmine.auto-close</key>\n" +
+    "      </item>\n" +
+    "    </value>\n" +
+    "  </entry>\n" +
+    "  <entry>\n" +
+    "    <key>1wPsrHPM81</key>\n" +
+    "    <value>\n" +
+    "      <item>\n" +
+    "        <key>notify.contact.repository</key>\n" +
+    "        <value>true</value>\n" +
+    "      </item>\n" +
+    "      <item>\n" +
+    "        <key>redmine.auto-close-username-transformer</key>\n" +
+    "        <value>fixed, fix, closed, close, resolved, resolve</value>\n" +
+    "      </item>\n" +
+    "      <item>\n" +
+    "        <key>redmine.url</key>\n" +
+    "        <value>https://redmine.example.net/projects/a2</value>\n" +
+    "      </item>\n" +
+    "      <item>\n" +
+    "        <key>redmine.update-issues</key>\n" +
+    "        <value>true</value>\n" +
+    "      </item>\n" +
+    "      <item>\n" +
+    "        <key>notify.email.per.push</key>\n" +
+    "        <value>true</value>\n" +
+    "      </item>\n" +
+    "      <item>\n" +
+    "        <key>notify.use.author.as.from.address</key>\n" +
+    "        <value>true</value>\n" +
+    "      </item>\n" +
+    "      <item>\n" +
+    "        <key>redmine.auto-close</key>\n" +
+    "        <value>true</value>\n" +
+    "      </item>\n" +
+    "    </value>\n" +
+    "  </entry>\n" +
+    "  <entry>\n" +
+    "    <key>WlDszQtZj4</key>\n" +
+    "    <value></value>\n" +
+    "  </entry>\n" +
+    "</configuration>";
+
+  /**
+   * This is a test for https://github.com/scm-manager/scm-manager/issues/1237
+   */
+  @Test
+  void shouldReadItemsWithEmptyValues(@TempDir Path temp) throws IOException {
+    Path configPath = temp.resolve("config");
+    Files.createDirectories(configPath);
+    Path propFile = configPath.resolve("repository-properties-v1.xml");
+    Files.write(propFile, PROPERTIES.getBytes());
+    XmlV1PropertyDAO dao = new XmlV1PropertyDAO(new JAXBConfigurationEntryStoreFactory(new SimpleContextProvider(temp), null, new SimpleKeyGenerator()));
+
+    dao.getProperties(new RepositoryV1PropertyReader())
+      .forEachEntry((key, prop) -> {
+        if (key.equals("9ZQKlvI401")) {
+          Assertions.assertThat(prop.getBoolean("redmine.auto-close")).isNotPresent();
+        } else if (key.equals("1wPsrHPM81")) {
+          Assertions.assertThat(prop.getBoolean("redmine.auto-close")).isPresent().get().isEqualTo(true);
+        }
+      });
+  }
+
+  private static class SimpleContextProvider implements SCMContextProvider {
+    private final Path temp;
+
+    public SimpleContextProvider(Path temp) {
+      this.temp = temp;
+    }
+
+    @Override
+    public File getBaseDirectory() {
+      return temp.toFile();
+    }
+
+    @Override
+    public Path resolve(Path path) {
+      return null;
+    }
+
+    @Override
+    public Stage getStage() {
+      return null;
+    }
+
+    @Override
+    public Throwable getStartupError() {
+      return null;
+    }
+
+    @Override
+    public String getVersion() {
+      return null;
+    }
+  }
+
+  private static class SimpleKeyGenerator implements KeyGenerator {
+    @Override
+    public String createKey() {
+      return "" + System.nanoTime();
+    }
+  }
+}