Feat: Use separate file to record product info (#120)

We found that AWS file metadata has length limitation, which means
not suitable to store long information like products info. So we need to
use a new file *.prodinfo for each uploaded files. Rules:
   * Only product tarball valid files should contain this prodinfo file
   * Metadta files like maven-metadata or npm package leveled
     package.json or index files will not have this prodinfo.

Author: ligangty

charon/constants.py

@@ -134,11 +134,9 @@
   <hr/>
   <main>
     <ul style="list-style: none outside;" id="contents">
-    {% for item in index.items %}{% if item.endswith("/") %}
-        <li><a href="{{ item }}index.html" title="{{ item }}">{{ item }}</a></li>
-    {% else %}
+    {% for item in index.items %}
         <li><a href="{{ item }}" title="{{ item }}">{{ item }}</a></li>
-    {% endif %}{% endfor%}
+    {% endfor%}
     </ul>
   </main>
   <hr/>
@@ -175,3 +173,5 @@
 </body>
 </html>
 '''
+
+PROD_INFO_SUFFIX = ".prodinfo"

charon/pkgs/npm.py

@@ -113,7 +113,7 @@ def handle_npm_uploading(
         _, _failed_metas = client.upload_metadatas(
             meta_file_paths=[meta_files[META_FILE_GEN_KEY]],
             bucket_name=bucket,
-            product=product,
+            product=None,
             root=target_dir,
             key_prefix=prefix_
         )
@@ -189,7 +189,7 @@ def handle_npm_del(
         all_meta_files.append(file)
     client.delete_files(
         file_paths=all_meta_files, bucket_name=bucket,
-        product=product, root=target_dir, key_prefix=prefix_
+        product=None, root=target_dir, key_prefix=prefix_
     )
     failed_metas = []
     if META_FILE_GEN_KEY in meta_files:

charon/storage.py

@@ -15,6 +15,7 @@
 """
 from boto3_type_annotations.s3.service_resource import Object
 from charon.utils.files import read_sha1
+from charon.constants import PROD_INFO_SUFFIX
 
 from boto3 import session
 from botocore.errorfactory import ClientError
@@ -134,8 +135,6 @@ def path_upload_handler(full_file_path: str, path: str, index: int, total: int)
                 f_meta = {}
                 if sha1.strip() != "":
                     f_meta[CHECKSUM_META_KEY] = sha1
-                if product:
-                    f_meta[PRODUCT_META_KEY] = product
                 try:
                     if not self.__dry_run:
                         if len(f_meta) > 0:
@@ -149,6 +148,8 @@ def path_upload_handler(full_file_path: str, path: str, index: int, total: int)
                                 full_file_path,
                                 ExtraArgs={'ContentType': content_type}
                             )
+                        if product:
+                            self.__update_prod_info(path_key, bucket_name, [product])
                     logger.info('Uploaded %s to bucket %s', path, bucket_name)
                     uploaded_files.append(path_key)
                 except (ClientError, HTTPClientError) as e:
@@ -170,25 +171,15 @@ def path_upload_handler(full_file_path: str, path: str, index: int, total: int)
                                  'one in S3. Product: %s', path_key, product)
                     return False
 
-                prods = []
-                try:
-                    prods = f_meta[PRODUCT_META_KEY].split(",")
-                except KeyError:
-                    pass
-                if not self.__dry_run and product not in prods:
+                (prods, no_error) = self.__get_prod_info(path_key, bucket_name)
+                if not self.__dry_run and no_error and product not in prods:
                     logger.info(
                         "File %s has new product, updating the product %s",
                         full_file_path,
                         product,
                     )
                     prods.append(product)
-                    try:
-                        self.__update_file_metadata(file_object, bucket_name,
-                                                    {PRODUCT_META_KEY: ",".join(prods)})
-                    except (ClientError, HTTPClientError) as e:
-                        logger.error("ERROR: file %s not uploaded to bucket"
-                                     " %s due to error: %s ", full_file_path,
-                                     bucket_name, e)
+                    if not self.__update_prod_info(path_key, bucket_name, prods):
                         return False
             return True
 
@@ -198,7 +189,7 @@ def path_upload_handler(full_file_path: str, path: str, index: int, total: int)
 
     def upload_metadatas(
         self, meta_file_paths: List[str], bucket_name: str,
-        product: Optional[str], root="/", key_prefix: str = None
+        product: Optional[str] = None, root="/", key_prefix: str = None
     ) -> Tuple[List[str], List[str]]:
         """ Upload a list of metadata files to s3 bucket. This function is very similar to
         upload_files, except:
@@ -237,14 +228,6 @@ def path_upload_handler(full_file_path: str, path: str, index: int, total: int):
                 )
 
             f_meta[CHECKSUM_META_KEY] = sha1
-            prods = (
-                f_meta[PRODUCT_META_KEY].split(",")
-                if PRODUCT_META_KEY in f_meta
-                else []
-            )
-            if product and product not in prods:
-                prods.append(product)
-                f_meta[PRODUCT_META_KEY] = ",".join(prods)
             try:
                 if not self.__dry_run:
                     if need_overwritten:
@@ -253,15 +236,16 @@ def path_upload_handler(full_file_path: str, path: str, index: int, total: int):
                             Metadata=f_meta,
                             ContentType=content_type
                         )
-
-                    else:
-                        # Should we update the s3 object metadata for metadata files?
-                        try:
-                            self.__update_file_metadata(file_object, bucket_name, f_meta)
-                        except (ClientError, HTTPClientError) as e:
-                            logger.error("ERROR: metadata %s not updated to bucket"
-                                         " %s due to error: %s ", full_file_path,
-                                         bucket_name, e)
+                    if product:
+                        # NOTE: This should not happen for most cases, as most of the metadata
+                        # file does not have product info. Just leave for requirement change in
+                        # future
+                        (prods, no_error) = self.__get_prod_info(path_key, bucket_name)
+                        if not no_error:
+                            return False
+                        if no_error and product not in prods:
+                            prods.append(product)
+                        if not self.__update_prod_info(path_key, bucket_name, prods):
                             return False
                 logger.info('Updated metadata %s to bucket %s', path, bucket_name)
                 uploaded_files.append(path_key)
@@ -306,11 +290,9 @@ def path_delete_handler(full_file_path: str, path: str, index: int, total: int):
                 # the product reference counts will be used (from object metadata).
                 prods = []
                 if product:
-                    try:
-                        prods = file_object.metadata[PRODUCT_META_KEY].split(",")
-                    except KeyError:
-                        pass
-
+                    (prods, no_error) = self.__get_prod_info(path_key, bucket_name)
+                    if not no_error:
+                        return False
                     if product in prods:
                         prods.remove(product)
 
@@ -321,10 +303,10 @@ def path_delete_handler(full_file_path: str, path: str, index: int, total: int):
                             " will remove %s from its metadata",
                             path, product
                         )
-                        self.__update_file_metadata(
-                            file_object,
+                        self.__update_prod_info(
+                            path_key,
                             bucket_name,
-                            {PRODUCT_META_KEY: ",".join(prods)},
+                            prods,
                         )
                         logger.info(
                             "Removed product %s from metadata of file %s",
@@ -341,6 +323,8 @@ def path_delete_handler(full_file_path: str, path: str, index: int, total: int):
                     try:
                         if not self.__dry_run:
                             bucket.delete_objects(Delete={"Objects": [{"Key": path_key}]})
+                            if not self.__update_prod_info(path_key, bucket_name, prods):
+                                return False
                         logger.info("Deleted %s from bucket %s", path, bucket_name)
                         deleted_files.append(path)
                         return True
@@ -439,23 +423,74 @@ def __file_exists(self, file_object: Object) -> bool:
         try:
             file_object.load()
             return True
-        except ClientError as e:
-            if e.response["Error"]["Code"] == "404":
+        except (ClientError, HTTPClientError) as e:
+            if isinstance(e, ClientError) and e.response["Error"]["Code"] == "404":
                 return False
             else:
-                raise e
-
-    def __update_file_metadata(
-        self, file_object: s3.Object, bucket_name: str, metadata: Dict
-    ):
-        if not self.__dry_run:
-            file_object.metadata.update(metadata)
-            file_object.copy_from(
-                CopySource={"Bucket": bucket_name, "Key": file_object.key},
-                Metadata=file_object.metadata,
-                ContentType=file_object.content_type,
-                MetadataDirective="REPLACE",
-            )
+                logger.error("Error: file existence check failed due "
+                             "to error: %s", e)
+
+    # def __update_file_metadata(
+    #     self, file_object: s3.Object, bucket_name: str, metadata: Dict
+    # ):
+    #     if not self.__dry_run:
+    #         file_object.metadata.update(metadata)
+    #         file_object.copy_from(
+    #             CopySource={"Bucket": bucket_name, "Key": file_object.key},
+    #             Metadata=file_object.metadata,
+    #             ContentType=file_object.content_type,
+    #             MetadataDirective="REPLACE",
+    #         )
+
+    def __get_prod_info(
+        self, file: str, bucket_name: str
+    ) -> Tuple[List[str], bool]:
+        logger.debug("Getting product infomation for file %s", file)
+        prod_info_file = file + PROD_INFO_SUFFIX
+        try:
+            info_file_content = self.read_file_content(bucket_name, prod_info_file)
+            prods = [p.strip() for p in info_file_content.split("\n")]
+            logger.debug("Got product information as below %s", prods)
+            return (prods, True)
+        except (ClientError, HTTPClientError) as e:
+            logger.error("ERROR: Can not get product info for file %s "
+                         "due to error: %s", file, e)
+            return ([], False)
+
+    def __update_prod_info(
+        self, file: str, bucket_name: str, prods: List[str]
+    ) -> bool:
+        prod_info_file = file + PROD_INFO_SUFFIX
+        bucket = self.__get_bucket(bucket_name)
+        file_obj = bucket.Object(prod_info_file)
+        content_type = "text/plain"
+        if len(prods) > 0:
+            logger.debug("Updating product infomation for file %s "
+                         "with products: %s", file, prods)
+            try:
+                file_obj.put(
+                    Body="\n".join(prods).encode("utf-8"),
+                    ContentType=content_type
+                )
+                logger.debug("Updated product infomation for file %s", file)
+                return True
+            except (ClientError, HTTPClientError) as e:
+                logger.error("ERROR: Can not update product info for file %s "
+                             "due to error: %s", file, e)
+                return False
+        else:
+            logger.debug("Removing product infomation file for file %s "
+                         "because no products left", file)
+            try:
+                if self.__file_exists(file_obj):
+                    bucket.delete_objects(
+                        Delete={"Objects": [{"Key": prod_info_file}]})
+                    logger.debug("Removed product infomation file for file %s", file)
+                return True
+            except (ClientError, HTTPClientError) as e:
+                logger.error("ERROR: Can not delete product info file for file %s "
+                             "due to error: %s", file, e)
+                return False
 
     def __do_path_cut_and(
         self, file_paths: List[str],

tests/base.py

@@ -17,8 +17,16 @@
 import tempfile
 import os
 import shutil
+import boto3
+from typing import List
 from charon.utils.files import overwrite_file
 from charon.config import CONFIG_FILE
+from charon.constants import PROD_INFO_SUFFIX
+from charon.pkgs.pkg_utils import is_metadata
+from charon.storage import PRODUCT_META_KEY, CHECKSUM_META_KEY
+from tests.commons import TEST_BUCKET
+from boto3_type_annotations import s3
+
 
 SHORT_TEST_PREFIX = "ga"
 LONG_TEST_PREFIX = "earlyaccess/all"
@@ -72,3 +80,49 @@ def get_temp_dir(self) -> str:
 
     def get_config_base(self) -> str:
         return os.path.join(self.get_temp_dir(), '.charon')
+
+
+class PackageBaseTest(BaseTest):
+    def setUp(self):
+        super().setUp()
+        # mock_s3 is used to generate expected content
+        self.mock_s3 = self.__prepare_s3()
+        self.mock_s3.create_bucket(Bucket=TEST_BUCKET)
+        self.test_bucket = self.mock_s3.Bucket(TEST_BUCKET)
+
+    def tearDown(self):
+        bucket = self.mock_s3.Bucket(TEST_BUCKET)
+        try:
+            bucket.objects.all().delete()
+            bucket.delete()
+        except ValueError:
+            pass
+        super().tearDown()
+
+    def __prepare_s3(self):
+        return boto3.resource('s3')
+
+    def check_product(self, file: str, prods: List[str]):
+        prod_file = file + PROD_INFO_SUFFIX
+        prod_f_obj = self.test_bucket.Object(prod_file)
+        content = str(prod_f_obj.get()['Body'].read(), 'utf-8')
+        self.assertEqual(
+            set(prods),
+            set([f for f in content.split("\n") if f.strip() != ""])
+        )
+
+    def check_content(self, objs: List[s3.ObjectSummary], products: List[str]):
+        for obj in objs:
+            file_obj = obj.Object()
+            if not file_obj.key.endswith(PROD_INFO_SUFFIX):
+                if not is_metadata(file_obj.key):
+                    self.check_product(file_obj.key, products)
+                else:
+                    self.assertNotIn(PRODUCT_META_KEY, file_obj.metadata)
+                    if file_obj.key.endswith("maven-metadata.xml"):
+                        sha1_checksum = file_obj.metadata[CHECKSUM_META_KEY].strip()
+                        sha1_obj = self.test_bucket.Object(file_obj.key + ".sha1")
+                        sha1_file_content = str(sha1_obj.get()['Body'].read(), 'utf-8')
+                        self.assertEqual(sha1_checksum, sha1_file_content)
+                self.assertIn(CHECKSUM_META_KEY, file_obj.metadata)
+                self.assertNotEqual("", file_obj.metadata[CHECKSUM_META_KEY].strip())

tests/commons.py

@@ -1,5 +1,5 @@
 # For maven
-TEST_MVN_BUCKET = "test_bucket"
+TEST_BUCKET = "test_bucket"
 COMMONS_CLIENT_456_FILES = [
     "org/apache/httpcomponents/httpclient/4.5.6/httpclient-4.5.6.pom.sha1",
     "org/apache/httpcomponents/httpclient/4.5.6/httpclient-4.5.6.jar",
@@ -49,6 +49,20 @@
     "commons-client-4.5.9/licenses/licenses.txt",
     "commons-client-4.5.9/README.md"
 ]
+COMMONS_CLIENT_456_MVN_NUM = (
+    len(COMMONS_CLIENT_456_FILES) +
+    len(COMMONS_LOGGING_FILES))
+COMMONS_CLIENT_459_MVN_NUM = (
+    len(COMMONS_CLIENT_459_FILES) +
+    len(COMMONS_LOGGING_FILES))
+COMMONS_CLIENT_MVN_NUM = (
+    len(COMMONS_CLIENT_456_FILES) +
+    len(COMMONS_CLIENT_459_FILES) +
+    len(COMMONS_LOGGING_FILES))
+COMMONS_CLIENT_META_NUM = (
+    len(COMMONS_CLIENT_METAS) +
+    len(COMMONS_LOGGING_METAS) +
+    len(ARCHETYPE_CATALOG_FILES))
 # For maven indexes
 COMMONS_CLIENT_456_INDEXES = [
     "index.html",