Hash files before upload and compare hash and size

Author: mcarans

requirements.txt

@@ -313,7 +313,7 @@ urllib3==2.5.0
     #   requests
 validators==0.35.0
     # via frictionless
-virtualenv==20.33.1
+virtualenv==20.34.0
     # via pre-commit
 watchdog==6.0.0
     # via mkdocs

src/hdx/api/utilities/filestore_helper.py

@@ -2,6 +2,7 @@
 
 from typing import TYPE_CHECKING, Any, Dict
 
+from hdx.api.utilities.size_hash import get_size_and_hash
 from hdx.utilities.dateparse import now_utc_notz
 
 if TYPE_CHECKING:
@@ -60,19 +61,25 @@ def check_filestore_resource(
         cls.resource_check_required_fields(resource_data_to_update, **kwargs)
         file_to_upload = resource_data_to_update.get_file_to_upload()
         if file_to_upload:
+            file_format = resource_data_to_update.get("format", "").lower()
+            size, hash = get_size_and_hash(file_to_upload, file_format)
             filestore_resources[resource_index] = file_to_upload
             resource_data_to_update["url"] = cls.temporary_url
+            resource_data_to_update["size"] = size
+            resource_data_to_update["hash"] = hash
 
     @classmethod
     def dataset_update_filestore_resource(
         cls,
+        original_resource_data: "Resource",
         resource_data_to_update: "Resource",
         filestore_resources: Dict[int, str],
         resource_index: int,
     ) -> None:
         """Helper method to merge updated resource from dataset into HDX resource read from HDX including filestore.
 
         Args:
+            original_resource_data (Resource): Original resource from dataset
             resource_data_to_update (Resource): Updated resource from dataset
             filestore_resources (Dict[int, str]): List of (index of resources, file to upload)
             resource_index (int): Index of resource
@@ -82,11 +89,21 @@ def dataset_update_filestore_resource(
         """
         file_to_upload = resource_data_to_update.get_file_to_upload()
         if file_to_upload:
-            filestore_resources[resource_index] = file_to_upload
-            resource_data_to_update["url"] = cls.temporary_url
-
-        data_updated = resource_data_to_update.is_marked_data_updated()
-        if data_updated:
+            file_format = resource_data_to_update.get("format", "").lower()
+            size, hash = get_size_and_hash(file_to_upload, file_format)
+            if size == original_resource_data.get(
+                "size"
+            ) and hash == original_resource_data.get("hash"):
+                # ensure last_modified is not updated if file hasn't changed
+                if "last_modified" in resource_data_to_update:
+                    del resource_data_to_update["last_modified"]
+            else:
+                # update file if size or hash has changed
+                filestore_resources[resource_index] = file_to_upload
+                resource_data_to_update["url"] = cls.temporary_url
+                resource_data_to_update["size"] = size
+                resource_data_to_update["hash"] = hash
+        elif resource_data_to_update.is_marked_data_updated():
             # Should not output timezone info here
             resource_data_to_update["last_modified"] = now_utc_notz().isoformat(
                 timespec="microseconds"

src/hdx/api/utilities/size_hash.py

@@ -0,0 +1,43 @@
+import hashlib
+from io import BytesIO
+from typing import Tuple
+
+from openpyxl import load_workbook
+
+
+def get_size_and_hash(file_to_upload: str, file_format: str) -> Tuple[int, str]:
+    """Return the size and hash of file to upload
+
+    Args:
+        file_to_upload: File to upload
+        file_format (str): File format
+
+    Returns:
+        Tuple[int, str]: Tuple (size, hash)
+    """
+    f = open(file_to_upload, "rb")
+    md5hash = hashlib.md5()
+    if file_format == "xlsx":
+        first_chunk = f.read(4096)
+        size = len(first_chunk)
+        signature = first_chunk[:4]
+        if signature == b"PK\x03\x04":  # xlsx
+            xlsxbuffer = bytearray(first_chunk)
+            while chunk := f.read(4096):
+                size += len(chunk)
+                xlsxbuffer.extend(chunk)
+            workbook = load_workbook(filename=BytesIO(xlsxbuffer), read_only=True)
+            for sheet_name in workbook.sheetnames:
+                sheet = workbook[sheet_name]
+                for cols in sheet.iter_rows(values_only=True):
+                    md5hash.update(bytes(str(cols), "utf-8"))
+            workbook.close()
+            return size, md5hash.hexdigest()
+        else:
+            md5hash.update(first_chunk)
+    else:
+        size = 0
+    while chunk := f.read(4096):
+        size += len(chunk)
+        md5hash.update(chunk)
+    return size, md5hash.hexdigest()

src/hdx/data/dataset.py

@@ -898,6 +898,7 @@ def _dataset_update_resources(
                     ]
                     logger.warning(f"Resource exists. Updating {resource['name']}")
                     FilestoreHelper.dataset_update_filestore_resource(
+                        resource,
                         resource_data_to_update,
                         filestore_resources,
                         i,
@@ -930,13 +931,15 @@ def _dataset_update_resources(
                 ):
                     if len(self.resources) > i:
                         updated_resource_name = resource_data_to_update["name"]
-                        resource_name = self.resources[i]["name"]
+                        resource = self.resources[i]
+                        resource_name = resource["name"]
                         logger.warning(f"Resource exists. Updating {resource_name}")
                         if resource_name != updated_resource_name:
                             logger.warning(
                                 f"Changing resource name to: {updated_resource_name}"
                             )
                         FilestoreHelper.dataset_update_filestore_resource(
+                            resource,
                             resource_data_to_update,
                             filestore_resources,
                             i,

src/hdx/data/resource.py

@@ -1,6 +1,5 @@
 """Resource class containing all logic for creating, checking, and updating resources."""
 
-import hashlib
 import logging
 import warnings
 from datetime import datetime
@@ -12,6 +11,7 @@
 from hdx.api.configuration import Configuration
 from hdx.api.utilities.date_helper import DateHelper
 from hdx.api.utilities.filestore_helper import FilestoreHelper
+from hdx.api.utilities.size_hash import get_size_and_hash
 from hdx.data.hdxobject import HDXError, HDXObject
 from hdx.data.resource_view import ResourceView
 from hdx.utilities.dateparse import now_utc, now_utc_notz, parse_date
@@ -359,18 +359,6 @@ def check_required_fields(self, ignore_fields: ListTuple[str] = tuple()) -> None
         self.check_url_filetoupload()
         self._check_required_fields("resource", ignore_fields)
 
-    def _get_hash(self) -> str:
-        """Return the hash of file to upload
-
-        Returns:
-            str: Hash of file to upload
-        """
-        md5 = hashlib.md5()
-        f = open(self.file_to_upload, "rb")
-        while chunk := f.read(4096):
-            md5.update(chunk)
-        return md5.hexdigest()
-
     def _resource_merge_hdx_update(
         self,
         **kwargs: Any,
@@ -389,9 +377,16 @@ def _resource_merge_hdx_update(
         data_updated = kwargs.pop("data_updated", self.data_updated)
         files = {}
         if self.file_to_upload:
-            hash = self._get_hash()
-            if hash != self.data.get("hash"):  # update file if hash has changed
+            file_format = self.old_data.get("format", "").lower()
+            size, hash = get_size_and_hash(self.file_to_upload, file_format)
+            if size == self.data.get("size") and hash == self.data.get("hash"):
+                # ensure last_modified is not updated if file hasn't changed
+                if "last_modified" in self.data:
+                    del self.data["last_modified"]
+            else:
+                # update file if size or hash has changed
                 files["upload"] = self.file_to_upload
+                self.old_data["size"] = size
                 self.old_data["hash"] = hash
         elif data_updated:
             # Should not output timezone info here
@@ -403,7 +398,7 @@ def _resource_merge_hdx_update(
         # old_data will be merged into data in the next step
         self._merge_hdx_update("resource", "id", files, True, **kwargs)
 
-    def update_in_hdx(self, **kwargs: Any) -> None:
+    def update_in_hdx(self, **kwargs: Any) -> int:
         """Check if resource exists in HDX and if so, update it. To indicate
         that the data in an external resource (given by a URL) has been
         updated, set data_updated to True, which will result in the resource
@@ -418,7 +413,7 @@ def update_in_hdx(self, **kwargs: Any) -> None:
             date_data_updated (datetime): Date to use for last_modified. Default to None.
 
         Returns:
-            None
+            int: Return status code
         """
         self._check_load_existing_object("resource", "id")
         if self.file_to_upload and "url" in self.data:
@@ -454,7 +449,9 @@ def create_in_hdx(self, **kwargs: Any) -> None:
             files = {}
             if self.file_to_upload:
                 files["upload"] = self.file_to_upload
-                self.data["hash"] = self._get_hash()
+                self.data["size"], self.data["hash"] = get_size_and_hash(
+                    self.file_to_upload, self.get_format()
+                )
             self._save_to_hdx("create", "name", files, True)
 
     def delete_from_hdx(self) -> None:

…ll-Africa-File_20170101-to-20170708.xlsx …ll-Africa-File_20170101-to-20170708.xlsxtests/fixtures/datastore/ACLED-All-Africa-File_20170101-to-20170708.xlsx renamed to tests/fixtures/size_hash/ACLED-All-Africa-File_20170101-to-20170708.xlsx tests/fixtures/datastore/ACLED-All-Africa-File_20170101-to-20170708.xlsx renamed to tests/fixtures/size_hash/ACLED-All-Africa-File_20170101-to-20170708.xlsx

@@ -46,10 +46,6 @@ def configuration(self):
     def datasetmetadata(self):
         return join("tests", "fixtures", "CKAN", "hdx_dataset_static.yaml")
 
-    @pytest.fixture(scope="function")
-    def testdata(self):
-        return join("tests", "fixtures", "test_data.csv")
-
     @pytest.fixture(scope="class")
     def params(self):
         return {
@@ -101,7 +97,7 @@ def setup_teardown_folder(self, configuration, gclient, params):
     def test_create_dataset(
         self,
         datasetmetadata,
-        testdata,
+        test_data,
         setup_teardown_folder,
         params,
     ):
@@ -156,7 +152,7 @@ def create_resource():
             filestore = resource_no % 2 == 0
             if filestore:
                 resource.set_format("csv")
-                resource.set_file_to_upload(testdata)
+                resource.set_file_to_upload(test_data)
             else:
                 wks, url = create_gsheet(
                     "resource1",
@@ -217,7 +213,7 @@ def create_resource():
         resources.pop()
         gsheet_resource = resources[5]
         gsheet_resource.set_format("csv")
-        gsheet_resource.set_file_to_upload(testdata)
+        gsheet_resource.set_file_to_upload(test_data)
         for resource in resources:
             del resource["package_id"]
         dataset.add_update_resources(resources)
@@ -249,6 +245,8 @@ def create_resource():
                     assert "humdata" not in updated_resource["url"]
             else:
                 assert "humdata" in updated_resource["url"]
+            assert updated_resource.get("size") == resource.get("size")
+            assert updated_resource.get("hash") == resource.get("hash")
 
         # modify dataset again starting with existing dataset
         title = "HDX Python API test changed again"
@@ -260,7 +258,7 @@ def create_resource():
         countryiso3s.append("YEM")
         dataset.add_country_location("YEM")
         dataset.delete_resource(updated_resources[5])
-        updated_resources[0].set_file_to_upload(testdata)
+        updated_resources[0].set_file_to_upload(test_data)
         create_resource()
         resources = dataset.get_resources()
 
@@ -294,6 +292,9 @@ def create_resource():
                     assert "humdata" not in updated_resource["url"]
             else:
                 assert "humdata" in updated_resource["url"]
+            if i != 7:
+                assert updated_resource.get("size") == resource.get("size")
+                assert updated_resource.get("hash") == resource.get("hash")
 
         # tear down
         dataset.delete_from_hdx()

tests/fixtures/update_dataset_resources/expected_resources_to_update.json

@@ -16,12 +16,13 @@
 
 
 class TestFilestoreHelper:
-    def test_dataset_update_filestore_resource(self, configuration):
+    def test_dataset_update_filestore_resource(self, configuration, test_data):
         resource_data_copy = copy.deepcopy(resource_data)
+        orig_resource = Resource(resource_data_copy)
         resource = Resource(resource_data_copy)
         filestore_resources = {}
         FilestoreHelper.dataset_update_filestore_resource(
-            resource, filestore_resources, 0
+            orig_resource, resource, filestore_resources, 0
         )
         assert resource == {
             "api_type": "api",
@@ -34,25 +35,45 @@ def test_dataset_update_filestore_resource(self, configuration):
         }
         assert filestore_resources == {}
 
-        resource.set_file_to_upload("test")
+        resource.set_file_to_upload(test_data)
         FilestoreHelper.dataset_update_filestore_resource(
-            resource, filestore_resources, 0
+            orig_resource, resource, filestore_resources, 0
         )
         assert resource == {
             "api_type": "api",
             "description": "My Resource",
             "format": "xlsx",
             "name": "MyResource1",
+            "hash": "3790da698479326339fa99a074cbc1f7",
+            "size": 1548,
             "package_id": "6f36a41c-f126-4b18-aaaf-6c2ddfbc5d4d",
             "resource_type": "api",
             "url": "updated_by_file_upload_step",
         }
-        assert filestore_resources == {0: "test"}
+        assert filestore_resources == {0: test_data}
 
+        filestore_resources = {}
+        FilestoreHelper.dataset_update_filestore_resource(
+            resource, resource, filestore_resources, 0
+        )
+        assert resource == {
+            "api_type": "api",
+            "description": "My Resource",
+            "format": "xlsx",
+            "name": "MyResource1",
+            "hash": "3790da698479326339fa99a074cbc1f7",
+            "size": 1548,
+            "package_id": "6f36a41c-f126-4b18-aaaf-6c2ddfbc5d4d",
+            "resource_type": "api",
+            "url": "updated_by_file_upload_step",
+        }
+        assert filestore_resources == {}
+
+        resource.file_to_upload = None
         resource.mark_data_updated()
         FilestoreHelper.dataset_update_filestore_resource(
-            resource, filestore_resources, 0
+            orig_resource, resource, filestore_resources, 0
         )
         regex = r"^\d\d\d\d-\d\d-\d\dT\d\d:\d\d:\d\d.\d\d\d\d\d\d$"
         assert re.match(regex, resource["last_modified"])
-        assert filestore_resources == {0: "test"}
+        assert filestore_resources == {}

tests/hdx/api/test_ckan.py

@@ -0,0 +1,24 @@
+from os.path import join
+
+import pytest
+
+from hdx.api.utilities.size_hash import get_size_and_hash
+
+
+class TestSizeHash:
+    @pytest.fixture(scope="class")
+    def test_xlsx(self, fixturesfolder):
+        return join(
+            fixturesfolder,
+            "size_hash",
+            "ACLED-All-Africa-File_20170101-to-20170708.xlsx",
+        )
+
+    def test_get_size_and_hash(self, test_data, test_xlsx):
+        size, hash = get_size_and_hash(test_data, "csv")
+        assert size == 1548
+        assert hash == "3790da698479326339fa99a074cbc1f7"
+
+        size, hash = get_size_and_hash(test_xlsx, "xlsx")
+        assert size == 23724
+        assert hash == "6b8acf7e28d62685a1e829e7fa220d17"