[jwrosewell]

Problem

For #12.

The Cloud Request Engine specification says a request authenticates with a Resource Key, a License Key or both, and that a list of Requested Properties is sent as values. The table of accepted combinations, as specifications#131 left it on rc, is:

Resource Key	License Key	Requested Properties	Valid	Reason
Yes	No	No	Yes	The Resource Key already states which Properties it carries.
Yes	Yes	No	Yes	The response holds what the Resource Key carries, and the License Key adds no Property.
Yes	Yes	Yes	Yes	The response adds the Requested Properties the License Key grants.
No	Yes	Yes	Yes	The License Key names no Properties, so the caller names the ones it wants.
No	Yes	No	No	The remote server answers 400 to such a request, so every request would fail.
Yes	No	Yes	Yes	Each requested Property is added, as null with a reason where no licence grants it.
No	No	Either	No	There is nothing to authenticate with, and the remote server answers 401.

An Engine MUST refuse an invalid combination when it is built, in its builder or constructor, rather than when a request is made.

The discovery section says:

An Engine built on a License Key alone MUST make the accessibleproperties request with the License Key, sent as license in the body of a POST request rather than in the URL, so that the key stays out of request logs.

James Rosewell decided on 10 November that only what the cloud service refuses today is refused at build. A Resource Key with a property list builds, with or without a License Key, and the list is sent.

On rc this repository could not do any of that.

• CloudRequestEngineBuilder.java refused to build without a resource key, so a license key alone could not build.
• The same method takes the properties that setProperty and setProperties collect, and never used them. The engine could not be told which properties to ask for.
• CloudRequestEngineDefault.java built the accessibleproperties URL from the resource key alone, so the products a license key adds were never discovered.

Change

• Build. CloudRequestEngineBuilder refuses only the two invalid rows, being no credential at all and a license key with no requested properties. Each message says why the combination cannot work, in the wording of the .NET draft, 51degrees-dotnet#129, and of the three build messages in the Python rc constants.py, less the one for a resource key with properties, which is no longer refused. A key that is empty or only white space counts as absent, and keys are trimmed, because one read from an environment variable often carries a trailing newline.
• Requested Properties. A new setRequestedProperties names them, as a list or as one comma separated string, which is the form a configuration file gives. Names are trimmed and sent as given, a name given again in another letter case is sent once as first given, and the list is sent as values whenever the list is not empty, beside a resource key, a license key or both. setProperty and setProperties are unchanged and their list is still not sent, for the reason under Effect on existing callers.
• Requests. The data request sends resource only when a resource key is set, then license when a license key is set, then values. The data endpoint keeps its route. With no resource key it is the plain json endpoint, as it already was on rc when no resource key was set.
• Discovery. When a license key is set, the accessibleproperties request is a POST carrying license, and resource if there is one, in its body, so the key never appears in a URL. This follows the Python rc engine (cloudrequestengine.py). A resource key alone is still sent with GET in the query string. evidencekeys is unchanged.
• Properties left out. As the specification's section on Properties the remote server did not return asks, CloudRequestEngineDefault compares the requested names with the answer, ignoring letter case, and warns once about any left out. A property present but null counts as answered. The warning never fails the request.
• Messages. Constants.java gains NoCredential, LicenseKeyNeedsProperties, PropertiesNotCovered and RequestedNamesNotProperties.

Review fixes

The same two findings as on 51degrees-dotnet#129, made the same way so the two languages match, against cloud-request-engine.md at e89679e9, and two more.

1. Names are sent as given (commit fd7242a). normalized lower-cased every name it sent as values, because "the cloud service answers in lower case". That is about the answer, which the comparison already reads without regard to case ("The comparison MUST be case insensitive, because the remote server lowercases Property names in its response"), and every measurement in the specification sends mixed case names. The names are now trimmed and sent as given, and a repeat that differs only in letter case is dropped, keeping the first spelling.

2. A name that is not a property gets its own warning (commits fd7242a and e99d627). The section says "With a Resource Key, Requested Properties and no License Key, a Property no licence grants is answered as null with its nullreason, and so counts as answered. A name the answer leaves out is then one that is not a Property, and the report says so rather than naming a License Key that was never given." With no license key the engine now logs RequestedNamesNotProperties, with the same text as .NET, and with a license key PropertiesNotCovered as before.

3. A null list is refused (commit fd7242a). Both setRequestedProperties overloads throw IllegalArgumentException naming properties, as .NET throws ArgumentNullException, where before addAll threw a NullPointerException from inside the list.

