Merge pull request #280 from ligangty/overlap

fix: MMENG-4284 npm del error when deleting a package which has overlapped name with others

Author: ligangty

charon/pkgs/npm.py

@@ -447,6 +447,11 @@ def _gen_npm_package_metadata_for_del(
     if prefix and prefix != "/":
         prefix_meta_key = os.path.join(prefix, package_metadata_key)
         path_prefix = os.path.join(prefix, package_path_prefix)
+    # MMENG-4284 we should make sure each path_prefix ends with "/" to avoid
+    # overlapping, like "backstage-plugin-orchestrator" vs
+    # "backstage-plugin-orchestrator-backend-dynamic"
+    if not path_prefix.endswith("/"):
+        path_prefix = path_prefix + "/"
     (existed_version_metas, success) = client.get_files(
         bucket_name=bucket, prefix=path_prefix, suffix=PACKAGE_JSON
     )
@@ -572,15 +577,23 @@ def _do_merge(original: NPMPackageMetadata, source: NPMPackageMetadata, is_lates
             original.repository = source.repository
             changed = True
     if source.maintainers:
-        for m in source.maintainers:
-            if m not in original.maintainers:
-                original.maintainers.append(m)
-                changed = True
+        if not original.maintainers:
+            original.maintainers = source.maintainers
+            changed = True
+        else:
+            for m in source.maintainers:
+                if m not in original.maintainers:
+                    original.maintainers.append(m)
+                    changed = True
     if source.keywords:
-        for k in source.keywords:
-            if k not in original.keywords:
-                original.keywords.append(k)
-                changed = True
+        if not original.keywords:
+            original.keywords = source.keywords
+            changed = True
+        else:
+            for k in source.keywords:
+                if k not in original.keywords:
+                    original.keywords.append(k)
+                    changed = True
     if source.users:
         for u in source.users.keys():
             original.users[u] = source.users.get(u)

tests/input/backstage-plugin-orchestrator-1.21.102.tgz

@@ -110,3 +110,106 @@ def __prepare_content(self, prefix: str = None):
             targets=[('', TEST_BUCKET, prefix, DEFAULT_REGISTRY)],
             dir_=self.tempdir, do_index=False
         )
+
+    def test_overlap_prefix_del(self):
+        self.__prepare_content_backstage()
+
+        test_bucket = self.mock_s3.Bucket(TEST_BUCKET)
+        objs = list(test_bucket.objects.all())
+        actual_files = [obj.key for obj in objs]
+        self.assertEqual(18, len(actual_files))
+
+        meta_obj = test_bucket.Object("@janus-idp/backstage-plugin-orchestrator/package.json")
+        meta_content_client = str(meta_obj.get()["Body"].read(), "utf-8")
+        self.assertIn("\"name\": \"@janus-idp/backstage-plugin-orchestrator\"", meta_content_client)
+        self.assertIn("\"version\": \"1.21.102\"", meta_content_client)
+        self.assertIn("\"version\": \"1.21.103\"", meta_content_client)
+        self.assertIn("\"1.21.103\": {\"name\":", meta_content_client)
+
+        meta_obj = test_bucket.Object(
+            "@janus-idp/backstage-plugin-orchestrator-backend-dynamic/package.json")
+        meta_content_client = str(meta_obj.get()["Body"].read(), "utf-8")
+        self.assertIn(
+            "\"name\": \"@janus-idp/backstage-plugin-orchestrator-backend-dynamic\"",
+            meta_content_client)
+        self.assertIn("\"version\": \"0.0.1\"", meta_content_client)
+        self.assertIn("\"version\": \"2.3.0\"", meta_content_client)
+        self.assertIn("\"2.3.0\": {\"name\":", meta_content_client)
+
+        test_prod = "backstage-plugin-orchestrator-1.21.103"
+        test_tgz = os.path.join(INPUTS, test_prod+".tgz")
+        handle_npm_del(
+            test_tgz, test_prod,
+            targets=[('', TEST_BUCKET, None, '')],
+            dir_=self.tempdir, do_index=False
+        )
+
+        test_bucket = self.mock_s3.Bucket(TEST_BUCKET)
+        objs = list(test_bucket.objects.all())
+        actual_files = [obj.key for obj in objs]
+        self.assertEqual(14, len(actual_files))
+
+        meta_obj = test_bucket.Object("@janus-idp/backstage-plugin-orchestrator/package.json")
+        meta_content_client = str(meta_obj.get()["Body"].read(), "utf-8")
+        self.assertIn("\"name\": \"@janus-idp/backstage-plugin-orchestrator\"", meta_content_client)
+        self.assertIn("\"version\": \"1.21.102\"", meta_content_client)
+        self.assertNotIn("\"version\": \"1.21.103\"", meta_content_client)
+        self.assertNotIn("\"1.21.103\": {\"name\":", meta_content_client)
+
+        meta_obj = test_bucket.Object(
+            "@janus-idp/backstage-plugin-orchestrator-backend-dynamic/package.json")
+        meta_content_client = str(meta_obj.get()["Body"].read(), "utf-8")
+        self.assertIn(
+            "\"name\": \"@janus-idp/backstage-plugin-orchestrator-backend-dynamic\"",
+            meta_content_client)
+        self.assertIn("\"version\": \"0.0.1\"", meta_content_client)
+        self.assertIn("\"version\": \"2.3.0\"", meta_content_client)
+        self.assertIn("\"2.3.0\": {\"name\":", meta_content_client)
+
+        test_prod = "backstage-plugin-orchestrator-backend-dynamic-2.3.0"
+        test_tgz = os.path.join(INPUTS, test_prod+".tgz")
+        handle_npm_del(
+            test_tgz, test_prod,
+            targets=[('', TEST_BUCKET, None, '')],
+            dir_=self.tempdir, do_index=False
+        )
+
+        test_bucket = self.mock_s3.Bucket(TEST_BUCKET)
+        objs = list(test_bucket.objects.all())
+        actual_files = [obj.key for obj in objs]
+        self.assertEqual(10, len(actual_files))
+
+        meta_obj = test_bucket.Object(
+            "@janus-idp/backstage-plugin-orchestrator/package.json")
+        meta_content_client = str(meta_obj.get()["Body"].read(), "utf-8")
+        self.assertIn(
+            "\"name\": \"@janus-idp/backstage-plugin-orchestrator\"",
+            meta_content_client)
+        self.assertIn("\"version\": \"1.21.102\"", meta_content_client)
+        self.assertNotIn("\"version\": \"1.21.103\"", meta_content_client)
+        self.assertNotIn("\"1.21.103\": {\"name\":", meta_content_client)
+
+        meta_obj = test_bucket.Object(
+            "@janus-idp/backstage-plugin-orchestrator-backend-dynamic/package.json")
+        meta_content_client = str(meta_obj.get()["Body"].read(), "utf-8")
+        self.assertIn(
+            "\"name\": \"@janus-idp/backstage-plugin-orchestrator-backend-dynamic\"",
+            meta_content_client)
+        self.assertIn("\"version\": \"0.0.1\"", meta_content_client)
+        self.assertNotIn("\"version\": \"2.3.0\"", meta_content_client)
+        self.assertNotIn("\"2.3.0\": {\"name\":", meta_content_client)
+
+    def __prepare_content_backstage(self, prefix: str = None):
+        test_prods = [
+            "backstage-plugin-orchestrator-1.21.102",
+            "backstage-plugin-orchestrator-1.21.103",
+            "backstage-plugin-orchestrator-backend-dynamic-0.0.1",
+            "backstage-plugin-orchestrator-backend-dynamic-2.3.0"
+        ]
+        for p in test_prods:
+            test_tgz = os.path.join(INPUTS, p+".tgz")
+            handle_npm_uploading(
+                test_tgz, p,
+                targets=[('', TEST_BUCKET, prefix, DEFAULT_REGISTRY)],
+                dir_=self.tempdir, do_index=False
+            )