Only apply preferences if they all can be applied

Update tests

lakesuperior/endpoints/ldp.py

@@ -73,6 +73,13 @@ std_headers = {
 vw_blacklist = {
 }
 
+"""Prefer representations currently supported"""
+option_to_uri = {
+    'embed_children': Ldpr.EMBED_CHILD_RES_URI,
+    'incl_children': Ldpr.RETURN_CHILD_RES_URI,
+    'incl_inbound': Ldpr.RETURN_INBOUND_REF_URI,
+    'incl_srv_mgd': Ldpr.RETURN_SRV_MGD_RES_URI
+}
 
 ldp = Blueprint(
         'ldp', __name__, template_folder='templates',
@@ -654,12 +661,6 @@ def parse_repr_options(retr_opts, out_headers):
     :param dict out_headers:: Response headers.
     """
     logger.debug('Parsing retrieval options: {}'.format(retr_opts))
-    uri_to_option = {
-        'embed_children': Ldpr.EMBED_CHILD_RES_URI,
-        'incl_children': Ldpr.RETURN_CHILD_RES_URI,
-        'incl_inbound': Ldpr.RETURN_INBOUND_REF_URI,
-        'incl_srv_mgd': Ldpr.RETURN_SRV_MGD_RES_URI
-    }
 
     if retr_opts.get('value') == 'minimal':
         imr_options = {
@@ -668,7 +669,7 @@ def parse_repr_options(retr_opts, out_headers):
             'incl_inbound' : False,
             'incl_srv_mgd' : False,
         }
-        out_headers['Preference-Applied'] = "return=\"minimal\""
+        out_headers['Preference-Applied'] = 'return="minimal"'
     else:
         # Default.
         imr_options = {
@@ -680,26 +681,29 @@ def parse_repr_options(retr_opts, out_headers):
 
         # Override defaults.
         if 'parameters' in retr_opts:
-            pref_imr_options = _valid_preferences(retr_opts)
-            include = list()
-            omit = list()
-            for k, v in pref_imr_options.items():
-                # pref_imr_options only contains requested preferences, override the defaults for those.
-                imr_options[k] = v
-                # This creates Preference-Applied headers for those things we support.
-                if v:
-                    list_holder = include
-                else:
-                    list_holder = omit
-                list_holder.append(uri_to_option[k])
-
-            header_output = ""
-            if len(include) > 0:
-                header_output += " include=\"" + " ".join(include) + "\";"
-            if len(omit) > 0:
-                header_output += " omit=\"" + " ".join(omit) + "\""
-            if len(header_output) > 0:
-                out_headers['Preference-Applied'] = "return=representation;" + header_output
+            try:
+                pref_imr_options = _valid_preferences(retr_opts)
+                include = list()
+                omit = list()
+                for k, v in pref_imr_options.items():
+                    # pref_imr_options only contains requested preferences, override the defaults for those.
+                    imr_options[k] = v
+                    # This creates Preference-Applied headers for those things we support.
+                    if v:
+                        list_holder = include
+                    else:
+                        list_holder = omit
+                    list_holder.append(str(option_to_uri[k]))
+                header_output = ''
+                if len(include) > 0:
+                    header_output += ' include="' + ' '.join(include) + '";'
+                if len(omit) > 0:
+                    header_output += ' omit="' + ' '.join(omit) + '";'
+                if len(header_output) > 0:
+                    out_headers['Preference-Applied'] = 'return=representation;' + header_output
+            except KeyError:
+                # Invalid Prefer header so we disregard the entire thing.
+                pass
 
     logger.debug('Retrieval options: {}'.format(pformat(imr_options)))
 
@@ -718,7 +722,7 @@ def _preference_decision(include, omit, header):
     if str(header) in include or str(header) in omit:
         if str(header) in include and str(header) in omit:
             # You can't include and omit, so ignore it.
-            return None
+            raise KeyError('Can\'t include and omit same preference')
         else:
             return str(header) in include
     return None
@@ -742,12 +746,14 @@ def _valid_preferences(retr_opts):
     logger.debug('Include: {}'.format(include))
     logger.debug('Omit: {}'.format(omit))
 
-    imr_options['embed_children'] = _preference_decision(include, omit, Ldpr.EMBED_CHILD_RES_URI)
-    imr_options['incl_children'] = _preference_decision(include, omit, Ldpr.RETURN_CHILD_RES_URI)
-    imr_options['incl_inbound'] = _preference_decision(include, omit, Ldpr.RETURN_INBOUND_REF_URI)
-    imr_options['incl_srv_mgd'] = _preference_decision(include, omit, Ldpr.RETURN_SRV_MGD_RES_URI)
-
-    imr_options = {k: v for k, v in imr_options.items() if v is not None}
+    distinct_representations = include.copy()
+    distinct_representations.extend(omit)
+    distinct_representations = set(distinct_representations)
+    uri_to_option = {str(v): k for k, v in option_to_uri.items()}
+    for uri in distinct_representations:
+        # Throws KeyError if we don't support the header
+        option = uri_to_option[uri]
+        imr_options[option] = _preference_decision(include, omit, uri)
 
     return imr_options
 

tests/3_endpoints/test_3_0_ldp.py

@@ -1875,7 +1875,7 @@ class TestPrefHeader:
 
     def test_contradicting_prefs(self):
         """
-        Test include and omit the same preference.
+        Test include and omit the same preference. Does not apply a preference or return a Preference-Applied header.
         """
         self.client.put('/ldp/test_contradicting_prefs01', content_type='text/turtle')
 
@@ -1886,19 +1886,27 @@ class TestPrefHeader:
         self._assert_pref_applied(resp1)
 
         resp2 = self.client.get('/ldp/test_contradicting_prefs01', headers={
-            'prefer': 'return=representation; include=\"{0} {1}\"; omit={0}'.format(Ldpr.RETURN_CHILD_RES_URI,
-                                                                                    Ldpr.RETURN_SRV_MGD_RES_URI)
+            'prefer': (
+                'return=representation; include="{0} {1}"; omit={0}'.format(
+                    Ldpr.RETURN_CHILD_RES_URI,
+                    Ldpr.RETURN_SRV_MGD_RES_URI
+                )
+            )
         })
         assert resp2.status_code == 200
-        self._assert_pref_applied(resp2, include=[Ldpr.RETURN_SRV_MGD_RES_URI])
+        self._assert_pref_applied(resp2)
 
         resp3 = self.client.get('/ldp/test_contradicting_prefs01', headers={
-            'prefer': 'return=representation; include=\"{0} {1}\"; omit=\"{0} {2}\"'.format(Ldpr.EMBED_CHILD_RES_URI,
-                                                                                            Ldpr.RETURN_SRV_MGD_RES_URI,
-                                                                                            Ldpr.RETURN_INBOUND_REF_URI)
+            'prefer': (
+                'return=representation; include="{0} {1}"; omit="{0} {2}"'.format(
+                    Ldpr.EMBED_CHILD_RES_URI,
+                    Ldpr.RETURN_SRV_MGD_RES_URI,
+                    Ldpr.RETURN_INBOUND_REF_URI
+                )
+            )
         })
         assert resp3.status_code == 200
-        self._assert_pref_applied(resp3, include=[str(Ldpr.RETURN_SRV_MGD_RES_URI)], omit=[str(Ldpr.RETURN_INBOUND_REF_URI)])
+        self._assert_pref_applied(resp3)
 
     def test_multiple_preferences(self):
         """
@@ -1907,24 +1915,107 @@ class TestPrefHeader:
         self.client.put('/ldp/test_multiple_preferences01', content_type='text/turtle')
 
         resp1 = self.client.get('/ldp/test_multiple_preferences01', headers={
-            'prefer': 'return=representation; include=\"{0} {1}\"; omit=\"{2}\"'.format(Ldpr.RETURN_CHILD_RES_URI,
-                                                                                        Ldpr.EMBED_CHILD_RES_URI,
-                                                                                        Ldpr.RETURN_SRV_MGD_RES_URI)
+            'prefer': (
+                'return=representation; include="{0} {1}"; omit="{2}"'.format(
+                    Ldpr.RETURN_CHILD_RES_URI,
+                    Ldpr.EMBED_CHILD_RES_URI,
+                    Ldpr.RETURN_SRV_MGD_RES_URI
+                )
+            )
         })
         assert resp1.status_code == 200
         self._assert_pref_applied(resp1, include=[Ldpr.RETURN_CHILD_RES_URI, Ldpr.EMBED_CHILD_RES_URI],
                                   omit=[Ldpr.RETURN_SRV_MGD_RES_URI])
 
+    def test_invalid_preference(self):
+        """
+        Test to ensure Prefer headers with unknown include/omit URIs are completely disregarded.
+        """
+        fake_preference = 'http://doesntexist.org/fake#preference'
+
+        self.client.put('/ldp/test_invalid_preference01', content_type='text/turtle')
+
+        resp1 = self.client.get('/ldp/test_invalid_preference01', headers={
+            'prefer': (
+                'return=representation; include="{0}"'.format(fake_preference)
+            )
+        })
+        assert resp1.status_code == 200
+        self._assert_pref_applied(resp1)
+
+        resp2 = self.client.get('/ldp/test_invalid_preference01', headers={
+            'prefer': 'return=representation; omit="{0}"'.format(fake_preference)
+        })
+        assert resp2.status_code == 200
+        self._assert_pref_applied(resp2)
+
+        resp3 = self.client.get('/ldp/test_invalid_preference01', headers={
+            'prefer': 'return=representation; include="{0} {1}"'.format(fake_preference, Ldpr.EMBED_CHILD_RES_URI)
+        })
+        assert resp3.status_code == 200
+        self._assert_pref_applied(resp3)
+
+        resp4 = self.client.get('/ldp/test_invalid_preference01', headers={
+            'prefer': 'return=representation; omit="{0} {1}"'.format(fake_preference, Ldpr.EMBED_CHILD_RES_URI)
+        })
+        assert resp4.status_code == 200
+        self._assert_pref_applied(resp4)
+
+        resp4 = self.client.get('/ldp/test_invalid_preference01', headers={
+            'prefer': (
+                'return=representation; include="{0}" omit="{1} {2}"'.format(
+                    fake_preference,
+                    Ldpr.EMBED_CHILD_RES_URI,
+                    Ldpr.RETURN_SRV_MGD_RES_URI
+                )
+            )
+        })
+        assert resp4.status_code == 200
+        self._assert_pref_applied(resp4)
+
+        resp5 = self.client.get('/ldp/test_invalid_preference01', headers={
+            'prefer': (
+                'return=representation; include="{0} {1}" omit="{2}"'.format(
+                    fake_preference,
+                    Ldpr.EMBED_CHILD_RES_URI,
+                    Ldpr.RETURN_SRV_MGD_RES_URI
+                )
+            )
+        })
+        assert resp5.status_code == 200
+        self._assert_pref_applied(resp5)
+
+    def test_return_minimal(self):
+        """
+        Test for Prefer: return=minimal
+        """
+        self.client.put('/ldp/test_return_minimal01', content_type='text/turtle')
+
+        resp1 = self.client.get('/ldp/test_return_minimal01', headers={
+            'prefer': 'return=minimal'
+        })
+        assert resp1.status_code == 200
+        self._assert_pref_applied(resp1, 'minimal')
+
+        resp2 = self.client.get('/ldp/test_return_minimal01', headers={
+            'prefer': 'return="minimal"'
+        })
+        assert resp2.status_code == 200
+        self._assert_pref_applied(resp2, 'minimal')
+
     def _assert_pref_applied(self, response, value='representation', include=None, omit=None):
         """
         Utility to test a response for a Preference-Applied header with the include or omit lists.
+
+        If both include and omit are empty and value is representation, it is expected that there is NO
+        Preference-Applied header.
+
         :param response:: The client response.
-        :param value:: The return=<value>.
-        :param include:: The list of include URIs.
-        :param omit:: The list of omit URIs.
-        :return:
+        :param string value:: The return=<value>.
+        :param list include:: Expected include URIs.
+        :param list omit:: Expected omit URIs.
         """
-        if include is None and omit is None:
+        if include is None and omit is None and value == 'representation':
             assert 'Preference-Applied' not in response.headers
         else:
             if include is None:
@@ -1941,10 +2032,9 @@ class TestPrefHeader:
     def _assert_pref_header_exists(self, expected, returned, type='include'):
         """
         Utility function to compare a list of expected preferences to an include or omit string.
-        :param self:
-        :param expected:
-        :param returned:
-        :return:
+
+        :param list expected:: List of expected preference URIs.
+        :param string returned:: Returned Prefer parameters
         """
         if len(expected) > 0:
             expected = [str(k) for k in expected]