Repository: dag-hammarskjold-library/dlx

Issue:
Include subfield $0 in all serializations
https://github.com/dag-hammarskjold-library/dlx-rest/issues/1539

Hints from issue comments:
@jbukhari: XML and CSV have been covered as of PR #426. ~MRK still remains~ MRK has been covered since #317 

Solution patch:
diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml
index 496c3a07..9269adfa 100644
--- a/.github/workflows/pythonpackage.yml
+++ b/.github/workflows/pythonpackage.yml
@@ -15,12 +15,12 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: ['3.9', '3.10', '3.11', '3.12']
+        python-version: ['3.9', '3.10', '3.11', '3.12', '3.13']
 
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v4
     - name: Set up Python ${{ matrix.python-version }}
-      uses: actions/setup-python@v1
+      uses: actions/setup-python@v5
       with:
         python-version: ${{ matrix.python-version }}
     - name: Install dependencies
diff --git a/dlx/marc/__init__.py b/dlx/marc/__init__.py
index a64e2624..dd8bde5e 100644
--- a/dlx/marc/__init__.py
+++ b/dlx/marc/__init__.py
@@ -134,6 +134,20 @@ def from_aggregation(cls, *args, **kwargs):
     def from_ids(cls, ids):
         return cls.from_query({'_id' : {'$in': ids}})
 
+    @classmethod
+    def sort_table_header(cls, header: list) -> list:
+        # Sorts the header of a MarcSet in util.Table for for use in 
+        # (de)serialization
+
+        return sorted(
+            header, 
+            key=lambda x: ( 
+                (re.match('\d+\.(\w+)', x)).group(1), # sort by tag
+                int(re.match('(\d+)\.', x).group(1)), # sort by prefix group
+                (re.match('\d\.\d{3}\$?(\w)?', x)).group(1) # sort by subfield code
+            )
+        )
+
     @classmethod
     def from_table(cls, table, auth_control=True, auth_flag=False, field_check=None):
         # does not support repeated subfield codes
@@ -144,16 +158,17 @@ def from_table(cls, table, auth_control=True, auth_flag=False, field_check=None)
         for temp_id in table.index.keys():
             record = cls().record_class()
 
-            for field_name in table.index[temp_id].keys():
+            # Sort the headers so that subfield $0 is always first for use in 
+            # subsequent subfields
+            header_fields = list(table.index[temp_id].keys())
+            header_fields = MarcSet.sort_table_header(header_fields)
+
+            for field_name in header_fields:
                 instance = 0
                 value = table.index[temp_id][field_name]
-
-                if value == '':
-                    continue
-
-                match = re.match(r'^(([1-9]+)\.)?(\d{3})(\$)?([a-z0-9])', str(field_name))
-
-                if match:
+                
+                # parse the column header into tag, code and place
+                if match := re.match(r'^(([1-9]+)\.)?(\d{3})(\$)?([a-z0-9])', str(field_name)):
                     if match.group(1):
                         instance = int(match.group(2))
                         instance -= 1 # place numbers start at 1 in col headers instead of 0
@@ -165,26 +180,44 @@ def from_table(cls, table, auth_control=True, auth_flag=False, field_check=None)
                     exceptions.append('Invalid column header "{}"'.format(field_name))
                     continue
 
-                if record.get_value(tag, code, address=[instance,0]):
+                if record.get_value(tag, code, address=[instance, 0]):
                     exceptions.append('Column header {}.{}{} is repeated'.format(instance, tag, code))
                     continue
 
+                # if the subfield field is authority-controlled, use subfield 
+                # $0 as the xref. $0 will be the first subfield in the field if
+                # it exists, because the header has been sorted.
+                xref = None
+
+                if Config.is_authority_controlled(self.record_type, tag, code):
+                    if field := record.get_field(tag, place=instance):
+                        if subfield := field.get_subfield('0'):
+                            xref = int(subfield.value)
+                            value = Auth.lookup(xref, code)
+
+                # flag if specified field value is already in the system
                 if field_check and field_check == tag + (code or ''):
                     if self.record_class.find_one(Condition(tag, {code: value}).compile()):
                         exceptions.append('{}${}: "{}" is already in the system'.format(tag, code, value))
                         continue
 
+                # update the marc object
                 if record.get_field(tag, place=instance):
                     try:
-                        record.set(tag, code, value, address=[instance], auth_control=auth_control)
+                        record.set(tag, code, xref if xref else value, address=[instance], auth_control=auth_control)
                     except Exception as e:
                         exceptions.append(str(e))
                 else:
                     try:
-                        record.set(tag, code, value, address=['+'], auth_control=auth_control)
+                        record.set(tag, code, xref if xref else value, address=['+'], auth_control=auth_control)
                     except Exception as e:
                         exceptions.append(str(e))
 
+                # if this is the last cell in the row, delete any subfield $0
+                if header_fields.index(field_name) == len(header_fields) - 1:
+                    field = record.get_field(tag, instance)
+                    field.subfields = list(filter(lambda x: x.code != '0', field.subfields))
+
             self.records.append(record)
 
         if exceptions:
@@ -215,7 +248,7 @@ def from_xml_raw(cls, root, *, auth_control=False):
         
     @classmethod
     def from_xml(cls, string, auth_control=False):
-        return cls.from_xml_raw(ElementTree.fromstring(string))
+        return cls.from_xml_raw(ElementTree.fromstring(string), auth_control=auth_control)
 
     @classmethod
     def from_mrk(cls, string, *, auth_control=True):
@@ -321,18 +354,19 @@ def to_table(self, *, write_id=True) -> Table:
                 for place, field in enumerate(record.get_fields(tag)):
                     place += 1
 
+                    xref = None
+
                     for subfield in field.subfields:
                         table.set(i, f'{place}.{field.tag}${subfield.code}', subfield.value)
+                        
+                        if hasattr(subfield, 'xref'):
+                            xref = subfield.xref
+                    
+                    if xref:
+                        table.set(i, f'{place}.{field.tag}$0', str(xref))
 
         # sort the table header
-        table.header = sorted(
-            table.header, 
-            key=lambda x: ( 
-                (re.match('\d+\.(\w+)', x)).group(1), # sort by tag
-                int(re.match('(\d+)\.', x).group(1)), # sort by prefix group
-                (re.match('.*\$?(\w)?', x)).group(1) # sort by subfield code
-            )
-        )
+        table.header = MarcSet.sort_table_header(table.header)
 
         return table
 
@@ -1395,7 +1429,7 @@ def from_mrc(cls, string):
             directory = directory[12:]
 
     @classmethod
-    def from_mrk(cls, string, auth_control=True):
+    def from_mrk(cls, string: str, auth_control=True):
         self = cls()
 
         for line in filter(None, string.split('\n')):
@@ -1410,16 +1444,28 @@ def from_mrk(cls, string, auth_control=True):
                 field = Datafield(record_type=cls.record_type, tag=tag, ind1=ind1, ind2=ind2)
                 fallback = {}
                 ambiguous = []
+                
+                # capture the xref from subfield $0, if exists
+                if match := re.search('\$0(\d+)', rest[2:]):
+                    xref = int(match.group(1))
+                else:
+                    xref = None
 
+                # parse the subfields
                 for chunk in filter(None, rest[2:].split('$')):
                     code, value = chunk[0], chunk[1:]
 
+                    if Config.is_authority_controlled(self.record_type, tag, code):
+                        value = xref
+
                     try:
                         field.set(code, value, place='+', auth_control=auth_control)
                     except(AmbiguousAuthValue):
                         fallback[code] = value
                         ambiguous.append(Literal(code, value))
 
+                # attempt to use multiple subfields to resolve ambiguity. may 
+                # be deprecated in the future
                 if fallback and len(fallback) > 1:
                     xrefs = Auth.xlookup_multi(tag, ambiguous, record_type=cls.record_type)
                     
@@ -1428,33 +1474,56 @@ def from_mrk(cls, string, auth_control=True):
                             for code in fallback.keys():
                                 field.set(code, xrefs[0], place='+')
                     elif len(xrefs) > 1:
-                        raise AmbiguousAuthValue('bib', field.tag, '*', str(fallback))
+                        raise AmbiguousAuthValue('bib', field.tag, '*', str(fallback))       
+            
+                # remove subfield $0
+                field.subfields = list(filter(lambda x: x.code != '0', field.subfields))
 
             self.fields.append(field)
 
         return self
     
     @classmethod
-    def from_xml_raw(cls, root, *, auth_control=False):
+    def from_xml_raw(cls, root, *, auth_control=True):
+        if DB.database_name == 'testing':
+            Auth({'_id': 1}).set('150', 'a', 'Header').commit()
+
         assert isinstance(root, ElementTree.Element)
         self = cls()
             
