Merge pull request #10862 from murdos/extensible-npm-version-source

fix: NpmPackagesVersions::merge was mutating the object passed in parameter
jhipster · Sep 16, 2024 · a0c1479 · a0c1479
2 parents 0134552 + 8882949
commit a0c1479
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 16 deletions.
diff --git a/src/main/java/tech/jhipster/lite/module/domain/npm/NpmPackage.java b/src/main/java/tech/jhipster/lite/module/domain/npm/NpmPackage.java
@@ -7,6 +7,10 @@ public NpmPackage(String name, String version) {
     this(new NpmPackageName(name), new NpmPackageVersion(version));
   }
 
+  public NpmPackage(NpmPackageName name, String version) {
+    this(name, new NpmPackageVersion(version));
+  }
+
   public NpmPackage {
     Assert.notNull("name", name);
     Assert.notNull("version", version);

diff --git a/src/main/java/tech/jhipster/lite/module/domain/npm/NpmPackagesVersions.java b/src/main/java/tech/jhipster/lite/module/domain/npm/NpmPackagesVersions.java
@@ -1,10 +1,6 @@
 package tech.jhipster.lite.module.domain.npm;
 
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Optional;
+import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Collectors;
 import tech.jhipster.lite.shared.error.domain.Assert;
@@ -39,7 +35,8 @@ public NpmPackageVersion get(NpmPackageName packageName, NpmVersionSource source
   public NpmPackagesVersions merge(NpmPackagesVersions other) {
     Assert.notNull("other", other);
 
-    Map<NpmVersionSource, NpmSourceVersions> mergedVersions = new HashMap<>(other.versions);
+    Map<NpmVersionSource, NpmSourceVersions> mergedVersions = new HashMap<>();
+    other.versions.forEach((source, packages) -> mergedVersions.computeIfAbsent(source, NpmSourceVersions::new).add(packages));
     versions.forEach((source, packages) -> mergedVersions.computeIfAbsent(source, NpmSourceVersions::new).add(packages));
 
     return new NpmPackagesVersions(mergedVersions);

diff --git a/src/test/java/tech/jhipster/lite/module/domain/npm/NpmPackagesVersionsTest.java b/src/test/java/tech/jhipster/lite/module/domain/npm/NpmPackagesVersionsTest.java
@@ -1,8 +1,12 @@
 package tech.jhipster.lite.module.domain.npm;
 
-import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static tech.jhipster.lite.module.domain.npm.NpmVersionSource.COMMON;
+import static tech.jhipster.lite.module.domain.npm.NpmVersionSource.VUE;
 
 import java.util.List;
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import tech.jhipster.lite.UnitTest;
 
@@ -13,19 +17,17 @@ class NpmPackagesVersionsTest {
   void shouldNotVersionFromUnknownSource() {
     NpmPackagesVersions versions = NpmPackagesVersions.builder().build();
 
-    assertThatThrownBy(() -> versions.get(new NpmPackageName("unknown"), NpmVersionSource.COMMON))
+    assertThatThrownBy(() -> versions.get(new NpmPackageName("unknown"), COMMON))
       .isExactlyInstanceOf(UnknownNpmPackageException.class)
       .hasMessageContaining("unknown")
       .hasMessageContaining("COMMON");
   }
 
   @Test
   void shouldNotGetUnknownPackageVersion() {
-    NpmPackagesVersions versions = NpmPackagesVersions.builder()
-      .put(NpmVersionSource.COMMON, List.of(new NpmPackage("vue", "1.2.3")))
-      .build();
+    NpmPackagesVersions versions = NpmPackagesVersions.builder().put(COMMON, List.of(new NpmPackage("vue", "1.2.3"))).build();
 
-    assertThatThrownBy(() -> versions.get(new NpmPackageName("unknown"), NpmVersionSource.COMMON))
+    assertThatThrownBy(() -> versions.get(new NpmPackageName("unknown"), COMMON))
       .isExactlyInstanceOf(UnknownNpmPackageException.class)
       .hasMessageContaining("unknown")
       .hasMessageContaining("COMMON");
@@ -34,11 +36,60 @@ void shouldNotGetUnknownPackageVersion() {
   @Test
   void shouldGetVersion() {
     NpmPackagesVersions versions = NpmPackagesVersions.builder()
-      .put(NpmVersionSource.COMMON, List.of(new NpmPackage("vue", "1.2.3")))
-      .put(NpmVersionSource.VUE, List.of(new NpmPackage("vue", "1.2.4")))
+      .put(COMMON, List.of(new NpmPackage("vue", "1.2.3")))
+      .put(VUE, List.of(new NpmPackage("vue", "1.2.4")))
       .build();
 
-    assertThat(versions.get(new NpmPackageName("vue"), NpmVersionSource.VUE)).isEqualTo(new NpmPackageVersion("1.2.4"));
-    assertThat(versions.get(new NpmPackageName("vue"), NpmVersionSource.COMMON)).isEqualTo(new NpmPackageVersion("1.2.3"));
+    assertThat(versions.get(new NpmPackageName("vue"), VUE)).isEqualTo(new NpmPackageVersion("1.2.4"));
+    assertThat(versions.get(new NpmPackageName("vue"), COMMON)).isEqualTo(new NpmPackageVersion("1.2.3"));
+  }
+
+  @Nested
+  class Merge {
+
+    public static final NpmPackageName TYPESCRIPT = new NpmPackageName("typescript");
+    public static final NpmPackageName PRETTIER = new NpmPackageName("prettier");
+
+    @Test
+    void shouldMergePackagesInSameSource() {
+      NpmPackagesVersions first = NpmPackagesVersions.builder().put(COMMON, List.of(new NpmPackage(TYPESCRIPT, "1.0.0"))).build();
+      NpmPackagesVersions second = NpmPackagesVersions.builder().put(COMMON, List.of(new NpmPackage(PRETTIER, "2.0.0"))).build();
+
+      assertThat(first.merge(second).get(TYPESCRIPT, COMMON)).isEqualTo(new NpmPackageVersion("1.0.0"));
+      assertThat(first.merge(second).get(PRETTIER, COMMON)).isEqualTo(new NpmPackageVersion("2.0.0"));
+
+      assertThat(second.merge(first).get(TYPESCRIPT, COMMON)).isEqualTo(new NpmPackageVersion("1.0.0"));
+      assertThat(second.merge(first).get(PRETTIER, COMMON)).isEqualTo(new NpmPackageVersion("2.0.0"));
+    }
+
+    @Test
+    void shouldMergeDifferentVersionsInSameSource() {
+      NpmPackagesVersions first = NpmPackagesVersions.builder().put(COMMON, List.of(new NpmPackage(TYPESCRIPT, "1.2.3"))).build();
+      NpmPackagesVersions second = NpmPackagesVersions.builder().put(COMMON, List.of(new NpmPackage(TYPESCRIPT, "1.2.6"))).build();
+
+      assertThat(first.merge(second).get(TYPESCRIPT, COMMON)).isEqualTo(new NpmPackageVersion("1.2.3"));
+      assertThat(second.merge(first).get(TYPESCRIPT, COMMON)).isEqualTo(new NpmPackageVersion("1.2.6"));
+    }
+
+    @Test
+    void shouldNotMuteOperands() {
+      NpmPackagesVersions versions = NpmPackagesVersions.builder().put(COMMON, List.of(new NpmPackage(TYPESCRIPT, "1.2.3"))).build();
+      NpmPackagesVersions other = NpmPackagesVersions.builder().put(COMMON, List.of(new NpmPackage(TYPESCRIPT, "1.2.6"))).build();
+
+      versions.merge(other);
+      assertThat(versions.get(TYPESCRIPT, COMMON)).isEqualTo(new NpmPackageVersion("1.2.3"));
+      assertThat(other.get(TYPESCRIPT, COMMON)).isEqualTo(new NpmPackageVersion("1.2.6"));
+    }
+
+    @Test
+    void shouldNotMergeWhenSourceIsDistinct() {
+      NpmPackagesVersions versions = NpmPackagesVersions.builder().put(COMMON, List.of(new NpmPackage(TYPESCRIPT, "1.2.3"))).build();
+      NpmPackagesVersions other = NpmPackagesVersions.builder().put(VUE, List.of(new NpmPackage(TYPESCRIPT, "1.2.6"))).build();
+
+      NpmPackagesVersions merged = versions.merge(other);
+
+      assertThat(merged.get(TYPESCRIPT, COMMON)).isEqualTo(new NpmPackageVersion("1.2.3"));
+      assertThat(merged.get(TYPESCRIPT, VUE)).isEqualTo(new NpmPackageVersion("1.2.6"));
+    }
   }
 }