4. The json route under a license key alone, measured. A runner posted to the plain route with the 51Did test license key and no resource key, on 11 November 2026 (run 43020665840). POST https://cloud.51degrees.com/api/v4/accessibleproperties with license answered HTTP 200, listing the product FODid with 6 properties. POST https://cloud.51degrees.com/api/v4/json with license, values=FODid.IdProbGlobal and User-Agent answered HTTP 200, with the body {}. The same request with no values answered HTTP 400, "This request authenticated with a license key but did not specify which properties to return." So the route accepts a license key alone and refuses one with no list, which is the second refused row of the Accepted combinations table.

5. The new engine constructor has its caller. The CloudRequestEngineDefault constructor that takes the requested properties is used by CloudRequestEngineBuilder.newEngine, the only place in this repository that builds the engine, in this change. The overloads that take no list delegate to it with an empty list, so code that builds the engine directly keeps working and sends no values.

The merge of rc (commit 3722d66). rc at ddb7d04 holds the eager discovery change, #86, and #99, and both conflicted with this draft in CloudRequestEngineBuilder.java and CloudRequestEngineDefault.java. The engine is built with this draft's trimmed keys and requested properties and then given rc's discovery, rc's setDiscoveryState and getDiscoveryState are kept, and the accessibleproperties request keeps this draft's POST under a license key, with rc's timeouts on both routes.

A test fix the merge needed (commit f45eced). Discovery on rc fetches the evidence keys on another thread while it fetches the accessible properties, so the tests' fake cloud records calls from two threads. Its plain ArrayList could lose a call or hold a null, which failed the resource key discovery and data request tests in one run (run 43023140398), depending on the order the tests ran in. It now uses a synchronized list, as rc's own CloudRequestEngineDiscoveryTests does.

Two more test fixes from the merge of rc at 528e0df (commits a9ecacc, 72faa2d and 4656994). rc's CloudRequestEngineTests build engines with a license key, and a license key makes the discovery requests POSTs, which the shared mock in CloudRequestEngineTestsBase answered with the data response, so three of those tests failed in the first full build of the merge (run 43072007057). The mock now answers a POST to the accessible properties or evidence keys endpoint as it answers a GET, and the three tests that count the data request count only that request (72faa2d). Separately, the license key tests' fake cloud looked each request's URL up in a HashMap that both discovery threads wrote to, which failed Process_ResourceKeyAndLicenseKey_SendsBoth in one run (run 43078881682), so it is now a ConcurrentHashMap (4656994).

Effect on existing callers

• A resource key alone sends the same requests as before.
• A resource key with a license key now sends discovery as a POST carrying both keys, rather than a GET carrying the resource key alone. Discovery then lists the products the license key grants as well, which is the gap Authenticate a Cloud Request Engine on a license key alone, and name the properties wanted #12 describes.
• Properties set with setProperty or setProperties are still not sent, so a caller who set them keeps the answer they had. No Java example sets them on the Cloud Request Engine, but two .NET examples do, as short names such as DeviceId. When an earlier head of the .NET draft sent that list as values, its full build (run 42787764085) failed 118 of the 156 cloud web example tests, because the answer then held no device.deviceid. An earlier head of this draft reused setProperty and setProperties in the same way. That is why the list has its own option here too.
• A resource key that is only white space was accepted on rc and sent as is. It now counts as absent, so the build is refused unless a license key is also set. A key with white space around it is now trimmed.
• The keys are now URL-encoded in the request body, as the Evidence values already were. A key made of letters, digits, - and _ is sent exactly as before.

Keys in logs

The license key travels only in request bodies, never in a URL, so no URL this engine logs or puts in an error message can hold it. Process_FailedRequest_LicenseKeyInNoUrlLogOrMessage checks that a good request and a failed one leave the key out of every URL, every log call and every error. Masking a key that the cloud service repeats in its own answer is #85's work, which adds the shared LicenseKeyMask, and this change does not touch those lines. An earlier version of this branch carried its own mask in validateResponse, and it is gone, because it clashed with #85 and showed half of a key of 8 characters or fewer, where logging.md asks for the last 4.

Tests, red and green

The new CloudRequestEngineLicenseKeyTests.java covers every row of the table, the data request for each combination, discovery for a license key alone, a resource key with properties and both keys, the report of properties left out, and the key staying out of URLs, logs and errors. Process_SetProperties_NotSent checks that setProperty and setProperties still send nothing, and Process_RequestedPropertiesAsText_Sent covers the comma separated form. It uses a recording HttpClient and needs no live key. The first targeted red and green runs build and test pipeline.cloudrequestengine on Java 8 and Java 17, and the later ones on Java 8 and Java 21.

