From e32afcad28b4661b465bab2df35ca30fa6cb2bf1 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 09:05:39 +0000 Subject: [PATCH 1/7] Test that requested properties beside a resource key are sent, with or without a licence key --- .../tests/test_credentials.py | 56 ++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py index a5a2b8ab..ebefd193 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py @@ -181,23 +181,39 @@ def test_licence_key_without_properties_is_refused(self): self.assertIn("400", str(context.exception)) self.assertIn("requested_properties", str(context.exception)) - def test_properties_with_resource_key_are_refused(self): + def test_properties_with_resource_key_are_sent(self): """ - Row five of the table, with and without a licence key beside the - resource key. The service ignores the list, so the engine refuses - to build and says the response would not narrow. + A list beside a resource key, with and without a licence key. The + service adds the listed properties to those the resource key + carries, so the engine builds and sends the list as values, with + the licence key in the body where there is one. """ for license_key in ("", LICENSE_KEY): - with self.assertRaises(ValueError) as context: - CloudRequestEngine({ - "resource_key": RESOURCE_KEY, - "license_key": license_key, - "requested_properties": REQUESTED, - "http_client": self.mock_http(), - }) + engine = CloudRequestEngine({ + "resource_key": RESOURCE_KEY, + "license_key": license_key, + "requested_properties": REQUESTED, + "http_client": self.mock_http(), + }) + + requests = self.process(engine) - self.assertIn("ignores the list", str(context.exception)) + for request_type, url, content in requests: + self.assertNotIn(LICENSE_KEY, url) + + request_type, url, content = self.data_request(requests) + self.assertTrue(url.endswith(RESOURCE_KEY + ".json?"), url) + self.assertEqual( + RESOURCE_KEY, content[Constants.RESOURCE_PARAMETER]) + self.assertEqual( + "device.ismobile,device.istablet", + content[Constants.VALUES_PARAMETER]) + if license_key == "": + self.assertNotIn(Constants.LICENSE_PARAMETER, content) + else: + self.assertEqual( + LICENSE_KEY, content[Constants.LICENSE_PARAMETER]) def test_no_credential_is_refused(self): """ @@ -302,14 +318,13 @@ def test_reserved_evidence_is_left_out_of_the_body(self): for message in messages: self.assertNotIn("another-key", message) - def test_properties_added_later_are_normalised_and_checked(self): + def test_properties_added_later_are_normalised_and_sent(self): """ A caller may add to the list after the engine is built, once the accessible properties say which client-side properties the key carries. The names added are trimmed and lower-cased as the - constructor's are, so they match the answer, and the rule that a - list only goes with a licence key alone is checked again, because - the constructor cannot see a list added after it ran. + constructor's are, so they match the answer, and they are sent + beside a resource key as well as beside a licence key alone. """ engine = CloudRequestEngine({ @@ -336,10 +351,13 @@ def test_properties_added_later_are_normalised_and_checked(self): "http_client": self.mock_http(), }) - with self.assertRaises(ValueError) as context: - engine.add_requested_properties(REQUESTED) + engine.add_requested_properties(REQUESTED) - self.assertIn("ignores the list", str(context.exception)) + request_type, url, content = self.data_request( + self.process(engine)) + self.assertEqual( + "device.ismobile,device.istablet", + content[Constants.VALUES_PARAMETER]) def test_properties_the_answer_leaves_out_are_reported_once(self): """ From a7d976a49ed4d814a6a34fde7d0a06b63ff78bfe Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 09:05:44 +0000 Subject: [PATCH 2/7] Send requested properties beside a resource key, with or without a licence key The engine refused requested_properties whenever a resource key was set, because the cloud service was thought to ignore the list. The service adds the listed properties to those the resource key carries, so the engine now builds and sends the list as values, and the licence key too where there is one. It still refuses no key at all, and a licence key with no resource key or list, which the service refuses on every request. The message for properties an answer leaves out now says they are not covered by the keys in use, as a resource key can be the only one. --- .../fiftyone_pipeline_cloudrequestengine/readme.md | 7 ++++--- .../cloudrequestengine.py | 14 +++----------- .../constants.py | 13 +++---------- 3 files changed, 10 insertions(+), 24 deletions(-) diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md b/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md index 5979c0e8..eb6260c4 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md @@ -29,10 +29,11 @@ property names to ask for, is also needed: | `resource_key` | `license_key` | `requested_properties` | Accepted | | --- | --- | --- | --- | | Yes | No | No | Yes. The resource key states which properties it carries. | -| Yes | Yes | No | Yes. The licence key adds the products it grants. | +| Yes | Yes | No | Yes. The answer holds what the resource key carries. | +| Yes | Yes | Yes | Yes. The service adds the listed properties the licence key grants. | +| Yes | No | Yes | Yes. The service adds the listed properties, as null with a reason where no licence grants one. | | No | Yes | Yes | Yes. A licence key names no properties, so the list says which are wanted. | | No | Yes | No | No. The service answers 400 to every such request. | -| Yes | Either | Yes | No. The service ignores the list while a resource key is present. | | No | No | Either | No. There is nothing to authenticate with. | A combination that is not accepted raises `ValueError` when the engine is @@ -47,7 +48,7 @@ key and name the properties it wants, so the credential stays on the server and each answer carries only what that caller needs. A licence key is sent in the body of every request, never in a URL. Where a request named properties and the answer left some out, the engine warns once that -they are not covered by the licence key; the properties that came back +they are not covered by the keys in use, and the properties that came back are usable as normal. See the [Credentials section of the cloud request engine specification](https://github.com/51Degrees/specifications/blob/main/pipeline-specification/pipeline-elements/cloud-request-engine.md#credentials) diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py index 42cd6be8..e07b7dee 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py @@ -57,9 +57,8 @@ def __init__(self, settings = {}): license_key, and optionally 1) requested_properties, the fully qualified names of the properties to ask for, as a list or a comma separated string. - Required with a license_key alone and refused with a resource_key, - because the cloud service ignores the list whenever a resource key - is present + Required with a license_key alone. Beside a resource_key the cloud + service adds them to the properties the resource key carries 2) a cloud_endpoint to overwrite the default baseurl 3) an cloud_request_origin to use when making requests @@ -178,9 +177,7 @@ def add_requested_properties(self, names): Add to the properties each request asks for, after the engine is built. The names are trimmed and lower-cased as the constructor does, so a name given in the case the documentation uses still - matches the answer, and the credential rules are checked again, - because a list only has meaning on a licence key alone and the - constructor's check cannot see a list added later. + matches the answer. @type names: list or string @param names: the names to add, as a list or one comma separated @@ -189,7 +186,6 @@ def add_requested_properties(self, names): self.requested_properties.extend( self.setting_names({"names": names}, "names")) - self.check_credentials() def check_credentials(self): @@ -210,10 +206,6 @@ def check_credentials(self): if has_resource_key == False and has_requested_properties == False: raise ValueError(Constants.MESSAGE_LICENSE_KEY_NEEDS_PROPERTIES) - if has_resource_key == True and has_requested_properties == True: - raise ValueError( - Constants.MESSAGE_PROPERTIES_IGNORED_WITH_RESOURCE_KEY) - def get_credentials(self): """! diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py index 8014286d..7aa25d6b 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py @@ -108,18 +108,11 @@ class Constants: "properties it wants. Set requested_properties to the fully " "qualified names wanted, for example " "['device.ismobile', 'device.iscrawler'].") - MESSAGE_PROPERTIES_IGNORED_WITH_RESOURCE_KEY = ( - "CloudRequestEngine has requested_properties and a resource key. " - "The cloud service ignores the list whenever a resource key is " - "present and answers with everything the resource key carries, so " - "the caller would believe the response had narrowed when it had " - "not. Either leave out requested_properties, or authenticate with " - "a licence key alone.") # Logged once when a request named properties and the response left # some of them out without saying so. MESSAGE_PROPERTIES_NOT_COVERED = ( "The cloud service did not return the requested properties {} in " - "its answer. They are not covered by the licence key in use, so " - "they will be missing from every result. The other requested " - "properties came back and are usable.") + "its answer. They are not covered by the keys in use, so they will " + "be missing from every result. The other requested properties came " + "back and are usable.") From e520db1d88c6f7c3fe10d31117d8f569685b1abf Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 17:48:43 +0000 Subject: [PATCH 3/7] Test that a name left out beside a resource key alone is reported as not a property --- .../tests/test_credentials.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py index eedc6c6a..6e24fcad 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py @@ -423,6 +423,38 @@ def test_properties_the_answer_leaves_out_are_reported_once(self): self.assertNotIn("device.istablet", messages[0]) self.assertNotIn("device.ismobile", messages[0]) + def test_a_name_left_out_beside_a_resource_key_alone_is_not_a_property( + self): + """ + With a resource key, a list and no licence key, a property no + licence grants comes back as null with its nullreason, so a name the + answer leaves out is not a property at all. The engine says so, + once, and does not name a licence key that was never given. + """ + + engine = CloudRequestEngine({ + "resource_key": RESOURCE_KEY, + "requested_properties": ["device.value", "device.notaproperty"], + "http_client": self.mock_http(), + }) + + pipeline = PipelineBuilder().add(engine).build() + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + for _ in range(2): + data = pipeline.create_flowdata() + data.evidence.add("query.User-Agent", "iPhone") + data.process() + + messages = [ + str(warning.message) for warning in caught + if "device.notaproperty" in str(warning.message)] + self.assertEqual(1, len(messages), messages) + self.assertIn("not properties the cloud service knows", messages[0]) + self.assertNotIn("licence key", messages[0]) + self.assertNotIn("device.value", messages[0]) + def test_no_report_when_every_property_came_back(self): """ An answer carrying every requested property raises no warning, and From 559f7908bc9436f95fbb423cd97ebe05ba0e2358 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 17:48:44 +0000 Subject: [PATCH 4/7] Report a name left out beside a resource key alone as not a property, and drop history from two comments --- .../readme.md | 6 +++-- .../cloudrequestengine.py | 24 +++++++++++-------- .../constants.py | 19 +++++++++++---- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md b/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md index eb6260c4..4270a72b 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md @@ -48,8 +48,10 @@ key and name the properties it wants, so the credential stays on the server and each answer carries only what that caller needs. A licence key is sent in the body of every request, never in a URL. Where a request named properties and the answer left some out, the engine warns once that -they are not covered by the keys in use, and the properties that came back -are usable as normal. +they are not covered by the licence key, and the properties that came back +are usable as normal. With a resource key and no licence key, a property no +licence grants comes back as null with a reason, so the warning says instead +that a name left out is not a property the cloud service knows. See the [Credentials section of the cloud request engine specification](https://github.com/51Degrees/specifications/blob/main/pipeline-specification/pipeline-elements/cloud-request-engine.md#credentials) and the rules in diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py index e07b7dee..1279a44b 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py @@ -266,7 +266,7 @@ def get_engine_properties(self): """ # Get properties for all engines. A resource key alone goes in the - # query string as it always has. A licence key identifies an + # query string. A licence key identifies an # account, so it is posted in the body instead, where it stays out # of request logs. The cloud service reads both keys from either # place, answers a licence key alone with everything the licence @@ -404,7 +404,7 @@ def process_internal(self, flowdata): """ - # A resource key names the route, as it always has. A licence key + # A resource key names the route. A licence key # alone has no resource key to put there, so it uses the data # endpoint that takes none and travels in the body with the list # of properties wanted. @@ -436,13 +436,16 @@ def report_properties_not_covered(self, result): Compare the properties a request asked for with the ones its answer carries, and warn once about any it left out. - The cloud service drops a property the licence key does not cover - without naming it, whenever the request also asked for a property - the key does cover, so the engine has to notice the difference - itself. A property that is present but null counts as answered, + With a licence key, the cloud service drops a property the licence + key does not cover without naming it, whenever the request also + asked for a property the key does cover, so the engine has to + notice the difference itself. With a resource key and no licence + key, a property no licence grants comes back as null with a reason, + so a name left out is not a property, and the warning says that + instead. A property that is present but null counts as answered, because it carries a nullreason beside it saying why it has no value on this request, which is a different matter from - entitlement. The answer cannot change while the key and the list + entitlement. The answer cannot change while the keys and the list stay the same, so each distinct list is reported once. @type result: string @@ -472,9 +475,10 @@ def report_properties_not_covered(self, result): self.reported_not_covered.add(requested) if len(not_covered) > 0: - warnings.warn( - Constants.MESSAGE_PROPERTIES_NOT_COVERED.format( - ", ".join(not_covered))) + message = (Constants.MESSAGE_PROPERTIES_NOT_COVERED + if self.license_key != "" + else Constants.MESSAGE_PROPERTIES_NOT_KNOWN) + warnings.warn(message.format(", ".join(not_covered))) def get_content(self, flowData): diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py index 7aa25d6b..e2b94691 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py @@ -109,10 +109,19 @@ class Constants: "qualified names wanted, for example " "['device.ismobile', 'device.iscrawler'].") - # Logged once when a request named properties and the response left - # some of them out without saying so. + # Logged once when a request with a licence key named properties and + # the response left some of them out without saying so. MESSAGE_PROPERTIES_NOT_COVERED = ( "The cloud service did not return the requested properties {} in " - "its answer. They are not covered by the keys in use, so they will " - "be missing from every result. The other requested properties came " - "back and are usable.") + "its answer. They are not covered by the licence key in use, so " + "they will be missing from every result. The other requested " + "properties came back and are usable.") + # Logged once when a request with a resource key and no licence key + # named properties and the response left some of them out. Without a + # licence key a property no licence grants comes back as null with a + # reason, so a name left out is not a property. + MESSAGE_PROPERTIES_NOT_KNOWN = ( + "The cloud service did not return the requested properties {} in " + "its answer. They are not properties the cloud service knows, so " + "check the names. The other requested properties came back and " + "are usable.") From cb86932bdcfb9f8f7d5a6e9cb04601b1ba8cc1cb Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 07:27:49 +0000 Subject: [PATCH 5/7] Say the keys in use in the properties not covered warning, and drop a stale claim about discovery .NET, Java, Node.js and PHP all word the warning as not covered by the keys in use, because the licence behind a resource key and a licence key beside it both decide what comes back. The name not known warning gains the same closing line and the spelling advice those languages give. The discovery comment no longer says a licence key beside a resource key widens the answer, which the corrected Accepted combinations table replaced. Tests assert both wordings. --- .../fiftyone_pipeline_cloudrequestengine/readme.md | 2 +- .../cloudrequestengine.py | 10 ++++------ .../constants.py | 13 ++++++++----- .../tests/test_credentials.py | 9 ++++++--- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md b/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md index 20d048e7..598adf49 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md @@ -48,7 +48,7 @@ key and name the properties it wants, so the credential stays on the server and each answer carries only what that caller needs. A licence key is sent in the body of every request, never in a URL. Where a request named properties and the answer left some out, the engine warns once that -they are not covered by the licence key, and the properties that came back +they are not covered by the keys in use, and the properties that came back are usable as normal. With a resource key and no licence key, a property no licence grants comes back as null with a reason, so the warning says instead that a name left out is not a property the cloud service knows. diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py index b6daa2de..03f93125 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py @@ -269,12 +269,10 @@ def get_engine_properties(self): """ # Get properties for all engines. A resource key alone goes in the - # query string. A licence key identifies an - # account, so it is posted in the body instead, where it stays out - # of request logs. The cloud service reads both keys from either - # place, answers a licence key alone with everything the licence - # entitles, and widens a resource key's answer by the products a - # licence key sent beside it grants. + # query string. A licence key identifies an account, so it is + # posted in the body instead, where it stays out of request logs. + # The cloud service reads both keys from either place, and answers + # a licence key alone with every property the licence entitles. propertiesURL = self.baseURL + "accessibleProperties" diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py index 332e1326..31a1a118 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py @@ -117,11 +117,13 @@ class Constants: "['device.ismobile', 'device.iscrawler'].") # Logged once when a request with a licence key named properties and - # the response left some of them out without saying so. + # the response left some of them out without saying so. Both the + # licence behind a resource key and a licence key beside it decide + # what comes back, so the message names the keys rather than one key. MESSAGE_PROPERTIES_NOT_COVERED = ( "The cloud service did not return the requested properties {} in " - "its answer. They are not covered by the licence key in use, so " - "they will be missing from every result. The other requested " + "its answer. They are not covered by the keys in use, so they " + "will be missing from every result. The other requested " "properties came back and are usable.") # Logged once when a request with a resource key and no licence key # named properties and the response left some of them out. Without a @@ -130,5 +132,6 @@ class Constants: MESSAGE_PROPERTIES_NOT_KNOWN = ( "The cloud service did not return the requested properties {} in " "its answer. They are not properties the cloud service knows, so " - "check the names. The other requested properties came back and " - "are usable.") + "they will be missing from every result. Check the spelling of " + "each one. The other requested properties came back and are " + "usable.") diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py index de82e522..e7a4c462 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py @@ -90,9 +90,10 @@ def test_resource_key_alone_is_accepted(self): def test_resource_key_with_licence_key_is_accepted(self): """ - Row two of the table. The licence key adds the products it grants, - so it travels in the body of the data request and of the accessible - properties request, and no list of properties is sent. + Row two of the table. With no list the answer holds what the + resource key carries and the licence key adds no property, so the + licence key travels in the body of the data request and of the + accessible properties request, and no list of properties is sent. """ engine = CloudRequestEngine({ @@ -501,6 +502,7 @@ def test_properties_the_answer_leaves_out_are_reported_once(self): str(warning.message) for warning in caught if "not covered" in str(warning.message)] self.assertEqual(1, len(messages), messages) + self.assertIn("not covered by the keys in use", messages[0]) self.assertIn("device.iscrawler", messages[0]) self.assertNotIn("device.istablet", messages[0]) self.assertNotIn("device.ismobile", messages[0]) @@ -534,6 +536,7 @@ def test_a_name_left_out_beside_a_resource_key_alone_is_not_a_property( if "device.notaproperty" in str(warning.message)] self.assertEqual(1, len(messages), messages) self.assertIn("not properties the cloud service knows", messages[0]) + self.assertIn("Check the spelling of each one", messages[0]) self.assertNotIn("licence key", messages[0]) self.assertNotIn("device.value", messages[0]) From 9abbec311eee09c834c2e6311a008158f02363b3 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 09:15:45 +0000 Subject: [PATCH 6/7] Test an answer that is not an object, and the message .NET and Java write The report compares the answer with the list it asked for, and an answer that parses as JSON but is not an object has nothing to compare, so the test pins that the request still succeeds and nothing is written. The warning for a name that is not a property is asserted as .NET and Java word it, both of them carrying the same sentence. --- .../tests/test_credentials.py | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py index e7a4c462..e29c2e64 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py @@ -535,11 +535,40 @@ def test_a_name_left_out_beside_a_resource_key_alone_is_not_a_property( str(warning.message) for warning in caught if "device.notaproperty" in str(warning.message)] self.assertEqual(1, len(messages), messages) - self.assertIn("not properties the cloud service knows", messages[0]) + self.assertIn( + "Every property is answered, with a reason when it has no " + "value, so these names are not properties", messages[0]) self.assertIn("Check the spelling of each one", messages[0]) self.assertNotIn("licence key", messages[0]) self.assertNotIn("device.value", messages[0]) + def test_an_answer_that_is_not_an_object_does_not_fail_the_request(self): + """ + The report compares the answer with the list, and an answer that + parses as JSON but is not an object has nothing to compare. It + must not fail the request, because the request itself succeeded, + so the engine leaves the answer alone and says nothing. + """ + + engine = CloudRequestEngine({ + "license_key": LICENSE_KEY, + "requested_properties": REQUESTED, + "http_client": self.mock_http( + license_json_response='["device", "ismobile"]'), + }) + + pipeline = PipelineBuilder().add(engine).build() + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + data = pipeline.create_flowdata() + data.evidence.add("query.User-Agent", "iPhone") + data.process() + + self.assertEqual( + [], [str(warning.message) for warning in caught + if "did not return" in str(warning.message)]) + def test_no_report_when_every_property_came_back(self): """ An answer carrying every requested property raises no warning, and From 039c1282fcb51e8b17484b4b64364fa31baf3a00 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 09:16:28 +0000 Subject: [PATCH 7/7] Guard the report against an answer that is not an object, and take .NET and Java's wording An answer that parses as JSON but is not an object has no sections to look in, so the report returned an AttributeError out of processing and failed a request that had succeeded. It now leaves such an answer alone, with the same guard and the same wider catch that is_cacheable uses on the same text. The warning for a name that is not a property is now word for word the sentence .NET and Java both write, so Python's two warnings match theirs exactly. --- pipeline/fiftyone_pipeline_cloudrequestengine/readme.md | 3 ++- .../cloudrequestengine.py | 8 +++++++- .../src/fiftyone_pipeline_cloudrequestengine/constants.py | 6 +++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md b/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md index 598adf49..de740e3b 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md @@ -51,7 +51,8 @@ named properties and the answer left some out, the engine warns once that they are not covered by the keys in use, and the properties that came back are usable as normal. With a resource key and no licence key, a property no licence grants comes back as null with a reason, so the warning says instead -that a name left out is not a property the cloud service knows. +that every property is answered and these names are not properties, and +asks the caller to check the spelling of each one. See the [Credentials section of the cloud request engine specification](https://github.com/51Degrees/specifications/blob/main/pipeline-specification/pipeline-elements/cloud-request-engine.md#credentials) and the rules in diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py index 03f93125..5dfc6bc0 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py @@ -500,11 +500,17 @@ def report_properties_not_covered(self, result): try: answer = json.loads(result) - except JSONDecodeError: + except (AttributeError, JSONDecodeError, TypeError, ValueError): # validate_response has already dealt with an answer that is # not JSON, so there is nothing to compare here. return + # An answer that parses but is not an object, such as an array, + # has no sections to look in. Reporting must not fail the request, + # which succeeded, so there is nothing to say. + if isinstance(answer, dict) == False: + return + not_covered = [] for name in sorted(requested): section, _, property_name = name.partition( diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py index 31a1a118..60bd4d24 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py @@ -131,7 +131,7 @@ class Constants: # reason, so a name left out is not a property. MESSAGE_PROPERTIES_NOT_KNOWN = ( "The cloud service did not return the requested properties {} in " - "its answer. They are not properties the cloud service knows, so " - "they will be missing from every result. Check the spelling of " - "each one. The other requested properties came back and are " + "its answer. Every property is answered, with a reason when it has " + "no value, so these names are not properties. Check the spelling " + "of each one. The other requested properties came back and are " "usable.")