Misc minor refactoring

Author: rogerdahl

conftest.py

@@ -41,6 +41,7 @@
 import d1_test.d1_test_case
 import d1_test.instance_generator.random_data
 import d1_test.sample
+import d1_test.test_files
 
 from d1_client.cnclient_1_2 import CoordinatingNodeClient_1_2 as cn_v1
 from d1_client.cnclient_2_0 import CoordinatingNodeClient_2_0 as cn_v2
@@ -53,8 +54,6 @@
 import django.db.transaction
 import django.db.utils
 
-
-
 D1_SKIP_LIST = 'skip_passed/list'
 D1_SKIP_COUNT = 'skip_passed/count'
 
@@ -146,20 +145,18 @@ def pytest_sessionstart(session):
   """Called by pytest before calling session.main()
   - When running in parallel with xdist, this is called once for each worker. By
   default, the number of workers is the same as the number of CPU cores.
-  - NOTE: These cannot be used when running in parallel with xdist.
   """
   exit_if_switch_used_with_xdist([
     '--sample-ask',
     '--sample-update',
     '--sample-review',
-    '--sample-tidy',
+    # --sample-tidy is supported with xdist
     '--pycharm',
     '--fixture-refresh',
     '--skip',
     '--skip-clear',
     '--skip-print',
   ])
-
   if pytest.config.getoption('--sample-tidy'):
     logging.info('Starting sample tidy')
     d1_test.sample.start_tidy()
@@ -169,11 +166,9 @@ def pytest_sessionstart(session):
       'Dropping and creating GMN template database from JSON fixture file'
     )
     db_drop(TEMPLATE_DB_KEY)
-
   if pytest.config.getoption('--skip-clear'):
     logging.info('Clearing list of passed tests')
     _clear_skip_list()
-
   if pytest.config.getoption('--skip-print'):
     logging.info('Printing list of passed tests')
     _print_skip_list()
@@ -260,9 +255,15 @@ def _print_skip_list():
 
 def _open_error_in_pycharm(call):
   """Attempt to open error locations in PyCharm. Use with --exitfirst (-x)"""
-  src_path = call.excinfo.traceback[-1].path
-  src_line = call.excinfo.traceback[-1].lineno + 1
-  d1_test.pycharm.open_and_set_cursor(src_path, src_line)
+  logging.error('Test raised exception: {}'.format(call.excinfo.exconly()))
+  test_path, test_lineno = d1_test.d1_test_case.D1TestCase.get_d1_test_case_location(call.excinfo.tb)
+  logging.error('D1TestCase location: {}:{}'.format(test_path, test_lineno))
+  exc_frame = call.excinfo.traceback.getcrashentry()
+  logging.error('Exception location: {}({})'.format(exc_frame.path, exc_frame.lineno + 1))
+  # src_path = call.excinfo.traceback[-1].path
+  # src_line = call.excinfo.traceback[-1].lineno + 1
+  d1_test.pycharm.open_and_set_cursor(test_path, test_lineno)
+
 
 # Fixtures
 
@@ -351,6 +352,17 @@ def mn_client_v2(request):
 def mn_client_v1_v2(request):
   yield request.param(d1_test.d1_test_case.MOCK_MN_BASE_URL)
 
+@pytest.fixture(
+  scope='function',
+  params=d1_test.test_files.TRICKY_IDENTIFIER_LIST
+)
+def tricky_identifier_tup(request):
+  """Unicode identifiers that use various reserved characters and embedded URL
+  segments. Each fixture is a 2-tuple where the first value is a
+  Unicode identifier and the second is a URL escaped version of the identifier.
+  """
+  yield request.param
+
 
 # Settings
 
@@ -408,9 +420,8 @@ def django_db_setup(request, django_db_blocker):
     # acquire() and release(). It's probably related to how the worker processes
     # relate to each other when launched by pytest-xdist as compared to what the
     # multiprocessing module expects.