• Red, the tests on rc (commit a6376df): run 42794288211. On Java 8 and on Java 17, 14 of the 23 new tests fail, 7 as failures and 7 as errors. setRequestedProperties does not exist on rc, so the red copy calls setProperty and setProperties in its place, which rc collects and never sends. Build_LicenseKeyAndProperties_Accepted, Process_LicenseKeyAlone_SendsLicenseAndValues, Process_RequestedPropertiesAsText_Sent, Discovery_LicenseKeyAlone_PostsCredentialsInTheBody, Process_FailedRequest_LicenseKeyInNoUrlLogOrMessage, Process_PropertyDropped_ReportedOnceAndRequestSucceeds and Process_PropertyPresentButNull_CountsAsAnswered are refused with "A resource key is required". Build_NoCredential_Refused, Build_LicenseKeyWithoutProperties_Refused and Process_SetProperties_NotSent get that message instead of one that explains the 401 or the 400. Build_BlankValuesCountAsAbsent builds with a resource key of two spaces. Discovery_BothKeys_PostsBothInTheBody finds a GET. Process_ResourceKeyAndProperties_SendsResourceAndValues and Process_AllThree_SendsResourceLicenseAndValues find no values.
• Green, the change (commit 9ef779d, the head 6bedc93 plus a temporary workflow): run 42794289207. All 63 pipeline.cloudrequestengine tests pass on Java 8 and on Java 17.
• Red, the review fixes' tests on the merge of rc (commit 46de8b5): run 43022436285. On Java 21, 8 of 26 CloudRequestEngineLicenseKeyTests fail, being the five that send names (the name given twice among them), the property dropped report, the new not-a-property test and the null list. On Java 8 the same 8 fail, and Discovery_ResourceKeyAndProperties_GetsWithTheKeyInTheQuery also errors with a NullPointerException from the fake cloud's unsafe list, which f45eced fixed, so that error is not from this change.
• Green, f45eced: run 43024692676. On Java 8 and 21, all 26 pass.
• Green, head 4656994, both test classes: run 43080111414. On Java 8 and 21, all 38 CloudRequestEngineTests and all 26 CloudRequestEngineLicenseKeyTests pass.

Full pull request build

Run 43084404100, attempt 1, on the head 4656994, which merges rc at 528e0df. Eight of the 9 jobs without macOS passed, being Ubuntu Java 8, 11, 17 and 21, Ubuntu ARM Java 17, and Windows Java 8, 11 and 21. Windows Java 17 ran every test and passed them, both Selenium contract runs 9 of 9 included, and then failed when it uploaded its results, with "Failed to CreateArtifact: Failed to make request after 5 attempts: Unexpected token '<', "<!DOCTYPE "... is not valid JSON", during a GitHub Actions outage from about 20:30 UTC. Every Setting 'PullRequestSha' line, in Configure and in the 9 jobs, is 4656994, and Configure printed "Left out 6 macOS entries". The release build with macOS on Java's final rc head runs every test again.

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

Other drafts

#89 (reserved Evidence names), #84 (missing product), #85 (license key masking) and #86 (eager discovery) change CloudRequestEngineDefault.java and Constants.java, and all four merged into rc on 10 November. #86 renamed the method that makes the accessibleproperties request and set a timeout beside the line this change replaces, and the merge of rc in 3722d66, under Review fixes, carried the POST under a license key across. The head merges rc at 528e0df, which holds all four.

What remains

• A live test in the suite against the service on a license key alone. The measurement above was made by hand on a runner, and the 51Did license key it used answers only 51Did properties.
• The cloud pipeline builders, such as DeviceDetectionCloudPipelineBuilder, pass a license key on but have no option for the properties wanted. The .NET draft adds one to its pipeline builder base. Java's pipeline builders have no such option yet.
• The draft that sends one Evidence name once whatever its letter case, Send one Evidence name once, whatever its letter case, by precedence #107, changes getFormData in CloudRequestEngineDefault.java at other lines, and it merged into rc at 20:26 UTC on 11 November. The head is 4 commits behind rc (Move cxx to the 51degrees-cxx release 4.5.105 and read by index on an engine built with no names #82, Make the data update follow the specification #79, Give every environment variable one _51DEGREES_ name #105 and Send one Evidence name once, whatever its letter case, by precedence #107) and GitHub reports it MERGEABLE and CLEAN, so it merges as it is. The build of rc's head with macOS before rc goes to main proves the two together.