From f976a0c0499b7dec87400d49c61312d60561363b Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 5 Sep 2026 01:38:20 +0100 Subject: [PATCH] FIX: Let cloud examples run without the on-premise engine, report why a property has no value, and make a broken example go red Four of the eight cloud examples imported fiftyone_devicedetection.devicedetection_pipelinebuilder, which unconditionally imports the on-premise engine, so a cloud only example could not be run without building the on-premise native library from source. PyPI publishes that package as an sdist with no wheel, so anyone following a cloud example needed a C++ toolchain first. The four now use DeviceDetectionCloudPipelineBuilder, which does exactly the same work on the cloud path, and one of them imported the builder without ever using it. A new test imports every cloud example in a separate process with the on-premise packages blocked, so the dependency cannot come back. The hardware profile cloud engine threw away the reason the cloud service gives when a resource key is not entitled to a property, so the TAC and native model examples stopped with AttributeError: 'NoneType' object has no attribute 'no_value_message'. The engine now carries each aspect level value and its companion nullreason into every profile, and get_human_readable reports the reason instead of calling a method on None. A new unit test runs the engine against fixed cloud responses, so it needs no resource key and no network connection, and the cloud package is added to the unit job so it runs on every build. The cloud example tests asserted nothing about what the examples printed. They now capture the output and assert the headings each example must print, that nothing in it reads as a programming fault, and that each lookup says something about the devices it found. Tests that cannot run without a resource key now skip with a message naming the variable they wanted, rather than returning quietly and passing. The property coverage tests skip when the CSV asset is absent rather than failing to import. Resource key environment variables move onto the '_51DEGREES_RESOURCE_KEY' convention, with the previous names still read as a fallback so an existing setup keeps working. --- .github/workflows/nightly-pipeline.yml | 4 +- .github/workflows/publish.yml | 2 +- .github/workflows/pull-requests.yml | 2 +- ci/run-integration-tests.ps1 | 8 +- ci/run-unit-tests.ps1 | 6 +- .../hardwareprofile_cloud.py | 62 +++++- .../tests/test_cloud.py | 53 ++++-- .../tests/test_hardwareprofile_cloud.py | 176 ++++++++++++++++++ .../tests/test_properties.py | 40 +++- fiftyone_devicedetection_cloud/tox.ini | 1 + fiftyone_devicedetection_examples/setup.py | 6 +- .../cloud/configurator_console.py | 12 +- .../cloud/gettingstarted_console.py | 3 +- .../cloud/gettingstarted_web/app.py | 8 +- .../cloud/metadata_console.py | 6 +- .../cloud/nativemodellookup_console.py | 8 +- .../cloud/taclookup_console.py | 8 +- .../example_utils.py | 96 +++++++++- .../tests/test_cloudexampleimports.py | 106 +++++++++++ .../tests/test_cloudexamples.py | 103 +++++++++- fiftyone_devicedetection_examples/tox.ini | 1 + fiftyone_devicedetection_onpremise/tox.ini | 1 + 22 files changed, 646 insertions(+), 66 deletions(-) create mode 100644 fiftyone_devicedetection_cloud/tests/test_hardwareprofile_cloud.py create mode 100644 fiftyone_devicedetection_examples/tests/test_cloudexampleimports.py diff --git a/.github/workflows/nightly-pipeline.yml b/.github/workflows/nightly-pipeline.yml index 779c25b41..cfa6e53d6 100644 --- a/.github/workflows/nightly-pipeline.yml +++ b/.github/workflows/nightly-pipeline.yml @@ -33,7 +33,7 @@ jobs: secrets: token: ${{ secrets.ACCESS_TOKEN }} DeviceDetection: ${{ secrets.DEVICE_DETECTION_KEY }} - TestResourceKey: ${{ secrets.SUPER_RESOURCE_KEY}} + TestResourceKey: ${{ secrets._51DEGREES_RESOURCE_KEY_SUPER || secrets.SUPER_RESOURCE_KEY }} DeviceDetectionUrl: ${{ secrets.DEVICE_DETECTION_URL }} CsvUrl: ${{ secrets.CSV_URL }} @@ -50,7 +50,7 @@ jobs: secrets: token: ${{ secrets.ACCESS_TOKEN }} DeviceDetection: ${{ secrets.DEVICE_DETECTION_KEY }} - TestResourceKey: ${{ secrets.SUPER_RESOURCE_KEY}} + TestResourceKey: ${{ secrets._51DEGREES_RESOURCE_KEY_SUPER || secrets.SUPER_RESOURCE_KEY }} DeviceDetectionUrl: ${{ secrets.DEVICE_DETECTION_URL }} CsvUrl: ${{ secrets.CSV_URL }} PypiToken: ${{ secrets.PYPI_TOKEN }} diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 52df476dc..a6eac8dae 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -19,7 +19,7 @@ jobs: secrets: token: ${{ secrets.ACCESS_TOKEN }} DeviceDetection: ${{ secrets.DEVICE_DETECTION_KEY }} - TestResourceKey: ${{ secrets.SUPER_RESOURCE_KEY}} + TestResourceKey: ${{ secrets._51DEGREES_RESOURCE_KEY_SUPER || secrets.SUPER_RESOURCE_KEY }} DeviceDetectionUrl: ${{ secrets.DEVICE_DETECTION_URL }} CsvUrl: ${{ secrets.CSV_URL }} PypiToken: ${{ secrets.PYPI_TOKEN }} diff --git a/.github/workflows/pull-requests.yml b/.github/workflows/pull-requests.yml index c9960eed3..b26a365b2 100644 --- a/.github/workflows/pull-requests.yml +++ b/.github/workflows/pull-requests.yml @@ -19,6 +19,6 @@ jobs: secrets: token: ${{ secrets.ACCESS_TOKEN }} DeviceDetection: ${{ secrets.DEVICE_DETECTION_KEY }} - TestResourceKey: ${{ secrets.SUPER_RESOURCE_KEY}} + TestResourceKey: ${{ secrets._51DEGREES_RESOURCE_KEY_SUPER || secrets.SUPER_RESOURCE_KEY }} DeviceDetectionUrl: ${{ secrets.DEVICE_DETECTION_URL }} CsvUrl: ${{ secrets.CSV_URL }} diff --git a/ci/run-integration-tests.ps1 b/ci/run-integration-tests.ps1 index 45e618633..b9bca8dc9 100644 --- a/ci/run-integration-tests.ps1 +++ b/ci/run-integration-tests.ps1 @@ -29,6 +29,12 @@ if ($env:GITHUB_JOB -eq "Test") { pip install $RepoName/fiftyone_devicedetection_examples || $(throw "pip install failed") } +# Resource key environment variables follow the 51Degrees convention, which +# is that every one of them starts with '_51DEGREES_RESOURCE_KEY'. The name +# used before the convention was adopted is set by common-ci and is still +# read as a fallback, so anything not yet moved over keeps working. +$env:_51DEGREES_RESOURCE_KEY = $Keys.TestResourceKey + ./python/run-integration-tests.ps1 -RepoName $RepoName -Packages $packages -Keys $Keys $status = $LASTEXITCODE @@ -41,7 +47,7 @@ try { $py = $IsWindows ? ".venv/Scripts/python.exe" : ".venv/bin/python" & $py -m pip install -e . $env:PORT = 8097 - $env:resource_key = $Keys.TestResourceKey + $env:_51DEGREES_RESOURCE_KEY = $Keys.TestResourceKey $env:cloud_endpoint = "https://cloud.51degrees.com/api/v4/" $example = & $py -m fiftyone_devicedetection_examples.cloud.gettingstarted_web 2>&1 & } finally { Pop-Location } diff --git a/ci/run-unit-tests.ps1 b/ci/run-unit-tests.ps1 index 08c1215ca..7bb9dea9f 100644 --- a/ci/run-unit-tests.ps1 +++ b/ci/run-unit-tests.ps1 @@ -9,7 +9,11 @@ if ($IsWindows) { Write-Output $env:TEMP } -$packages = "fiftyone_devicedetection_onpremise" +# The cloud package's tests run against fixed cloud responses, so they need +# no resource key, no licence key and no network connection. Running them +# here means a broken cloud example shows as red on every build, rather than +# only when the integration tests happen to have a key to run with. +$packages = "fiftyone_devicedetection_cloud", "fiftyone_devicedetection_onpremise" ./python/run-unit-tests.ps1 -RepoName $RepoName -Packages $packages exit $LASTEXITCODE diff --git a/fiftyone_devicedetection_cloud/src/fiftyone_devicedetection_cloud/hardwareprofile_cloud.py b/fiftyone_devicedetection_cloud/src/fiftyone_devicedetection_cloud/hardwareprofile_cloud.py index a2d92cfb1..4f40f3c10 100644 --- a/fiftyone_devicedetection_cloud/src/fiftyone_devicedetection_cloud/hardwareprofile_cloud.py +++ b/fiftyone_devicedetection_cloud/src/fiftyone_devicedetection_cloud/hardwareprofile_cloud.py @@ -28,6 +28,40 @@ import json +# The suffix the cloud service adds to a property name to carry the reason +# that property has no value, for example "hardwarevendornullreason". +NULL_REASON_SUFFIX = "nullreason" + + +def _build_aspect_values(hardware): + """! + Build the aspect level property values from the "hardware" section of a + cloud response, pairing each null value with the reason the service gave + for it. + + @type hardware: dict + @param hardware: the "hardware" section of the cloud response + + @rtype dict + @return property name to AspectPropertyValue + """ + + values = {} + + for key, value in hardware.items(): + if key == "profiles" or key.endswith(NULL_REASON_SUFFIX): + continue + + if value is None: + reason = hardware.get(key + NULL_REASON_SUFFIX) + values[key] = AspectPropertyValue( + no_value_message=reason if isinstance(reason, str) else None) + else: + values[key] = AspectPropertyValue(value=value) + + return values + + class HardwareProfileCloud(CloudEngine): """! The hardware profile cloud engine @@ -44,18 +78,40 @@ def process_internal(self, flowdata): cloud_data = json.loads(cloud_data) - # Loop over cloud_data.devices properties to check if they have a value + hardware = cloud_data.get("hardware") or {} + + if not isinstance(hardware, dict): + hardware = {} + + # Properties the resource key is not entitled to are returned by the + # cloud service at the aspect level rather than inside each profile, + # with a companion "nullreason" saying why there is no value. + # Collect those so the reason can travel with every profile instead + # of being thrown away. + aspect_values = _build_aspect_values(hardware) devices = [] - for profile in cloud_data["hardware"]["profiles"]: + for profile in hardware.get("profiles") or []: device = {} for property_key, property_value in profile.items(): device[property_key] = AspectPropertyValue(value=property_value) + # Add the properties the service could not supply, carrying the + # reason it gave, so a caller reading a profile gets an + # explanation rather than nothing at all. + for property_key, aspect_value in aspect_values.items(): + if property_key not in device: + device[property_key] = aspect_value + devices.append(device) - data = AspectDataDictionary(self, {"profiles": devices}) + # The aspect level values are exposed alongside the profiles so a + # caller with no matching profiles can still read the reason. + contents = dict(aspect_values) + contents["profiles"] = devices + + data = AspectDataDictionary(self, contents) flowdata.set_element_data(data) \ No newline at end of file diff --git a/fiftyone_devicedetection_cloud/tests/test_cloud.py b/fiftyone_devicedetection_cloud/tests/test_cloud.py index 0cf57d4b1..2a78291c9 100644 --- a/fiftyone_devicedetection_cloud/tests/test_cloud.py +++ b/fiftyone_devicedetection_cloud/tests/test_cloud.py @@ -29,8 +29,29 @@ from fiftyone_devicedetection_cloud.devicedetection_cloud import DeviceDetectionCloud -if not "resource_key" in os.environ: - print("To run the cloud tests, please set a valid 51Degrees cloud resource key as the resource_key environment variable. e.g `export resource_key=MYresource_key` on the command line") +# Resource key environment variables follow the 51Degrees convention, which +# is that every one of them starts with "_51DEGREES_RESOURCE_KEY". The name +# used before the convention was adopted is still read, so an existing setup +# keeps working. +RESOURCE_KEY_ENV_VAR = "_51DEGREES_RESOURCE_KEY" +LEGACY_RESOURCE_KEY_ENV_VAR = "resource_key" + +MISSING_KEY_MESSAGE = ( + "No resource key found, so the tests that call the cloud service " + f"cannot run. Set the environment variable '{RESOURCE_KEY_ENV_VAR}' " + f"(the older name '{LEGACY_RESOURCE_KEY_ENV_VAR}' is still read). " + "Create a resource key for free at " + "https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=device-detection-python&utm_content=fiftyone_devicedetection_cloud-tests-test_cloud.py&utm_term=resource-key-required") + + +def get_resource_key(): + """! + The resource key from the environment, or None when neither the aligned + nor the older variable is set. + """ + + return (os.environ.get(RESOURCE_KEY_ENV_VAR) + or os.environ.get(LEGACY_RESOURCE_KEY_ENV_VAR)) mobile_ua = "Mozilla/5.0 (iPhone; CPU iPhone OS 11_2 like Mac OS X) AppleWebKit/604.4.7 (KHTML, like Gecko) Mobile/15C114" @@ -38,15 +59,19 @@ class DeviceDetectionTests(unittest.TestCase): + def setUp(self): + self.resource_key = get_resource_key() + + if not self.resource_key: + self.skipTest(MISSING_KEY_MESSAGE) + + def test_pipeline_builder_cloud_engine_init(self): """! Tests whether the device detection pipeline builder adds the correct engines when initialised with a resource key """ - if not "resource_key" in os.environ: - return - - pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = os.environ['resource_key']).build() + pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = self.resource_key).build() self.assertTrue(pipeline.flow_elements[0].datakey == "cloud") self.assertTrue(pipeline.flow_elements[1].datakey == "device") @@ -56,10 +81,7 @@ def test_properties_cloud(self): Tests whether a properties list is created on the cloud engine """ - if not "resource_key" in os.environ: - return - - pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = os.environ['resource_key']).build() + pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = self.resource_key).build() properties = pipeline.flow_elements[1].get_properties() @@ -70,10 +92,7 @@ def test_basic_get_cloud(self): Check property lookup works """ - if not "resource_key" in os.environ: - return - - pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = os.environ['resource_key']).build() + pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = self.resource_key).build() fd = pipeline.create_flowdata() @@ -89,7 +108,7 @@ def test_missing_property_service_element_not_found(self): not available in any datafile """ - pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = os.environ['resource_key']).build() + pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = self.resource_key).build() fd = pipeline.create_flowdata() @@ -118,7 +137,7 @@ def test_engine_init_performance(self): start = time.time() - pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = os.environ['resource_key']).build() + pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = self.resource_key).build() fd = pipeline.create_flowdata() @@ -144,7 +163,7 @@ def test_missing_property_service_not_found_anywhere(self): not available in cloud """ - pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = os.environ['resource_key']).build() + pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = self.resource_key).build() fd = pipeline.create_flowdata() diff --git a/fiftyone_devicedetection_cloud/tests/test_hardwareprofile_cloud.py b/fiftyone_devicedetection_cloud/tests/test_hardwareprofile_cloud.py new file mode 100644 index 000000000..1ea4e7815 --- /dev/null +++ b/fiftyone_devicedetection_cloud/tests/test_hardwareprofile_cloud.py @@ -0,0 +1,176 @@ +# ********************************************************************* +# This Original Work is copyright of 51 Degrees Mobile Experts Limited. +# Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, +# Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. +# +# This Original Work is licensed under the European Union Public Licence +# (EUPL) v.1.2 and is subject to its terms as set out below. +# +# If a copy of the EUPL was not distributed with this file, You can obtain +# one at https://opensource.org/licenses/EUPL-1.2. +# +# The 'Compatible Licences' set out in the Appendix to the EUPL (as may be +# amended by the European Commission) shall be deemed incompatible for +# the purposes of the Work and the provisions of the compatibility +# clause in Article 5 of the EUPL shall not apply. +# +# If using the Work as, or as part of, a network application, by +# including the attribution notice(s) required under Article 5 of the EUPL +# in the end user terms of the application under an appropriate heading, +# such notice(s) shall fulfill the requirements of that article. +# ********************************************************************* + +"""! +These tests run against fixed cloud responses, so they need no network +connection and no resource key. They cover the case that used to stop the +TAC and native model examples with an AttributeError, which is a resource +key that returns the hardware profiles but is not entitled to the hardware +properties themselves. +""" + +import json +import unittest + +from fiftyone_devicedetection_cloud.hardwareprofile_cloud import HardwareProfileCloud + +# A response of the shape the cloud service returns for a TAC lookup when +# the resource key has the hardware aspect but is not entitled to the +# hardware properties. The values are null at the aspect level and each one +# is paired with a reason. The profiles carry only the properties the key is +# entitled to. +NOT_ENTITLED_RESPONSE = json.dumps({ + "hardware": { + "profiles": [ + {"devicetype": "SmartPhone", "ismobile": True} + ], + "hardwarevendor": None, + "hardwarevendornullreason": + "HardwareVendor is a paid feature. You need a licence key to " + "retrieve data.", + "hardwarename": None, + "hardwarenamenullreason": + "HardwareName is a paid feature. You need a licence key to " + "retrieve data.", + "hardwaremodel": None, + "hardwaremodelnullreason": + "HardwareModel is a paid feature. You need a licence key to " + "retrieve data." + } +}) + +# A response from a fully entitled resource key. +ENTITLED_RESPONSE = json.dumps({ + "hardware": { + "profiles": [ + { + "hardwarevendor": "Apple", + "hardwarename": ["iPhone 6"], + "hardwaremodel": "A1586" + } + ] + } +}) + +# A response where the hardware aspect is absent altogether, which is what a +# resource key without the hardware aspect returns. +NO_HARDWARE_RESPONSE = json.dumps({"device": {"ismobile": True}}) + + +class _FakeCloudElementData: + """! + Stands in for the element data the cloud request engine publishes. + """ + + def __init__(self, response): + self._response = response + + def get(self, key): + return self._response + + +class _FakeFlowData: + """! + Stands in for a FlowData carrying a fixed cloud response, so the engine + can be tested with no network connection and no resource key. + """ + + def __init__(self, response): + self._response = response + self.element_data = None + + def get(self, key): + return _FakeCloudElementData(self._response) + + def set_element_data(self, data): + self.element_data = data + + +def _process(response): + """! + Run a fixed cloud response through the hardware profile engine and + return the resulting element data. + """ + + engine = HardwareProfileCloud() + flowdata = _FakeFlowData(response) + + engine.process_internal(flowdata) + + return flowdata.element_data + + +class HardwareProfileCloudTests(unittest.TestCase): + + def test_null_reason_is_carried_into_each_profile(self): + hardware = _process(NOT_ENTITLED_RESPONSE) + profiles = hardware.get("profiles") + + self.assertEqual(1, len(profiles)) + + profile = profiles[0] + + for name in ["hardwarevendor", "hardwarename", "hardwaremodel"]: + self.assertIn( + name, profile, + f"'{name}' should be present on the profile so the reason " + "it has no value can be reported") + self.assertFalse(profile[name].has_value()) + self.assertIn("paid feature", profile[name].no_value_message()) + + def test_null_reason_is_reported_by_the_example_helper(self): + # The example helper lives in the examples package, which is not a + # dependency of this one, so the same formatting is asserted here + # against the values the engine produced. + hardware = _process(NOT_ENTITLED_RESPONSE) + profile = hardware.get("profiles")[0] + value = profile["hardwarevendor"] + + self.assertEqual( + "HardwareVendor is a paid feature. You need a licence key to " + "retrieve data.", + value.no_value_message()) + + def test_entitled_values_are_reported_as_values(self): + hardware = _process(ENTITLED_RESPONSE) + profile = hardware.get("profiles")[0] + + self.assertTrue(profile["hardwarevendor"].has_value()) + self.assertEqual("Apple", profile["hardwarevendor"].value()) + self.assertEqual(["iPhone 6"], profile["hardwarename"].value()) + self.assertEqual("A1586", profile["hardwaremodel"].value()) + + def test_absent_hardware_aspect_gives_no_profiles_rather_than_an_error(self): + hardware = _process(NO_HARDWARE_RESPONSE) + + self.assertEqual([], hardware.get("profiles")) + + def test_aspect_level_values_are_exposed_alongside_the_profiles(self): + # A caller with no matching profiles can still read the reason. + hardware = _process(NOT_ENTITLED_RESPONSE) + + self.assertIn("paid feature", + hardware.get("hardwarevendor").no_value_message()) + + +if __name__ == "__main__": + unittest.main() diff --git a/fiftyone_devicedetection_cloud/tests/test_properties.py b/fiftyone_devicedetection_cloud/tests/test_properties.py index a54b3aa90..f14d58e7e 100644 --- a/fiftyone_devicedetection_cloud/tests/test_properties.py +++ b/fiftyone_devicedetection_cloud/tests/test_properties.py @@ -43,11 +43,41 @@ # TODO remove setheader properties from this list once UACH datafile is released. exclude_properties = ["setheaderbrowseraccept-ch", "setheaderplatformaccept-ch", "setheaderhardwareaccept-ch"] -if "resource_key" in os.environ: - resource_key = os.environ["resource_key"] -else: - raise Exception("To run the cloud tests, please set a valid 51Degrees " - "cloud resource key as the resource_key environment variable.") +# Resource key environment variables follow the 51Degrees convention, which +# is that every one of them starts with "_51DEGREES_RESOURCE_KEY". The name +# used before the convention was adopted is still read, so an existing setup +# keeps working. +RESOURCE_KEY_ENV_VAR = "_51DEGREES_RESOURCE_KEY" +LEGACY_RESOURCE_KEY_ENV_VAR = "resource_key" + +resource_key = (os.environ.get(RESOURCE_KEY_ENV_VAR) + or os.environ.get(LEGACY_RESOURCE_KEY_ENV_VAR)) + +if not resource_key: + # Skipping rather than raising, so a run without a key shows as skipped + # and names the variable, instead of turning into a collection error. + import pytest + + pytest.skip( + "No resource key found, so the tests that call the cloud service " + f"cannot run. Set the environment variable '{RESOURCE_KEY_ENV_VAR}' " + f"(the older name '{LEGACY_RESOURCE_KEY_ENV_VAR}' is still read). " + "Create a resource key for free at " + "https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=device-detection-python&utm_content=fiftyone_devicedetection_cloud-tests-test_properties.py&utm_term=resource-key-required", + allow_module_level=True) + +if not os.path.isfile(header_file_path): + # The property list comes from an asset the build fetches with a licence + # key. Without it these tests cannot run, so skip rather than turn a + # missing asset into a failure. + import pytest + + pytest.skip( + f"The property list file '{header_file_path}' is not present, so " + "the property coverage tests cannot run. It is fetched by " + "ci/fetch-assets.ps1 when a device detection licence key is " + "available.", + allow_module_level=True) # Get Properties list properties_list = get_properties_from_header_file(header_file_path) diff --git a/fiftyone_devicedetection_cloud/tox.ini b/fiftyone_devicedetection_cloud/tox.ini index 6ed905064..13627d8e0 100644 --- a/fiftyone_devicedetection_cloud/tox.ini +++ b/fiftyone_devicedetection_cloud/tox.ini @@ -16,6 +16,7 @@ deps = commands = pytest --cov={envsitepackagesdir}/fiftyone_devicedetection_cloud {tty:--color=yes} {posargs} pass_env = + _51DEGREES_RESOURCE_KEY resource_key license_key diff --git a/fiftyone_devicedetection_examples/setup.py b/fiftyone_devicedetection_examples/setup.py index 7553004c6..06813df22 100644 --- a/fiftyone_devicedetection_examples/setup.py +++ b/fiftyone_devicedetection_examples/setup.py @@ -57,7 +57,11 @@ def read(file_name): "fiftyone_devicedetection_examples.cloud.useragentclienthints_web", ], package_dir={"": "src"}, - install_requires=["fiftyone_devicedetection", "flask", "flask-unittest", "json5", "ruamel.yaml"], + # The cloud examples need only fiftyone_devicedetection_cloud, which is + # named here so that is visible to anyone reading them. + # fiftyone_devicedetection is still required because the on-premise + # examples in this same package use the on-premise engine. + install_requires=["fiftyone_devicedetection_cloud", "fiftyone_devicedetection", "flask", "flask-unittest", "json5", "ruamel.yaml"], license="EUPL-1.2", classifiers=[ "Development Status :: 5 - Production/Stable", diff --git a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/configurator_console.py b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/configurator_console.py index fb5776bdb..8e029e734 100644 --- a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/configurator_console.py +++ b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/configurator_console.py @@ -22,7 +22,7 @@ import sys from fiftyone_pipeline_core.logger import Logger -from fiftyone_devicedetection.devicedetection_pipelinebuilder import DeviceDetectionPipelineBuilder +from fiftyone_devicedetection_cloud.devicedetection_cloud_pipelinebuilder import DeviceDetectionCloudPipelineBuilder from fiftyone_devicedetection_examples.example_utils import ExampleUtils # This example is displayed at the end of the [Configurator](https://configure.51degrees.com/?utm_source=code&utm_medium=example&utm_campaign=device-detection-python&utm_content=fiftyone_devicedetection_examples-src-fiftyone_devicedetection_examples-cloud-configurator_console.py&utm_term=top) @@ -35,13 +35,13 @@ # for a fuller example. # # Required PyPi Dependencies: -# - [fiftyone_devicedetection](https://pypi.org/project/fiftyone-devicedetection/) +# - [fiftyone_devicedetection_cloud](https://pypi.org/project/fiftyone-devicedetection-cloud/) class ConfiguratorConsole(): def run(self, resource_key, logger, output): # Create a minimal pipeline to access the cloud engine # you only need one pipeline for multiple requests - pipeline = DeviceDetectionPipelineBuilder( + pipeline = DeviceDetectionCloudPipelineBuilder( resource_key = resource_key).add_logger(logger).build() # Get a flow data from the singleton pipeline for each detection @@ -73,7 +73,11 @@ def run(self, resource_key, logger, output): # Get the results. device = data.device - output(f"device.ismobile: {device.ismobile.value()}") + # Read through the helper so that a resource key without access to + # 'ismobile' reports the reason the cloud service gave, rather than + # stopping the example part way through. + output("device.ismobile: " + + f"{ExampleUtils.get_human_readable(device, 'ismobile')}") def main(argv): # Use the command line args to get the resource key if present. diff --git a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/gettingstarted_console.py b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/gettingstarted_console.py index 801898521..d7b4aac4d 100644 --- a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/gettingstarted_console.py +++ b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/gettingstarted_console.py @@ -36,12 +36,11 @@ # @include{doc} example-require-resourcekey.txt # # Required PyPi Dependencies: -# - [fiftyone_devicedetection](https://pypi.org/project/fiftyone-devicedetection/) +# - [fiftyone_devicedetection_cloud](https://pypi.org/project/fiftyone-devicedetection-cloud/) import json5 from pathlib import Path import sys -from fiftyone_devicedetection.devicedetection_pipelinebuilder import DeviceDetectionPipelineBuilder from fiftyone_pipeline_core.logger import Logger from fiftyone_pipeline_core.pipelinebuilder import PipelineBuilder from fiftyone_devicedetection_examples.example_utils import ExampleUtils diff --git a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/gettingstarted_web/app.py b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/gettingstarted_web/app.py index 37000400f..11eb46906 100644 --- a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/gettingstarted_web/app.py +++ b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/gettingstarted_web/app.py @@ -30,12 +30,12 @@ # @include{doc} example-require-resourcekey.txt # # Required PyPi Dependencies: -# - [fiftyone_devicedetection](https://pypi.org/project/fiftyone-devicedetection/) +# - [fiftyone_devicedetection_cloud](https://pypi.org/project/fiftyone-devicedetection-cloud/) # - [flask](https://pypi.org/project/flask/) # # ## Overview # -# The `DeviceDetectionPipelineBuilder` class is used to create a Pipeline instance from the configuration +# The `DeviceDetectionCloudPipelineBuilder` class is used to create a Pipeline instance from the configuration # that is supplied. # The fiftyone_pipeline_core.web module contains helpers which deal with # automatically populating evidence from a web request. @@ -77,7 +77,7 @@ from fiftyone_devicedetection_examples.example_utils import ExampleUtils from flask import Flask, request, render_template from flask.helpers import make_response -from fiftyone_devicedetection.devicedetection_pipelinebuilder import DeviceDetectionPipelineBuilder +from fiftyone_devicedetection_cloud.devicedetection_cloud_pipelinebuilder import DeviceDetectionCloudPipelineBuilder from fiftyone_pipeline_core.logger import Logger from fiftyone_pipeline_core.web import webevidence, set_response_header import json @@ -113,7 +113,7 @@ def build(self, resource_key, logger): if cloud_endpoint: pipeline_settings["cloud_endpoint"] = cloud_endpoint - GettingStartedWeb.pipeline = DeviceDetectionPipelineBuilder( + GettingStartedWeb.pipeline = DeviceDetectionCloudPipelineBuilder( **pipeline_settings).add_logger(logger).build() return self diff --git a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/metadata_console.py b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/metadata_console.py index 478e199b1..6f8b4236b 100644 --- a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/metadata_console.py +++ b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/metadata_console.py @@ -43,12 +43,12 @@ # @include{doc} example-require-resourcekey.txt # # Required PyPi Dependencies: -# - [fiftyone_devicedetection](https://pypi.org/project/fiftyone-devicedetection/) +# - [fiftyone_devicedetection_cloud](https://pypi.org/project/fiftyone-devicedetection-cloud/) # from pathlib import Path import sys -from fiftyone_devicedetection.devicedetection_pipelinebuilder import DeviceDetectionPipelineBuilder +from fiftyone_devicedetection_cloud.devicedetection_cloud_pipelinebuilder import DeviceDetectionCloudPipelineBuilder from fiftyone_pipeline_core.logger import Logger from fiftyone_pipeline_core.basiclist_evidence_keyfilter import BasicListEvidenceKeyFilter from fiftyone_devicedetection_examples.example_utils import ExampleUtils @@ -60,7 +60,7 @@ class MetaDataConsole(): def run(self, resource_key, logger, output): - pipeline = DeviceDetectionPipelineBuilder( + pipeline = DeviceDetectionCloudPipelineBuilder( resource_key = resource_key).add_logger(logger).build() self.outputProperties(pipeline.get_element("device"), output) diff --git a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/nativemodellookup_console.py b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/nativemodellookup_console.py index c8c2f06a4..7bdf6e895 100644 --- a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/nativemodellookup_console.py +++ b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/nativemodellookup_console.py @@ -96,7 +96,13 @@ def analyseModel(self, nativemodel, pipeline, output): # The 'hardware.profiles' object contains one or more devices. # This is the same interface used for standard device detection, so we have # access to all the same properties. - for device in result.profiles: + profiles = ExampleUtils.get_profiles(result) + + if len(profiles) == 0: + output(ExampleUtils.get_no_profiles_message()) + return + + for device in profiles: vendor = ExampleUtils.get_human_readable(device, "hardwarevendor") name = ExampleUtils.get_human_readable(device, "hardwarename") model = ExampleUtils.get_human_readable(device, "hardwaremodel") diff --git a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/taclookup_console.py b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/taclookup_console.py index 9b7287c3d..0092c6da6 100644 --- a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/taclookup_console.py +++ b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/cloud/taclookup_console.py @@ -85,7 +85,13 @@ def analyseTac(self, tac, pipeline, output): # The 'hardware.profiles' object contains one or more devices. # This is the same interface used for standard device detection, so we have # access to all the same properties. - for device in result.profiles: + profiles = ExampleUtils.get_profiles(result) + + if len(profiles) == 0: + output(ExampleUtils.get_no_profiles_message()) + return + + for device in profiles: vendor = ExampleUtils.get_human_readable(device, "hardwarevendor") name = ExampleUtils.get_human_readable(device, "hardwarename") model = ExampleUtils.get_human_readable(device, "hardwaremodel") diff --git a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py index d7ea329bb..236c6f937 100644 --- a/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py +++ b/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py @@ -34,9 +34,15 @@ class ExampleUtils: # be displayed. DATA_FILE_AGE_WARNING = 30 - # The default environment variable used to get the resource key - # to use when running examples. - RESOURCE_KEY_ENV_VAR = "resource_key" + # The environment variable used to get the resource key to use when + # running examples. This follows the 51Degrees convention that every + # resource key variable starts with "_51DEGREES_RESOURCE_KEY". + RESOURCE_KEY_ENV_VAR = "_51DEGREES_RESOURCE_KEY" + + # The environment variable this repository used before the convention + # above was adopted. It is still read, so an existing setup keeps + # working, but new setups should use RESOURCE_KEY_ENV_VAR. + LEGACY_RESOURCE_KEY_ENV_VAR = "resource_key" ENDPOINT_ENV_VAR = "cloud_endpoint" @@ -72,18 +78,90 @@ def set_data_file_in_config(config, file): @staticmethod def get_human_readable(device, property): + """! + Format a property value for display. + + A property can be unavailable for three reasons, and each one reads + differently so the person running the example can tell them apart. + The property may have a value, it may have no value with the cloud + service giving a reason (most often that the resource key is not + entitled to it), or it may not be in the results at all. + """ + try: value = device.get(property) except Exception: - return "Property not found in data file" - if value and value.has_value(): - return value.value() - else: - return f"Unknown ({value.no_value_message()})" + value = None + + if value is None: + return (f"Unknown (the property '{property}' is not in the " + "results, so the current resource key does not " + "include it)") + + if value.has_value(): + result = value.value() + if isinstance(result, list): + return ", ".join(str(item) for item in result) + return result + + reason = value.no_value_message() + + if not reason: + return (f"Unknown (the cloud service returned no value for " + f"'{property}' and gave no reason)") + + return f"Unknown ({reason})" + + @staticmethod + def get_profiles(hardware): + """! + Read the list of hardware profiles from the "hardware" element data. + + An empty list is returned when the resource key has no access to the + hardware aspect at all, because reading a property that is not there + raises rather than returning an empty list. + """ + + try: + profiles = hardware.profiles + except Exception: + return [] + + return profiles if isinstance(profiles, list) else [] + + @staticmethod + def get_no_profiles_message(): + """! + The line printed when a lookup returned no device profiles at all. + """ + + return ("\tNo device profiles were returned. The current resource " + "key does not include the hardware properties this example " + "needs. See " + "https://51degrees.com/pricing?utm_source=code&utm_medium=example&utm_campaign=device-detection-python&utm_content=fiftyone_devicedetection_examples-src-fiftyone_devicedetection_examples-example_utils.py&utm_term=no-profiles") @staticmethod def get_resource_key(): - return ExampleUtils.__get_env_variable(ExampleUtils.RESOURCE_KEY_ENV_VAR) + key = ExampleUtils.__get_env_variable(ExampleUtils.RESOURCE_KEY_ENV_VAR) + + if not key: + key = ExampleUtils.__get_env_variable( + ExampleUtils.LEGACY_RESOURCE_KEY_ENV_VAR) + + return key + + @staticmethod + def get_missing_resource_key_message(): + """! + The message shown when no resource key is set, naming the variable + that was wanted rather than leaving the reader to guess. + """ + + return (f"No resource key found. Set the environment variable " + f"'{ExampleUtils.RESOURCE_KEY_ENV_VAR}' (the older name " + f"'{ExampleUtils.LEGACY_RESOURCE_KEY_ENV_VAR}' is still " + "read). Create a resource key for free at " + "https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=device-detection-python&utm_content=fiftyone_devicedetection_examples-src-fiftyone_devicedetection_examples-example_utils.py&utm_term=resource-key-required") @staticmethod def get_cloud_endpoint(): diff --git a/fiftyone_devicedetection_examples/tests/test_cloudexampleimports.py b/fiftyone_devicedetection_examples/tests/test_cloudexampleimports.py new file mode 100644 index 000000000..a28b1700f --- /dev/null +++ b/fiftyone_devicedetection_examples/tests/test_cloudexampleimports.py @@ -0,0 +1,106 @@ +# ********************************************************************* +# This Original Work is copyright of 51 Degrees Mobile Experts Limited. +# Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, +# Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. +# +# This Original Work is licensed under the European Union Public Licence +# (EUPL) v.1.2 and is subject to its terms as set out below. +# +# If a copy of the EUPL was not distributed with this file, You can obtain +# one at https://opensource.org/licenses/EUPL-1.2. +# +# The 'Compatible Licences' set out in the Appendix to the EUPL (as may be +# amended by the European Commission) shall be deemed incompatible for +# the purposes of the Work and the provisions of the compatibility +# clause in Article 5 of the EUPL shall not apply. +# +# If using the Work as, or as part of, a network application, by +# including the attribution notice(s) required under Article 5 of the EUPL +# in the end user terms of the application under an appropriate heading, +# such notice(s) shall fulfill the requirements of that article. +# ********************************************************************* + +"""! +A cloud example has to run with only the cloud packages installed, because +the on-premise package ships as source and needs a C++ toolchain to build. +Someone following a cloud example should not have to build that. + +These tests import each cloud example in a separate process with the +on-premise package blocked, so they prove the point whether or not the +on-premise package happens to be installed on the machine running them. +They need no resource key and no network connection. +""" + +import subprocess +import sys +import unittest + +# The packages a cloud example must not reach for, directly or through +# anything it imports. +BLOCKED = [ + "fiftyone_devicedetection_onpremise", + "fiftyone_devicedetection", +] + +# Every cloud example module. Adding a cloud example means adding it here. +CLOUD_EXAMPLES = [ + "fiftyone_devicedetection_examples.cloud.configurator_console", + "fiftyone_devicedetection_examples.cloud.failuretomatch", + "fiftyone_devicedetection_examples.cloud.gettingstarted_console", + "fiftyone_devicedetection_examples.cloud.metadata_console", + "fiftyone_devicedetection_examples.cloud.nativemodellookup_console", + "fiftyone_devicedetection_examples.cloud.taclookup_console", + "fiftyone_devicedetection_examples.cloud.gettingstarted_web.app", + "fiftyone_devicedetection_examples.cloud.useragentclienthints_web.app", +] + +# Run in a separate process so that a module already imported by another +# test cannot hide the dependency. The finder is placed at the front of +# sys.meta_path, so it is consulted before the ordinary import machinery. +SCRIPT = """ +import importlib.abc +import sys + +blocked = {blocked!r} + + +class Blocker(importlib.abc.MetaPathFinder): + def find_module(self, fullname, path=None): + return self.find_spec(fullname, path) + + def find_spec(self, fullname, path=None, target=None): + root = fullname.split(".")[0] + if root in blocked: + raise ImportError( + "'" + fullname + "' must not be needed by a cloud example") + return None + + +sys.meta_path.insert(0, Blocker()) + +import {module} +print("ok") +""" + + +class CloudExampleImportTests(unittest.TestCase): + + def test_every_cloud_example_imports_without_the_onpremise_package(self): + for module in CLOUD_EXAMPLES: + with self.subTest(module=module): + script = SCRIPT.format(blocked=BLOCKED, module=module) + + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True) + + self.assertEqual( + 0, result.returncode, + f"'{module}' could not be imported with the on-premise " + f"packages blocked, so a cloud example needs the " + f"on-premise native engine:\n{result.stderr}") + + +if __name__ == "__main__": + unittest.main() diff --git a/fiftyone_devicedetection_examples/tests/test_cloudexamples.py b/fiftyone_devicedetection_examples/tests/test_cloudexamples.py index d4923a8a1..125f972e6 100644 --- a/fiftyone_devicedetection_examples/tests/test_cloudexamples.py +++ b/fiftyone_devicedetection_examples/tests/test_cloudexamples.py @@ -32,37 +32,120 @@ from fiftyone_devicedetection_examples.cloud.gettingstarted_console import GettingStartedConsole from fiftyone_devicedetection_examples.cloud.configurator_console import ConfiguratorConsole +# Text that shows an example printed a programming fault rather than a +# result. An example that reaches any of these is broken, however little of +# the data the resource key is entitled to. +FAULT_MARKERS = [ + "Traceback", + "AttributeError", + "TypeError", + "KeyError", + "Unknown ()", + "Unknown (None)" +] + + class DeviceDetectionExampleTests(unittest.TestCase): - # Init method - specify Resource Key to run examples here or - # set a Resource Key in an environment variable called 'resource_key'. + # Init method - specify a resource key here, or set one in the + # environment variable named by ExampleUtils.RESOURCE_KEY_ENV_VAR. def setUp(self): self.resource_key = ExampleUtils.get_resource_key() self.logger = Logger() if not self.resource_key: - self.fail( - "ResourceKey must be specified in the setUp method" + - " or as an Environment variable") + # Skipping rather than passing, so a run without a key is not + # mistaken for a run that proved something. The message names + # the variable that was wanted. + self.skipTest(ExampleUtils.get_missing_resource_key_message()) + + def run_example(self, example): + """! + Run an example, collecting everything it writes through its output + callback, and fail if any of it reads as a programming fault. + """ + + lines = [] + example(lines.append) + output = "\n".join(str(line) for line in lines) + + self.assertGreater(len(lines), 0, + "The example produced no output at all") + + for marker in FAULT_MARKERS: + self.assertNotIn(marker, output, + f"The example output contains '{marker}', which means it " + f"failed rather than reporting a result. Output was:\n" + f"{output}") + + return output + + def assert_device_lines_are_meaningful(self, output): + """! + Every device line the TAC and native model examples print must say + something useful. Either it names a device, or it says the property + has no value and gives the reason the cloud service supplied. + """ + + lines = [line for line in output.split("\n") + if line.startswith("\t")] + + self.assertGreater(len(lines), 0, + "The example listed no devices and gave no reason for it") + + for line in lines: + self.assertNotEqual("", line.strip(), "A device line is empty") def test_cloud_getting_started_console(self): example = GettingStartedConsole() configFile = Path(inspect.getfile(example.__class__)).parent.resolve().joinpath("gettingstarted_console.json").read_text() config = json5.loads(configFile) ExampleUtils.set_resource_key_in_config(config, self.resource_key) - example.run(config, self.logger, print) + + output = self.run_example( + lambda out: example.run(config, self.logger, out)) + + self.assertIn("Input values:", output) + self.assertIn("Mobile Device:", output) + def test_cloud_nativemodellookup_console(self): example = NativeModelLookupConsole() - example.run(self.resource_key, self.logger, print) + + output = self.run_example( + lambda out: example.run(self.resource_key, self.logger, out)) + + self.assertIn( + "Which devices are associated with the native model name " + "'SC-03L'?", output) + self.assert_device_lines_are_meaningful(output) + def test_cloud_taclookup_console(self): example = TacLookupConsole() configFile = Path(inspect.getfile(example.__class__)).parent.resolve().joinpath("taclookup_console.json").read_text() config = json5.loads(configFile) ExampleUtils.set_resource_key_in_config(config, self.resource_key) - example.run(config, self.logger, print) + + output = self.run_example( + lambda out: example.run(config, self.logger, out)) + + self.assertIn( + "Which devices are associated with the TAC '35925406'?", output) + self.assert_device_lines_are_meaningful(output) + def test_cloud_metadata_console(self): example = MetaDataConsole() - example.run(self.resource_key, self.logger, print) + + output = self.run_example( + lambda out: example.run(self.resource_key, self.logger, out)) + + self.assertIn("Accepted evidence keys:", output) + self.assertIn("Property - ", output) + def test_cloud_configurator_console(self): example = ConfiguratorConsole() - example.run(self.resource_key, self.logger, print) \ No newline at end of file + + output = self.run_example( + lambda out: example.run(self.resource_key, self.logger, out)) + + self.assertIn("device.ismobile: ", output) + self.assertNotEqual("device.ismobile:", output.strip()) diff --git a/fiftyone_devicedetection_examples/tox.ini b/fiftyone_devicedetection_examples/tox.ini index 3432f48e0..af1c530bc 100644 --- a/fiftyone_devicedetection_examples/tox.ini +++ b/fiftyone_devicedetection_examples/tox.ini @@ -26,6 +26,7 @@ commands = pwsh -ExecutionPolicy Bypass -File ../ci/build-onpremise-c-module.ps1 {[common]commands} pass_env = + _51DEGREES_RESOURCE_KEY resource_key license_key run_performance_tests diff --git a/fiftyone_devicedetection_onpremise/tox.ini b/fiftyone_devicedetection_onpremise/tox.ini index 93240ac54..4e57a534c 100644 --- a/fiftyone_devicedetection_onpremise/tox.ini +++ b/fiftyone_devicedetection_onpremise/tox.ini @@ -26,6 +26,7 @@ commands = python setup.py build_clib build_ext {[common]commands} pass_env = + _51DEGREES_RESOURCE_KEY resource_key license_key TMPDIR