diff --git a/location/src/fiftyone_location/location_cloud.py b/location/src/fiftyone_location/location_cloud.py index ec25062d..596a1b21 100644 --- a/location/src/fiftyone_location/location_cloud.py +++ b/location/src/fiftyone_location/location_cloud.py @@ -44,12 +44,3 @@ def __init__(self, location_provider="fiftyonedegrees"): self.datakey = "location_digitalelement" else: raise Exception("The location provider " + location_provider + " was not recognized.") - - def on_registration(self, pipeline): - available_properties = pipeline.flow_elements_list.get( - "cloud", {}).flow_element_properties - if "location" not in available_properties: - raise Exception("Location data was not available. " - "Check that this key is authorised " - "for geolocation data") - return super().on_registration(pipeline) diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudengine.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudengine.py index 79bb5007..886591e0 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudengine.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudengine.py @@ -82,11 +82,15 @@ def on_registration(self, pipeline): # The accessible properties come from the cloud service for the # key in use, a resource key or a licence key, so a section missing - # from them means that key does not carry the product at all. - if not self.datakey in pipeline.flow_elements_list["cloud"].flow_element_properties: - raise Exception("The key in use does not include access to any properties under the engine with key " + self.datakey + " that was added to the pipeline. For more details on resource keys, see our explainer: https://51degrees.com/documentation/_services__cloud__resource_keys.html?utm_source=code&utm_medium=comment&utm_campaign=51degrees-python&utm_content=fiftyone_pipeline_cloudrequestengine-src-fiftyone_pipeline_cloudrequestengine-cloudengine.py&utm_term=engine-properties-not-accessible " + "Available engine data keys are: " + str([e for e in pipeline.flow_elements_list["cloud"].flow_element_properties])) - - self.properties = pipeline.flow_elements_list["cloud"].flow_element_properties[self.datakey] + # from them means that key does not carry the product at all. That + # is not a configuration error, so the engine is built with no + # properties and the missing product is logged here, once. + carried = pipeline.flow_elements_list["cloud"].flow_element_properties + if not self.datakey in carried: + pipeline.log("warning", "The key in use does not include access to any properties under the engine with key " + self.datakey + " that was added to the pipeline. For more details on resource keys, see our explainer: https://51degrees.com/documentation/_services__cloud__resource_keys.html?utm_source=code&utm_medium=comment&utm_campaign=51degrees-python&utm_content=fiftyone_pipeline_cloudrequestengine-src-fiftyone_pipeline_cloudrequestengine-cloudengine.py&utm_term=engine-properties-not-accessible " + "Available engine data keys are: " + str([e for e in carried])) + self.properties = {} + else: + self.properties = carried[self.datakey] # Add a special message to the pipeline # when requesting an element that does not exist diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudengine_missing_product.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudengine_missing_product.py new file mode 100644 index 00000000..8604ea23 --- /dev/null +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudengine_missing_product.py @@ -0,0 +1,97 @@ + +""" +A Cloud Aspect Engine whose product the key does not include builds with +empty Property metadata, logs the missing product once, and logs nothing +about it on each response, as the Cloud Aspect Engine specification says. +""" + +import json +import unittest + +from fiftyone_pipeline_cloudrequestengine.cloudengine import CloudEngine +from fiftyone_pipeline_core.elementdata_dictionary import ElementDataDictionary +from fiftyone_pipeline_core.flowelement import FlowElement +from fiftyone_pipeline_core.logger import Logger +from fiftyone_pipeline_core.pipelinebuilder import PipelineBuilder + + +class RecordingLogger(Logger): + + def __init__(self): + super(RecordingLogger, self).__init__("debug") + self.logged = [] + + def log_internal(self, level, log): + self.logged.append((level, log["message"])) + + +class FakeCloudRequestEngine(FlowElement): + """Stands in for the Cloud Request Engine of a key that carries only the + device product.""" + + def __init__(self): + super(FakeCloudRequestEngine, self).__init__() + self.datakey = "cloud" + self.flow_element_properties = { + "device": {"ismobile": {"name": "IsMobile", "type": "Boolean"}}} + + def process_internal(self, flowdata): + flowdata.set_element_data(ElementDataDictionary( + self, {"cloud": json.dumps({"device": {"ismobile": True}})})) + + +def location_engine(): + engine = CloudEngine() + engine.datakey = "location" + return engine + + +class MissingProductTests(unittest.TestCase): + + def build(self): + logger = RecordingLogger() + builder = PipelineBuilder({"add_javascript_builder": False, + "use_setheader_properties": False}) + builder.add_logger(logger) + engine = location_engine() + pipeline = builder.add(FakeCloudRequestEngine()).add(engine).build() + self.addCleanup(pipeline.close) + return pipeline, engine, logger + + def testPipelineBuildsWithEmptyMetadata(self): + pipeline, engine, logger = self.build() + self.assertEqual({}, engine.get_properties()) + + def testMissingProductIsLoggedOnce(self): + pipeline, engine, logger = self.build() + missing = [entry for entry in logger.logged + if "location" in entry[1]] + self.assertEqual(1, len(missing), logger.logged) + # The specification asks for the warning level. + self.assertEqual("warning", missing[0][0]) + + def testNothingIsLoggedOnEachResponse(self): + pipeline, engine, logger = self.build() + before = len(logger.logged) + for _ in range(3): + flowdata = pipeline.create_flowdata() + flowdata.process() + self.assertEqual(0, len(flowdata.errors)) + self.assertEqual({}, flowdata.get("location").as_dictionary()) + self.assertEqual(before, len(logger.logged), logger.logged) + + def testEngineWithItsProductStillBuildsWithItsProperties(self): + logger = RecordingLogger() + builder = PipelineBuilder({"add_javascript_builder": False, + "use_setheader_properties": False}) + builder.add_logger(logger) + engine = CloudEngine() + engine.datakey = "device" + pipeline = builder.add(FakeCloudRequestEngine()).add(engine).build() + self.addCleanup(pipeline.close) + self.assertEqual(["ismobile"], list(engine.get_properties())) + self.assertEqual([], logger.logged) + + +if __name__ == "__main__": + unittest.main()