-        for c in filter(lambda x: re.search('controlfield$', x.tag), root):
-            self.set(c.attrib['tag'], None, c.text)
+        for node in filter(lambda x: re.search('controlfield$', x.tag), root):
+            self.set(node.attrib['tag'], None, node.text)
+
+        for field_node in filter(lambda x: re.search('datafield$', x.tag), root):
+            field = Datafield(record_type=cls.record_type, tag=field_node.attrib['tag'], ind1=field_node.attrib['ind1'], ind2=field_node.attrib['ind2'])
+            tag_nodes = filter(lambda x: re.search('subfield$', x.tag), field_node)
+
+            # capture the xref if any
+            xref = None
 
-        for d in filter(lambda x: re.search('datafield$', x.tag), root):
-            field = Datafield(record_type=cls.record_type, tag=d.attrib['tag'], ind1=d.attrib['ind1'], ind2=d.attrib['ind2'])
+            for subfield_node in copy.deepcopy(tag_nodes): # use a copy to prevent the iterating orginal
+                if subfield_node.attrib['code'] == '0':
+                    xref = int(subfield_node.text)
+
+            # iterate though nodes and set the marc values
+            for subfield_node in tag_nodes:
+                if Config.is_authority_controlled(self.record_type, field.tag, subfield_node.attrib['code']):
+                    value = xref if xref else subfield_node.text
+                else:
+                    value = str(subfield_node.text)
+
+                # .set handles auth control
+                field.set(subfield_node.attrib['code'], value, auth_control=auth_control, place='+')
             
-            for s in filter(lambda x: re.search('subfield$', x.tag), d):
-                field.set(s.attrib['code'], s.text, auth_control=auth_control, place='+')
+            field.subfields = list(filter(lambda x: x.code != '0', field.subfields))
                 
             self.fields.append(field)
             
         return self
         
     @classmethod
-    def from_xml(cls, string):
-        return cls.from_xml_raw(ElementTree.fromstring(string))
+    def from_xml(cls, string, auth_control=True):
+        return cls.from_xml_raw(ElementTree.fromstring(string), auth_control=auth_control)
 
     @classmethod
     def from_json(cls, string, auth_control=False):