-    with posix_ipc.Semaphore(
-        '/{}'.format(__name__), flags=posix_ipc.O_CREAT, initial_value=1
-    ):
+    with posix_ipc.Semaphore('/{}'.format(__name__), flags=posix_ipc.O_CREAT,
+                             initial_value=1):
       logging.warning(
         'LOCK BEGIN {} {}'.format(
           db_get_name_by_key(TEMPLATE_DB_KEY), d1_common.date_time.utc_now()
@@ -464,8 +475,9 @@ def db_create_from_template():
   new_db_name = db_get_name_by_key(TEST_DB_KEY)
   template_db_name = db_get_name_by_key(TEMPLATE_DB_KEY)
   logging.info(
-    'Creating new db from template. new_db="{}" template_db="{}"'.
-    format(new_db_name, template_db_name)
+    'Creating new db from template. new_db="{}" template_db="{}"'.format(
+      new_db_name, template_db_name
+    )
   )
   run_sql(
     'postgres',
@@ -480,7 +492,7 @@ def db_populate_by_json(db_key):
     'loaddata',
     'db_fixture',
     database=db_key,
-    # d1_test.sample.get_path('db_fixture.json.bz2'),
+    # d1_test.sample.get_path_list('db_fixture.json.bz2'),
     # verbosity=0,
     # commit=False,
   )

gmn/src/d1_gmn/app/auth.py

@@ -216,7 +216,6 @@ def assert_allowed(request, level, pid):
   """Assert that one or more subjects are allowed to perform action on object.
   Raise NotAuthorized if object exists and subject is not allowed.
   Raise NotFound if object does not exist.
-  Return NoneType if subject is allowed.
   """
   if not d1_gmn.app.models.ScienceObject.objects.filter(pid__did=pid).exists():
     raise d1_common.types.exceptions.NotFound(

gmn/src/d1_gmn/app/middleware/session_cert.py

@@ -29,21 +29,6 @@
 equivalent identities and group memberships (no SubjectInfo). This limits the
 user's access to data that is publicly available and that is available directly
 to that user (as designated in the Subject DN).
-
-The list of subjects to use for access control is created with the following
-algorithm:
-
-- Start with empty set of subjects
-- Add the symbolic subject, "public"
-- If the connection was made without a certificate:
-  - Stop.
-- Add the symbolic subject, "authenticatedUser"
-- Get the DN from the Subject and serialize it to a standardized string. This
-  string is called Subject below.
-- Add Subject
-- If the certificate does not have a SubjectInfo extension:
-  - Stop.
-- Add subjects from SubjectInfo.
 """
 
 import d1_common.cert.subjects
@@ -71,14 +56,14 @@ def get_subjects(request):
 
 
 def get_authenticated_subjects(cert_pem):
-  primary_str, equivalent_set = d1_common.cert.subjects.extract_subjects(
-    cert_pem.encode('utf-8')
+  """Return primary subject and set of equivalents authenticated by certificate
+  - {cert_pem} can be str or bytes
+  """
+  if isinstance(cert_pem, str):
+    cert_pem = cert_pem.encode('utf-8')
+  return d1_common.cert.subjects.extract_subjects(
+    cert_pem
   )
-  equivalent_set |= {
-    d1_common.const.SUBJECT_PUBLIC,
-    d1_common.const.SUBJECT_AUTHENTICATED,
-  }
-  return primary_str, equivalent_set
 
 
 def _is_certificate_provided(request):

lib_common/src/d1_common/type_conversions.py

@@ -19,20 +19,29 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 """
-In the DataONE Python stack, XML docs are represented in a few different ways.
+- Handle conversions between XML representations used in the D1 Python stack
+- Handle conversions between v1 and v2 DataONE XML types
 
-- Received and transmitted as utf-8 text documents.
-- On the borders of the Python domain, handled as utf-8 or Unicode strings.
-- Schema validation and manipulation in Python code as PyXB binding objects.
-- General processing as ElementTrees.
+In the stack, XML docs are represented as follows:
+
+- As native Unicode str, typically "pretty printed" with indentations, when
+formatted for display
+- As UTF-8 encoded byte strings when send sending or receiving over the network,
+or loading or saving as files
+- Schema validation and manipulation in Python code as PyXB binding objects
+- General processing as ElementTrees
+
+In order to allow conversions between all representations without having to
+implement separate conversions for each combination of input and output
+representation, a "hub and spokes" model is used. Native Unicode str was
+selected as the "hub" representation due to:
 
 - PyXB provides translation to/from string and DOM.
 - ElementTree provides translation to/from string.
-
-We select string as the "hub" representation for XML.
 """
 
 import re
+import xml.etree
 import xml.etree.ElementTree
 
 import pyxb
@@ -48,13 +57,23 @@
 # PyXB shares information about all known types between all imported bindings.
 PYXB_BINDING = d1_common.types.dataoneTypes_v1
 
+# Map common namespace prefixes to namespaces
 NS_DICT = {
   'v1': str(v1_0.Namespace),
   'v1_1': str(v1_1.Namespace),
   'v1_2': str(v1_2.Namespace),
   'v2': str(v2_0.Namespace),
+  'rdfs': 'http://www.w3.org/2000/01/rdf-schema#',
+  'ore': 'http://www.openarchives.org/ore/terms/',
+  'rdf': 'http://www.w3.org/1999/02/22-rdf-syntax-ns#',
+  'dcterms': 'http://purl.org/dc/terms/',
+  'cito': 'http://purl.org/spar/cito/',
 }
 
+
+# Map common namespaces to prefixes
+NS_REVERSE_DICT = { v: k for k, v in NS_DICT.items() }
+
 BINDING_TO_VERSION_TAG_DICT = {
   v1_0: 'v1',
   v1_1: 'v1',
@@ -271,9 +290,19 @@ def str_to_pyxb(xml_str):
   return PYXB_BINDING.CreateFromDocument(xml_str)
 
 
-def str_to_etree(xml_str):
-  return xml.etree.ElementTree.fromstring(xml_str)
+def str_to_etree(xml_str, encoding='utf-8'):
+  """Parse an XML doc to an ElementTree"""
+  # parser = xml.etree.ElementTree.XMLParser(encoding=encoding)
+  # return xml.etree.ElementTree.ElementTree(
+  # return xml.etree.ElementTree.fromstring(xml_str)
+  # )
+  # parser = xml.etree.ElementTree.XMLParser(encoding=encoding)
+  # return xml.etree.ElementTree.ElementTree(
+  #   xml.etree.ElementTree.fromstring(xml_str, parser=parser)
+  # )
 
+  parser = xml.etree.ElementTree.XMLParser(encoding=encoding)
+  return xml.etree.ElementTree.fromstring(xml_str, parser=parser)
 
 def pyxb_to_str(pyxb_obj):
   return pyxb_obj.toxml('utf-8')
@@ -292,21 +321,30 @@ def etree_to_pyxb(etree_obj):
 
 
 # ElementTree
-# https://docs.python.org/2/library/xml.etree.elementtree.html
+
+def replace_namespace_with_prefix(tag_str, ns_reverse_dict=None):
+  """Given a tag on the form "{namespace}name", return "prefix:name"
+  E.g.: {http://www.openarchives.org/ore/terms/}ResourceMap -> ore:ResourceMap
+  """
+  ns_reverse_dict = ns_reverse_dict or NS_REVERSE_DICT
+  for namespace_str, prefix_str in ns_reverse_dict.items():
+    tag_str = tag_str.replace('{{{}}}'.format(namespace_str), '{}:'.format(prefix_str))
+  return tag_str
 
 
 def etree_replace_namespace(etree_obj, ns_str):
   _replace_namespace_recursive(etree_obj, ns_str)
 
 
 def _replace_namespace_recursive(el, ns_str):
-  el.tag = re.sub(r'\{.*\}', '{{{}}}'.format(ns_str), el.tag)
+  el.tag = re.sub(r'{.*\}', '{{{}}}'.format(ns_str), el.tag)
   el.text = el.text.strip() if el.text else None
   el.tail = el.tail.strip() if el.tail else None
   for child_el in el:
     _replace_namespace_recursive(child_el, ns_str)
 
 
+
 def strip_v2_elements(etree_obj):
   """Remove elements and attributes that are only valid in v2 types"""
   if etree_obj.tag == v2_0_tag('logEntry'):
@@ -358,3 +396,5 @@ def strip_node_list(etree_obj):
 
 def v2_0_tag(element_name):
   return '{{{}}}{}'.format(NS_DICT['v2'], element_name)
+
+