From 4976178b833979fa2fea305100231c98af0db0c0 Mon Sep 17 00:00:00 2001
From: nhertz <nhertz@nrao.edu>
Date: Thu, 1 Jul 2021 13:53:47 -0600
Subject: [PATCH] Fixed failing on null `subdirectory` value, and included a
 default value validator that will fill in `subdirectory` if it's `None`; also
 includes test

---
 .../productfetcher/locations.py               | 18 +++++--
 .../productfetcher/tests/test_locations.py    | 52 ++++++++++++++++---
 2 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/apps/cli/executables/pexable/productfetcher/productfetcher/locations.py b/apps/cli/executables/pexable/productfetcher/productfetcher/locations.py
index 90639e3ea..98e898e9f 100644
--- a/apps/cli/executables/pexable/productfetcher/productfetcher/locations.py
+++ b/apps/cli/executables/pexable/productfetcher/productfetcher/locations.py
@@ -1,20 +1,24 @@
 """ LocationsReport conveniences """
 import http
-from abc import abstractmethod, ABC
+import logging
+from abc import ABC, abstractmethod
 from enum import Enum
 from pathlib import Path
 from typing import NamedTuple, Optional
 
-
 # pylint: disable=E0239, E0401, E0402, E1136, R0201, R0902, R0903, R0913, W0613
 import requests
 from marshmallow import Schema, fields, post_load
+from marshmallow.decorators import validates_schema
 from pycapo import CapoConfig
 
-from .exceptions import LocatorServiceException, FetchError
+from .exceptions import FetchError, LocatorServiceException
 from .interfaces import LocatedFile, LocationReport, Locator
 from .ngas import NgasConnection
 
+logger = logging.getLogger("productfetcher.locations")
+logger.setLevel(logging.INFO)
+
 
 class Location(Enum):
     """
@@ -248,7 +252,7 @@ class NgasFileSchema(Schema):
     """ One of the items in a location report's "files" list """
 
     ngas_file_id = fields.Str()
-    subdirectory = fields.Str()
+    subdirectory = fields.Str(allow_none=True)
     relative_path = fields.Str()
     checksum = fields.Integer()
     checksum_type = fields.Str()
@@ -258,6 +262,12 @@ class NgasFileSchema(Schema):
     table = fields.Str()
     archive_uid = fields.Str()
 
+    @validates_schema
+    def validate_subdirectory(self, data, **kwargs):
+        # Fill in a subdirectory value that is None with the default
+        if data["subdirectory"] is None:
+            data["subdirectory"] = "retrieved-products"
+
     @post_load
     def make_filespec(self, data, **kwargs):
         if "table" in data:
diff --git a/apps/cli/executables/pexable/productfetcher/tests/test_locations.py b/apps/cli/executables/pexable/productfetcher/tests/test_locations.py
index 3c0cc50c4..7a4d3f56b 100644
--- a/apps/cli/executables/pexable/productfetcher/tests/test_locations.py
+++ b/apps/cli/executables/pexable/productfetcher/tests/test_locations.py
@@ -4,21 +4,21 @@
 
 from pathlib import Path
 from typing import NamedTuple
+from unittest.mock import MagicMock, patch
 
-from unittest.mock import patch, MagicMock
+import marshmallow.exceptions
 import pytest
-
-
 import requests_mock
-
 from productfetcher.interfaces import LocationReport
 from productfetcher.locations import (
+    Cluster,
     FileLocator,
-    OracleXml,
+    Location,
     NgasFile,
+    NgasFileSchema,
     NgasServer,
-    Location,
-    Cluster,
+    NgasServerSchema,
+    OracleXml,
     ServiceLocator,
 )
 
@@ -151,3 +151,41 @@ def test_direct_copy_detection():
     for location in Location:
         for cluster in Cluster:
             assert not NgasServer("", location, cluster).can_direct_copy_to(home)
+
+
+class TestNgasFileSchema:
+    def test_null_value_in_subdirectory_field(self):
+        """
+        Tests that when the subdirectory value is provided to NgasFileSchema it will accept it and fill it in with
+        a default value
+        """
+        fake_oracle_xml_data = {
+            "archive_uid": "1341983",
+            "table": "cloth",
+            "subdirectory": None,
+            "relative_path": "relpath",
+            "size": 1000000,
+        }
+        try:
+            fake_file = NgasFileSchema().load(data=fake_oracle_xml_data)
+            assert type(fake_file) is OracleXml
+            assert fake_file.subdirectory == "retrieved-products"
+        except marshmallow.exceptions.ValidationError:
+            pytest.fail("Subdirectory field should be nullable.")
+
+        fake_ngas_file_data = {
+            "ngas_file_id": "12345",
+            "subdirectory": None,
+            "relative_path": "relpath",
+            "checksum": 12345,
+            "checksum_type": "foo",
+            "version": 1,
+            "size": 1000000,
+            "server": {"server": "nmngas07", "location": "DSOC", "cluster": "DSOC"},
+        }
+        try:
+            fake_file = NgasFileSchema().load(data=fake_ngas_file_data)
+            assert type(fake_file) is NgasFile
+            assert fake_file.subdirectory == "retrieved-products"
+        except marshmallow.exceptions.ValidationError:
+            pytest.fail("Subdirectory field should be nullable.")
-- 
GitLab