@@ -2079,7 +2148,11 @@ def set(self, code, new_val, *, ind1=None, ind2=None, place=0, auth_control=True
 
         if new_val:
             # existing subfield
-            # walk to the tree to replace the subfield object
+            if auth_control == False:
+                # force string because only xrefs can be ints
+                new_val = str(new_val)
+
+            # walk the tree to replace the subfield object
             i, j = 0, 0
             
             for i, sub in enumerate(self.subfields):
diff --git a/dlx/util.py b/dlx/util.py
index 4d2c016a..4eb9aaba 100644
--- a/dlx/util.py
+++ b/dlx/util.py
@@ -63,7 +63,7 @@ def set(self, rowx, field_name, value):
         self.index[rowx].setdefault(field_name, {})
 
         if existing := self.index[rowx].get(field_name):
-            # if this is a repeated subfield, append the next value 
+            # if this is a repeated field, append the next value into this cell
             self.index[rowx][field_name] += f'||{value}'
         else:
             self.index[rowx][field_name] = value
diff --git a/requirements.txt b/requirements.txt
index 6b087076..5bcbd58a 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,42 +1,12 @@
-attrs==23.2.0
 boto3==1.35.53
 certifi==2024.7.4
-cffi==1.16.0
-charset-normalizer==3.3.2
-click==8.1.7
-cryptography==43.0.1
-dnspython==2.6.1
-exceptiongroup==1.2.2
-idna==3.7
-iniconfig==2.0.0
-Jinja2==3.1.4
-jmespath==1.0.1
-joblib==1.4.2
 jsonschema==4.0.0
-lxml==5.2.1
-MarkupSafe==2.1.5
 mongomock==4.1.2
 moto==5.0.8
 nltk==3.9.1
-packaging==24.1
-pluggy==1.5.0
-pycparser==2.22
-pymongo==4.6.3
-pyrsistent==0.20.0
+pymongo==4.10.1
 pytest==8.2.0
-python-dateutil==2.9.0.post0
-pytz==2024.1
-PyYAML==6.0.2
-regex==2024.9.11
 requests>=2.32.2
 responses==0.25.3
-s3transfer==0.10.3
-sentinels==1.0.0
-six==1.16.0
-tomli==2.0.2
-tqdm==4.66.5
-urllib3==1.26.19
-Werkzeug==3.1.0
 xlrd==2.0.1
 xmldiff==2.7.0
-xmltodict==0.13.0


Test patch:
diff --git a/tests/test_marcset.py b/tests/test_marcset.py
index d7ff4b84..7cc4c1de 100644
--- a/tests/test_marcset.py
+++ b/tests/test_marcset.py
@@ -80,28 +80,41 @@ def test_from_ids(db):
     
     bibs = BibSet.from_ids([1, 2])
     assert [x.id for x in bibs] == [1, 2]
-        
+
+def test_sort_table_header():
+    from dlx.marc import MarcSet
+
+    header = ['1.269$a', '1.246$b', '1.246$a', '1.650$a', '1.650$0']
+
+    assert MarcSet.sort_table_header(header) == ['1.246$a',  '1.246$b',  '1.269$a', '1.650$0', '1.650$a']
+
 def test_from_table(db):
     from dlx.marc import BibSet
     from dlx.util import Table
     
-    t = Table([
-        ['246a',  '1.246$b',  '1.269c',    '2.269c'],
-        ['title', 'subtitle', '1999-12-31','repeated'],
-        ['title2','subtitle2','2000-01-01','repeated'],
+    table = Table([
+        ['1.246$a',  '1.246$b',  '1.269$c', '2.269$c', '1.650$a', '1.650$0'],
+        ['title', 'subtitle', '1999-12-31','repeated', '', 1],
+        ['title2','subtitle2','2000-01-01','repeated', '', 1],
     ])
     
-    bibset = BibSet.from_table(t)
+    bibset = BibSet.from_table(table)
+
     for bib in bibset.records:
         assert bib.get_value('246','b')[:8] == 'subtitle'
         assert bib.get_values('269','c')[1] == 'repeated'
+        assert bib.get_value('650', 'a') == 'Header'
+        assert not bib.get_value('650', '0')
         
     with pytest.raises(Exception):
+        # dupe field check
         bibset = BibSet.from_table(Table([['245a'], ['This']]), field_check='245a')
 
     with pytest.raises(Exception):
+        # auth control
         bibset = BibSet.from_table(Table([['650a'], ['Invalid']]), auth_control=True)
 
+
 @pytest.mark.skip(reason='xlrd is obsolete. needs review')
 def test_from_excel():
     from dlx.marc import BibSet
@@ -116,16 +129,57 @@ def test_from_excel():
 def test_from_mrk(db):
     from dlx.marc import BibSet
 
-    control = '=000  leader\n=008  controlfield\n=245  \\\\$aThis$bis the$ctitle\n=520  \\\\$aDescription\n=520  \\\\$aAnother description$aRepeated subfield\n=650  \\\\$aHeader\n=710  \\\\$aAnother header\n\n=000  leader\n=245  \\\\$aAnother$bis the$ctitle\n=650  \\\\$aHeader\n'
+    control = '=000  leader\n=008  controlfield\n=245  \\\\$aThis$bis the$ctitle\n=520  \\\\$aDescription\n=520  \\\\$aAnother description$aRepeated subfield\n=650  \\\\$aWill be replaced because of xref$01\n=710  \\\\$aAnother header\n\n=000  leader\n=245  \\\\$aAnother$bis the$ctitle\n=650  \\\\$aHeader\n'
     bibs = BibSet.from_mrk(control)
     assert(len(bibs.records)) == 2
+    assert bibs.records[0].get_value('650', 'a') == 'Header'
 
 def test_from_xml(db):
     from dlx.marc import BibSet
     
-    string = '<collection><record><controlfield tag="000">leader</controlfield><controlfield tag="008">controlfield</controlfield><datafield ind1=" " ind2=" " tag="245"><subfield code="a">This</subfield><subfield code="b">is the</subfield><subfield code="c">title</subfield></datafield><datafield ind1=" " ind2=" " tag="520"><subfield code="a">Description</subfield></datafield><datafield ind1=" " ind2=" " tag="520"><subfield code="a">Another description</subfield><subfield code="a">Repeated subfield</subfield></datafield><datafield ind1=" " ind2=" " tag="650"><subfield code="a">Header</subfield><subfield code="0">1</subfield></datafield><datafield ind1=" " ind2=" " tag="710"><subfield code="a">Another header</subfield><subfield code="0">2</subfield></datafield></record><record><controlfield tag="000">leader</controlfield><datafield ind1=" " ind2=" " tag="245"><subfield code="a">Another</subfield><subfield code="b">is the</subfield><subfield code="c">title</subfield></datafield><datafield ind1=" " ind2=" " tag="650"><subfield code="a">Header</subfield><subfield code="0">1</subfield></datafield></record></collection>'
-    bibset = BibSet.from_xml(string)
+    string = '''
+        <collection>
+            <record>
+                <controlfield tag="000">leader</controlfield>
+                <controlfield tag="008">controlfield</controlfield>
+                <datafield ind1=" " ind2=" " tag="245">
+                    <subfield code="a">This</subfield>
+                    <subfield code="b">is the</subfield>
+                    <subfield code="c">title</subfield>
+                </datafield><datafield ind1=" " ind2=" " tag="520">
+                    <subfield code="a">Description</subfield>
+                </datafield>
+                <datafield ind1=" " ind2=" " tag="520">
+                    <subfield code="a">Another description</subfield>
+                    <subfield code="a">Repeated subfield</subfield>
+                </datafield>
+                <datafield ind1=" " ind2=" " tag="650">
+                    <subfield code="a">Header</subfield>
+                    <subfield code="0">1</subfield>
+                </datafield>
+                <datafield ind1=" " ind2=" " tag="710">
+                    <subfield code="a">Another header</subfield>
+                    <subfield code="0">2</subfield>
+                </datafield>
+            </record>
+            <record>
+                <controlfield tag="000">leader</controlfield>
+                <datafield ind1=" " ind2=" " tag="245">
+                    <subfield code="a">Another</subfield>
+                    <subfield code="b">is the</subfield>
+                    <subfield code="c">title</subfield>
+                </datafield>
+                <datafield ind1=" " ind2=" " tag="650">
+                    <subfield code="a">head</subfield>
+                    <subfield code="0">1</subfield>
+                </datafield>
+            </record>
+        </collection>
+    '''
+
+    bibset = BibSet.from_xml(string, auth_control=True)
     assert len(bibset.records) == 2
+    assert bibset.records[1].get_value('650', 'a') == 'Header'
         
 def test_to_mrc(db):
     from dlx.marc import BibSet
@@ -167,9 +221,9 @@ def test_to_csv(db):
 
     bibset = BibSet.from_query({})
     bibset.records = list(bibset.records)
-    #[bib.set('001', None, str(bib.id)) for bib in bibset.records]
-    assert bibset.to_csv(write_id=True) == '1.001,1.245$a,1.245$b,1.245$c,1.520$a,2.520$a,1.650$a,1.710$a\n1,This,is the,title,Description,Another description||Repeated subfield,Header,Another header\n2,Another,is the,title,,,Header,'
-
+    control = '1.001,1.245$a,1.245$b,1.245$c,1.520$a,2.520$a,1.650$a,1.650$0,1.710$a,1.710$0\n1,This,is the,title,Description,Another description||Repeated subfield,Header,1,Another header,2\n2,Another,is the,title,,,Header,1,,'
+    assert bibset.to_csv(write_id=True) == control
+    
     # comma and quote handling
     bibs = BibSet()
     bibs.records += [Bib().set('245', 'a', 'A title, with a comma').set('245', 'b', 'subtitle'), Bib().set('245', 'a', 'A "title, or name" with double quotes in the middle').set('245', 'b', 'subtitle')]
diff --git a/tests/test_scripts.py b/tests/test_scripts.py
index d5246e9b..349fbbd6 100644
--- a/tests/test_scripts.py
+++ b/tests/test_scripts.py
@@ -69,10 +69,14 @@ def test_auth_merge(db):
     assert Auth.from_id(2) is None
 
 def test_import_marc(db, bibs, auths):
+    from dlx import DB
+    from dlx.marc import Auth
     from dlx.scripts import marc_import
 
     control = os.path.dirname(__file__) + '/marc.mrk'
 
+    # these functions do not hve access to the fixture data for auth validation
+
     assert marc_import.run(
         connect='mongomock://localhost', 
         type='bib',