Add general improvement to e3.anod.store.*

This commit improves our current store implementation in several ways:
- It improves typing to accept any `os.PathLike[str]` object when possible.
- Change the time format stored on the database (the space between the
  date and the hour has been replaced by a `T`).
- Replace the `_id` field of `e3.anod.store.file.ResourceDict` by `id`.
- The metadata field is set to an empty dict by default instead of `None`.
- Transform the local function
  `e3.anod.store.StoreWriteOnly.submit_component.insert_to_component_file`
  to a default class instance method for future usage.
- Fix some typos and string errors. Also, improve logging.
- Add the `_add_component_attachment` method. This method does not commit
  the database changes at the end of the function. This is done for
  optimization purposes.

Related to it/org/software-supply-chain/production-pipeline/issues#1

Author: leocardao

src/e3/anod/store/__init__.py

@@ -201,23 +201,24 @@ class TableName(StrEnum):
         component_releases = "component_releases"
         components = "components"
 
-    def __init__(self, db: os.PathLike | None = None) -> None:
+    def __init__(self, db: os.PathLike[str] | str | None = None) -> None:
         """Initialize the Store class.
 
         This will create the database and its tables (if needed).
 
         :param db: A path to the database. If None, a new database is created with the
             name `.store.db`.
         """
-        self.connection = sqlite3.connect(db or ".store.db")
+        self.db_path = os.fspath(db or ".store.db")
+        self.connection = sqlite3.connect(self.db_path)
         self.cursor = self.connection.cursor()
         self.cursor.execute(
             f"CREATE TABLE IF NOT EXISTS {_Store.TableName.buildinfos}("
             "    id TEXT NOT NULL PRIMARY KEY,"
             "    build_date TEXT NOT NULL,"
             "    setup TEXT NOT NULL,"
             "    creation_date TEXT NOT NULL DEFAULT("
-            "        STRFTIME('%Y-%m-%d %H:%M:%f+00:00', 'now')"
+            "        STRFTIME('%Y-%m-%dT%H:%M:%f+00:00', 'now')"
             "    ),"
             "    build_version TEXT NOT NULL,"
             "    isready INTEGER NOT NULL DEFAULT 0 CHECK(isready in (0, 1))"
@@ -230,7 +231,7 @@ def __init__(self, db: os.PathLike | None = None) -> None:
             "    path TEXT NOT NULL,"
             "    size INTEGER NOT NULL,"
             "    creation_date TEXT NOT NULL DEFAULT("
-            "        STRFTIME('%Y-%m-%d %H:%M:%f+00:00', 'now')"
+            "        STRFTIME('%Y-%m-%dT%H:%M:%f+00:00', 'now')"
             "    )"
             ")"
         )
@@ -246,7 +247,7 @@ def __init__(self, db: os.PathLike | None = None) -> None:
             "   revision TEXT NOT NULL,"
             "   metadata TEXT NOT NULL,"
             "   creation_date TEXT NOT NULL DEFAULT("
-            "       STRFTIME('%Y-%m-%d %H:%M:%f+00:00', 'now')"
+            "       STRFTIME('%Y-%m-%dT%H:%M:%f+00:00', 'now')"
             "   )"
             ")"
         )
@@ -259,7 +260,7 @@ def __init__(self, db: os.PathLike | None = None) -> None:
             "   internal INTEGER NOT NULL DEFAULT 1 CHECK(internal IN (0, 1)),"
             "   attachment_name TEXT,"
             "   CHECK("
-            "       (attachment_name IS NOT NULL AND kind='attachment')"
+            "       (attachment_name IS NOT NULL AND kind='attachment') "
             "       OR kind IN ('file', 'source')"
             "   )"
             ")"
@@ -280,7 +281,7 @@ def __init__(self, db: os.PathLike | None = None) -> None:
             "   specname TEXT,"  # Can be Null
             "   build_id TEXT NOT NULL,"
             "   creation_date TEXT NOT NULL DEFAULT("
-            "       STRFTIME('%Y-%m-%d %H:%M:%f+00:00', 'now')"
+            "       STRFTIME('%Y-%m-%dT%H:%M:%f+00:00', 'now')"
             "   ),"
             "   is_valid INTEGER NOT NULL DEFAULT 1 CHECK(is_valid in (0, 1)),"
             "   is_published INTEGER NOT NULL DEFAULT 0 CHECK(is_published in (0, 1)),"
