Repository: jhipster/jhipster-lite
Pull request #10862 (2024-09-15 21:56:11)

fix: NpmPackagesVersions::merge was mutating the object passed in parameter


Patch:
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
index bc323d076..41d12735e 100644
--- 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 record NpmPackage(NpmPackageName name, NpmPackageVersion 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
index cc0081f23..5b4f9415b 100644
--- 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 final class NpmPackagesVersions {
   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);


Test patch:
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
index 410a45786..03d38e3d9 100644
--- 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,7 +17,7 @@ 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");
@@ -21,11 +25,9 @@ class NpmPackagesVersionsTest {
 
   @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 @@ class NpmPackagesVersionsTest {
   @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"));
+    }
   }
 }