@@ -341,6 +342,7 @@ def _select(
         if where_clause:
             sql = f"{sql}WHERE {where_clause} "
         sql = f"{sql}ORDER BY {order_by}"
+
         return self.cursor.execute(sql, dynamic_where_values).fetchall()
 
     def _select_one(
@@ -397,7 +399,7 @@ def _tuple_to_resource(cls, req_tuple: _Store.ResourceTuple) -> ResourceDict:
         """
         _, resource_id, path, size, creation_date = req_tuple
         return {
-            "_id": resource_id,
+            "id": resource_id,
             "path": path,
             "size": size,
             "creation_date": creation_date,
@@ -447,7 +449,7 @@ def _tuple_to_file(
                 self._select_one(_Store.TableName.buildinfos, build_id)  # type: ignore[arg-type]
             )
 
-        if not resource or resource["_id"] != resource_id:
+        if not resource or resource["id"] != resource_id:
             resource = self._tuple_to_resource(
                 self._select_one(
                     _Store.TableName.resources, resource_id, field_name="resource_id"  # type: ignore[arg-type]
@@ -461,7 +463,7 @@ def _tuple_to_file(
             "alias": alias,
             "filename": filename,
             "revision": revision,
-            "metadata": json.loads(metadata) if metadata else None,
+            "metadata": json.loads(metadata) if metadata else {},
             "build_id": str(build_id),
             "resource_id": resource_id,
             "build": buildinfo,
@@ -729,7 +731,8 @@ def _tuple_to_comp(
                 else self._list_component_files(
                     "attachment", comp_id, component_buildinfo=buildinfo
                 )
-            ),
+            )
+            or None,
             "build": buildinfo,
         }
 
@@ -822,29 +825,29 @@ def create_thirdparty(self, file_info: FileDict) -> FileDict:
         file_info["revision"] = ""
         return self.submit_file(file_info)
 
-    def submit_component(self, component_info: ComponentDict) -> ComponentDict:
-        """See e3.anod.store.interface.StoreWriteInterface."""
+    def _insert_to_component_files(
+        self,
+        kind: Literal["file"] | Literal["source"] | Literal["attachment"],
+        file_list: Sequence[tuple[str | None, FileDict]],
+        component_id: str | int,
+    ) -> None:
+        """Insert a list of files to the component_files database table.
 
-        def insert_to_component_files(
-            kind: Literal["file"] | Literal["source"] | Literal["attachment"],
-            file_list: Sequence[tuple[str | None, FileDict]],
-            component_id: str | int,
-        ) -> None:
-            """Insert a list of files to the component_files database table.
-
-            :param kind: The kind of file to insert.
-            :param file_list: A sequence of tuples. The first element of this tuple must
-                be None if the kind != attachment. Otherwise, this represents the
-                attachment name. The second element should always be a FileDict.
-            :param component_id: The component id linked to these files.
-            """
-            for att_name, f in file_list:
-                self._insert(
-                    _Store.TableName.component_files,
-                    ["kind", "file_id", "component_id", "attachment_name"],  # type: ignore[arg-type]
-                    [kind, f["_id"], component_id, att_name],
-                )
+        :param kind: The kind of file to insert.
+        :param file_list: A sequence of tuples. The first element of this tuple must
+            be None if the kind != attachment. Otherwise, this represents the
+            attachment name. The second element should always be a FileDict.
+        :param component_id: The component id linked to these files.
+        """
+        for att_name, f in file_list:
+            self._insert(
+                _Store.TableName.component_files,
+                ["kind", "file_id", "component_id", "attachment_name"],  # type: ignore[arg-type]
+                [kind, f["_id"], component_id, att_name],
+            )
 
+    def submit_component(self, component_info: ComponentDict) -> ComponentDict:
+        """See e3.anod.store.interface.StoreWriteInterface."""
         # Upload only readmes, binaries and attachments, as sources are supposed to be
         # already there.
 
@@ -859,7 +862,7 @@ def insert_to_component_files(
         # Retrieve binaries an upload them.
         files = [self._submit_file(file_info) for file_info in component_info["files"]]
 
-        # Retrieve attachment an upload them.
+        # Retrieve attachments and upload them.
         attachments_with_name: dict[str, FileDict] = {}
         attachments = component_info.get("attachments")
         if attachments:
@@ -914,10 +917,12 @@ def insert_to_component_files(
         )
         # Create relation between files/sources/attachment and the new component.
         comp_id = req_tuple[0]
-        insert_to_component_files("file", [(None, f) for f in files], comp_id)
+        self._insert_to_component_files("file", [(None, f) for f in files], comp_id)
         sources = component_info["sources"]
-        insert_to_component_files("source", [(None, src) for src in sources], comp_id)
-        insert_to_component_files(
+        self._insert_to_component_files(
+            "source", [(None, src) for src in sources], comp_id
+        )
+        self._insert_to_component_files(
             "attachment", list(attachments_with_name.items()), comp_id
         )
         # Add the list of releases linked to this component.
@@ -1002,12 +1007,12 @@ def update_file_metadata(self, file_info: FileDict) -> FileDict:
             _Store.TableName.files,
             fid,
             ["metadata"],  # type: ignore[arg-type]
-            [json.dumps(file_info["metadata"]) if file_info["metadata"] else ""],
+            [json.dumps(file_info["metadata"]) if file_info["metadata"] else "{}"],
         )
         self.connection.commit()
         return self._tuple_to_file(req_tuple, buildinfo=buildinfo)  # type: ignore[arg-type]
 
-    def add_component_attachment(
+    def _add_component_attachment(
         self, component_id: str, file_id: str, name: str
     ) -> None:
         """See e3.anod.store.interface.StoreWriteInterface."""
@@ -1016,6 +1021,12 @@ def add_component_attachment(
             ["kind", "file_id", "component_id", "attachment_name"],  # type: ignore[arg-type]
             ["attachment", file_id, component_id, name],
         )
+
+    def add_component_attachment(
+        self, component_id: str, file_id: str, name: str
+    ) -> None:
+        """See e3.anod.store.interface.StoreWriteInterface."""
+        self._add_component_attachment(component_id, file_id, name)
         self.connection.commit()
 
     ### PRIVATE ###
@@ -1087,7 +1098,7 @@ def _submit_file(self, file_info: FileDict) -> FileDict:
                 file_info["kind"],
                 resource_id,
                 file_info["revision"],
-                json.dumps(file_info["metadata"]) if file_info["metadata"] else "",
+                json.dumps(file_info["metadata"]) if file_info["metadata"] else "{}",
             ],
         )
         res = self._tuple_to_file(req_tuple, resource=resource)  # type: ignore[arg-type]
@@ -1258,7 +1269,7 @@ def get_build_info_list(
         date = date or "all"
 
         static_where_rules: tuple[str, ...]
-        if date == "all":
+        if not date or date == "all":
             static_where_rules = ()
         else:
             assert len(date) == 8

src/e3/anod/store/buildinfo.py

@@ -79,7 +79,7 @@ def __init__(
         build_version: str,
         isready: bool,
         store: StoreReadInterface | StoreRWInterface | None = None,
-    ):
+    ) -> None:
         """Initialize the BuildInfo object.
 
         :param build_date: Same as the attribute.

src/e3/anod/store/component.py

@@ -115,7 +115,7 @@ def __init__(
         else:
             self.attachments = attachments or {}
 
-        self.releases = releases
+        self.releases = releases or []
         self.is_valid = is_valid
         self.is_published = is_published
         self.build_info = build_info
@@ -583,7 +583,8 @@ def __eq__(self, other: object) -> bool:
             val = getattr(other, attr_name)
             if val != attr_val:
                 logger.debug(
-                    f"Component attribute {attr_name} differ: {val!r} != {attr_val!r}"
+                    f"Component attribute {attr_name!r} differ: "
+                    f"other.{attr_name} = {val!r}, self.{attr_name} = {attr_val!r}"
                 )
                 return False
         return True

src/e3/anod/store/file.py

@@ -23,8 +23,8 @@
     from e3.anod.store.buildinfo import BuildInfo, BuildInfoDict
 
     class ResourceDict(TypedDict):
-        _id: str
-        path: os.PathLike | str
+        id: str
+        path: os.PathLike[str] | str
         size: int
         creation_date: str
 
@@ -103,24 +103,29 @@ def __init__(
         build_info: BuildInfo | None = None,
         metadata: dict[str, Any] | None = None,
         store: StoreReadInterface | StoreRWInterface | None = None,
-        resource_path: str | None = None,
+        resource_path: os.PathLike[str] | str | None = None,
         unpack_dir: str | None = None,
     ) -> None:
         """Initialize a File.
 
-        :param file_id: see corresponding attribute
         :param build_id: see corresponding attribute.
-        :param kind: see corresponding attribute
-        :param name: see corresponding attribute
-        :param resource_id: see corresponding attribute
-        :param filename: see corresponding attribute
-        :param internal: see corresponding attribute (default to True)
-        :param alias: see corresponding attribute (if None, filename is used)
-        :param revision: see corresponding attribute (default is '')
-        :param build_info: see corresponding attribute (default is None)
-        :param metadata: see corresponding attribute (default is None)
-        :param store: a store instance
+        :param kind: see corresponding attribute.
+        :param name: see corresponding attribute.
+        :param filename: see corresponding attribute.
+        :param resource_id: see corresponding attribute.
+        :param file_id: see corresponding attribute.
+        :param internal: see corresponding attribute (default to True).
+        :param alias: see corresponding attribute (if None, filename is used).
+        :param revision: see corresponding attribute (default is '').
+        :param build_info: see corresponding attribute (default is None).
+        :param metadata: see corresponding attribute (default is None).
+        :param store: a store instance.
+        :param resource_path: the path of the file in the current computer. Generaly
+            used to create a file to push.
+        :param unpack_dir: the directory to unpack the file (if the file is an archive).
         """
+        if resource_path:
+            resource_path = os.fspath(resource_path)
         assert file_id is None or isinstance(
             file_id, str
         ), f"invalid file_id: {file_id}"
@@ -188,12 +193,14 @@ def __update(self, file: File) -> None:
         self.unpack_dir = file.unpack_dir
         self.build_info = file.build_info
 
-    def bind_to_resource(self, path: str) -> None:
+    def bind_to_resource(self, path: os.PathLike[str] | str) -> None:
         """Bind this File to the file at the given path.
 
         Unless self.resource_id is already set, this also sets self.resource_id
         using this file's contents (see e3.anod.store.interface.resource_id for
         more info on that).
+
+        :param path: the file path on the current computer.
         """
         if self.resource_id is None:
             self.resource_id = store_resource_id(path)
@@ -474,15 +481,31 @@ def load(
         if "build" in data:
             build_info = BuildInfo.load(data["build"])
 
+        # The `internal` default field value is related to the file kind.
+        #
+        # `internal` is used to said if some files must be distributed with a component
+        # or not.
+        #
+        # For a binary file, the `internal` default value is `False` because this is a
+        # file generated by a component build.
+        #
+        # For any other file type, if internal is not specified, then the default value
+        # is `True` for security reason: If internal is not specified by mistake,
+        # the risk of potential leaks is reduced.
+        kind = data["kind"]
+        internal = data.get("internal")
+        if internal is None:
+            internal = kind != "binary"
+
         try:
             result = File(
                 file_id=data["_id"],
                 build_id=data["build_id"],
-                kind=FileKind(data["kind"]),
+                kind=FileKind(kind),
                 name=data["name"],
                 resource_id=data["resource_id"],
                 filename=data["filename"],
-                internal=bool(data.get("internal")),
+                internal=bool(internal),
                 alias=data.get("alias"),
                 revision=data.get("revision", ""),
                 metadata=data.get("metadata"),
@@ -657,8 +680,12 @@ def __eq__(self, other: object) -> bool:
             if attr_name in ("downloaded_as", "unpack_dir", "store"):
                 # This attribute is not meaningful when comparing two files.
                 continue
-            if getattr(other, attr_name) != attr_val:
-                logger.debug(f"File attribute {attr_name} is different")
+            val = getattr(other, attr_name)
+            if val != attr_val:
+                logger.debug(
+                    f"File attribute {attr_name!r} is different: "
+                    f"other.{attr_name} = {val!r}, self.{attr_name} = {attr_val!r}"
+                )
                 return False
         return True
 

tests/tests_e3/anod/store/store_test.py

@@ -85,11 +85,11 @@ def test_file(store):
     assert f["alias"] == "test.txt"
     assert f["filename"] == "test.txt"
     assert f["revision"] == ""
-    assert f["metadata"] is None
+    assert f["metadata"] == {}
     assert f["build_id"] == buildinfo["_id"]
     assert "resource_id" in f
-    assert "_id" in f["resource"]
-    assert f["resource"]["_id"] == f["resource_id"]
+    assert "id" in f["resource"]
+    assert f["resource"]["id"] == f["resource_id"]
     assert f["resource"]["path"] == path_abs
     assert f["resource"]["size"] == 1
     date = datetime.fromisoformat(f["resource"]["creation_date"]).replace(