Repository navigation
Send only the Evidence the cloud accepts from the Cloud Request Engine - #89
Merged
Merged
Conversation
…and logs once A resource key can be valid and include no product that holds a cloud engine's properties. The Device Detection, IP Intelligence and Location cloud engines are tested to build with empty property metadata, to log the missing product once, to log and throw nothing when processing responses that hold nothing for them, and to report a property read as missing with ProductNotAccessibleWithResourceKey. The base class tests now expect a missing product to build with empty metadata and log once, keep the failure when the accessible properties are not loaded, and check that a property keyed engine gives no profiles. The Location cloud module gets Mockito as a test dependency for its first tests.
…nce, and process quietly A resource key can be valid and include no product that holds a cloud engine's properties. The Device Detection, IP Intelligence and Location cloud engines then left their property metadata null, logged two errors, and threw on every request when getJSONObject found no part of the response for them. The engines built on CloudAspectEngineBase, such as the hardware profile engines, threw when their metadata was read. CloudAspectEngineBase now has findProduct, which returns the engine's product from the accessible products, or an empty product when the resource key does not include it, logging that once for each engine instance. getElementJson returns the engine's part of the response, or an empty object when the product is missing. The three engines, the base class's own metadata loading and the property keyed base use them, so each builds with empty property metadata, logs one error, and processes a response with nothing for it without logging or throwing. Reading a property then throws PropertyMissingException with ProductNotAccessibleWithResourceKey, as for any property the response does not carry. Accessible products that were never loaded still fail as before. The Location engine's failure message named the Device Detection engine, and now names the Location engine.
…tries only after a failure that can pass CloudRequestEngineDiscoveryTests uses a stand-in for the HTTP client that counts the requests to each endpoint. A build fetches the accessible properties and the evidence keys once each and processing fetches neither again. A 4xx answer to either fails the build with a CloudRequestException carrying the status, once both requests are made, also when the other request failed in a way that can pass. A 5xx answer, a body that cannot be read, a refused connection and a timeout each complete the build with one warning. A list that arrived is kept, processing fetches only what is missing and sends no data request until both are known, the Pipeline's Evidence filter sees the keys a retry fetched, and the configured timeout applies to the discovery requests. The existing tests that expected getPublicProperties and getEvidenceKeyFilter to fetch and throw when the server was unavailable, and a 200 answer holding errors to throw, now expect the build to complete with one warning. The cloud aspect engines are tested to build without logging when the accessible products are not known yet, and to read their metadata once they are.
…se only after a failure that can pass The engine fetched its accessible properties and accepted evidence keys only when something first asked for them, and a failure surfaced wherever that was. The builder now calls discover, which makes both requests. A 4xx answer to either is a definitive configuration error, and its CloudRequestException fails the build once both requests are made. Any other failure, the service unreachable, a timeout, a 5xx answer or a body that cannot be read, is logged as one warning, the build completes and the list that arrived is kept. The configured timeout now applies to both requests. getPublicProperties and getEvidenceKeyFilter make no request. The products are null until they are known. The evidence filter is one CloudEvidenceKeyFilter that includes nothing until the keys are known and reads them on each call, and it derives from EvidenceKeyFilterWhitelist so the Pipeline's filter keeps it rather than copying its keys once. Processing fetches what is missing, evidence keys first, before it sends the data request, and a request that could not be made is reported as its IOException, as for the data request. The Device Detection, IP Intelligence and Location cloud engines, and CloudAspectEngineBase, read their metadata when the products are known rather than logging errors at build and keeping none. A missing product still builds and logs once. The web integration reads a filter that derives from EvidenceKeyFilterWhitelist for the Vary header, as it reads the exact class.
…er one discovered The specification's Resilience section says an engine SHOULD be buildable from a kept copy of the discovery results, the accepted evidence keys and the accessible properties, which depend only on the resource key, so that a Pipeline can be built offline and a host that starts often need not call the cloud service each time. CloudDiscoveryState holds the evidence keys and the accessibleproperties answer, and writes and reads them as one JSON document. CloudRequestEngineBuilder.getDiscoveryState returns the copy after a build that knows both lists, and setDiscoveryState makes a later build use it with no discovery request. The tests are in this commit because they use the new class and methods.
…conds, and that the Pipeline reads metadata again once it is known Each discovery request in the new test waits for the other to start, so they only succeed if they are made at once. A build with no timeout set applies 2 seconds to both, as the page's Configuration options table and .NET give. The Pipeline's map of available properties, built before the Cloud Request Engine knows the accessible properties, holds the cloud aspect engine's properties once they are known. The shared test client now gives each request a connection of its own, as the requests can be made at the same time.
…ult, and read metadata again until it is loaded The two discovery requests are now made at once, as the specification allows, so a service that does not answer delays the build by one timeout rather than two. The evidence keys request runs on another thread while the accessible properties request runs on the building thread, and the build waits for both. The default timeout for each request to the cloud service is now 2 seconds, as the specification's Configuration options table gives and as .NET's CLOUD_REQUEST_TIMEOUT_DEFAULT_SECONDS is. It was 100 seconds. FlowElement gains hasLoadedProperties, true by default, as .NET's aspect engines have HasLoadedProperties. The Cloud Request Engine returns true once both discovery lists are known, the cloud aspect engines once their metadata is loaded, and a parallel group once all its elements have. PipelineDefault keeps its map of available properties only once every element has loaded its properties, and builds it again on each call until then, as .NET's Pipeline.ElementAvailableProperties does after a PropertiesNotYetLoadedException. The web integration works out the Vary headers again on each response until every element has loaded, as .NET's ClientsidePropertyService.HeadersAffectingJavaScript does after an exception from an element's EvidenceKeyFilter.
…lement has loaded its properties With every element loaded, the configuration built from the Pipeline's available properties is built once and kept across requests. While an element has not loaded its properties, it is built again on each request, and kept once every element has loaded. Both fail on the code before the fix, where the configuration was built again on every request, because getOrDefault never stored it.
…aded its properties The SetHeaders element built its configuration from the Pipeline's available properties on every request, because getOrDefault never stored it. It now keeps it once every element of the Pipeline has loaded its properties, and builds it again on each request until then, as after a cloud element could not reach its service when the Pipeline was built. So the configuration gains the cloud's SetHeader properties once a retry fetches them, and is then built once. This is the same rule the Pipeline's map of available properties and the web integration's Vary header follow.
…eats raw types as errors
…accepts The Processing section of the cloud request engine specification says the engine sends only the Evidence keys the cloud service needs, and never Evidence whose key strips to resource, license or values. Two new tests check the posted request. process_SendsOnlyAcceptedEvidence sends Evidence the service does not list and expects only the listed keys, compared without regard to case. process_LeavesOutEvidenceNamedResourceLicenseOrValues sends Evidence named resource, license and values in several cases, with and without a configured license key, and expects the configured keys once each and nothing else. Existing tests that check the request now answer the evidencekeys request with the keys they send, as the cloud service would.
The Cloud Request Engine sends only the Evidence whose keys the cloud service accepts, from its evidencekeys request, and never Evidence whose key strips to resource, license or values, compared without regard to case, as the Processing section of the cloud request engine specification says. The request carries the configured resource key, and license key where one is configured, once each. Before, the engine sent all Evidence except the web request. Conflicts between prefixes are now reported only among the Evidence being sent.
…s warning as an error
…s warning as an error
…s warning as an error
thenReturn(null, products) uses the varargs overload, and a generic Map array is an unchecked warning that the build treats as an error, so each stub now chains two single-value thenReturn calls.
…d-request-engine-sent-evidence
…e build The specification now treats a 408 or a 429 answer to a discovery request as a transient failure, because the same request can succeed later, so 429 moves from the cases that fail the build to those that do not, and 408 cases join it.
… build A build fails on a 4xx answer because no retry with the same configuration can succeed, which is not so for a 408 or a 429, as RFC 9110 section 15.5.9 and RFC 6585 section 4 say, so those two are logged as a warning and retried on first use like a 5xx.
…d-request-engine-sent-evidence
The Processing section of the cloud request engine specification now also leaves out Evidence whose key strips to licence, because the cloud service reads the license key from licence where license is absent. process_LeavesOutEvidenceNamedLicence lists query.licence and cookie.LICENCE as accepted, adds Evidence under each, and expects only the configured keys and User-Agent, with and without a license key. It is written to fail on the previous commit, which sends both as licence and LICENCE.
Evidence whose key strips to licence, compared without regard to case, is now left out of the cloud request along with resource, license and values, as the Processing section of the cloud request engine specification says, because the cloud service reads the license key from licence where license is absent.
…fail the build" This reverts commit 7bc481c.
… fail the build" This reverts commit 40452b7.
…d-request-engine-sent-evidence
…no error The Start-up activity section of the cloud aspect engine specification on rc says the engine "MUST log the missing product once per Cloud Aspect Engine instance, at warning level, naming its Data Key". The Processing section says "Where the response JSON has no portion for the Engine's Data Key, the Cloud Aspect Engine leaves the Element Data without values and raises no error." The missing product tests now expect one warning and no error. ProductPresent_ResponseWithoutPart_NoValues, for each of the Device Detection, IP Intelligence and Location cloud engines, includes the engine's product and processes a response with no part for it, and expects no exception and a property without a value. propertyKeyed_productPresent_noProfiles does the same for a property keyed engine, with a response that has no part for it and one whose part has no profiles. All are written to fail on the previous commit, which logs an error and throws on a missing part when the product is present.
… as no values The Start-up activity section of the cloud aspect engine specification on rc says the missing product is logged "once per Cloud Aspect Engine instance, at warning level, naming its Data Key". findProduct now logs it with logger.warn. The Processing section says "Where the response JSON has no portion for the Engine's Data Key, the Cloud Aspect Engine leaves the Element Data without values and raises no error." getElementJson now returns an empty object whenever the response has no part for the engine, whether or not the product is included, and PropertyKeyedCloudEngineBase reads profiles with optJSONArray, so a part without profiles gives none. The productMissing flag and isProductMissing, which only chose between those two behaviours, are removed.
The specification on rc makes every 4xx answer to a discovery request a definitive configuration error, so the cases that fail the build now include 408 beside 429.
A 4xx answer is a definitive configuration error whatever its body holds, such as an error page from a proxy. The accessibleproperties answer is read as JSON before its status is used, so a 403 with an HTML body there fails this test, as the build completes with a warning.
An answer that cannot be read as JSON now raises a CloudRequestException carrying its HTTP status, as .NET does, so a 4xx answer to a discovery request fails the build whatever its body holds, as the specification says.
…d-request-engine-sent-evidence
This was referenced Nov 10, 2026
A comment-only change. The paragraph that describes findProduct left "returned, so the engine builds" on a short line of its own, so the paragraph is now wrapped at 80 columns. No code changes.
The one conflict is in CloudRequestEngineDefault.java, where rc adds getLicenseKeys() in the place where this branch adds setTimeouts(HttpURLConnection) and renames getCloudProperties to fetchPublicProperties. Both helpers are kept, and the accessible properties request keeps this branch's name, fetchPublicProperties, which every call site uses.
This branch carried the missing product change as it was at 33a13fe. Its head, 9fced9b, logs a missing product as a warning, treats a missing response part as no values, and removes isProductMissing. The conflicts are in the Device Detection and IP Intelligence ProductMissingTests and in GeoLocationCloudEngineTests. Each takes the file as it is at 9fced9b and keeps this branch's DiscoveryNotComplete_LoadsTheMetadataLater test above buildEngine.
jwrosewell
changed the base branch from
fix/cloud-request-engine-eager-discovery
to
rc
November 10, 2026 10:34
The eager discovery branch now has rc merged in, with the license key masking, and the missing product change at 9fced9b. Both merge into this branch with no conflict.
jwrosewell
marked this pull request as ready for review
November 10, 2026 13:49
jwrosewell
marked this pull request as draft
November 10, 2026 16:01
jwrosewell
marked this pull request as ready for review
November 10, 2026 16:28
This was referenced Nov 10, 2026
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #86, because the change reads the accepted Evidence keys through the
CloudEvidenceKeyFilterthat #86 adds, and relies on #86 fetching those keys before any data request is sent. This pull request now targetsrcand carries #86's head, 5a6fc84, so until #86 merges it also shows #86's commits, and with them #84's. It merges after #86. The head is b244e5a.The change
The Cloud Request Engine sends only the Evidence the cloud accepts, and never Evidence named resource, license, licence or values, as the Processing section says.
Before this change,
CloudRequestEngineDefault.getFormDatacopied every item of Evidence in the FlowData into the request, leaving out only the web request (server.http-request). The list of keys from the evidencekeys request was used for the engine's evidence key filter but not to choose what was sent, and Evidence whose key strips toresource,license,licenceorvalueswas sent as another form field of that name.What the specification says
From the Processing section of the Cloud Request Engine page on the specifications'
rc, as 51Degrees/specifications#125 left it when it merged as 425a6bd. The text below is fromrcat d729b7d.What changed
getFormDatanow keeps only the Evidence that the newisSentallows, before the precedence order picks one value for each name.isSentallows a key only when the evidencekeys list holds it, compared without regard to case, and only when the name left after the prefix is not one ofENGINE_FIELDS, beingresource,license,licenceandvalues, also compared without regard to case.licenceis there because the cloud service reads the license key fromlicencewherelicenseis absent. The web request is still never sent, even where the list holds its key. A key with no., which has no prefix to strip, is not sent either.getContentis unchanged.The data request is only made once the evidencekeys list is known, which #86 ensures in
completeDiscovery, so the list is always there whengetFormDataruns.The Evidence keys written in the Java sources were checked against the list that
https://cloud.51degrees.com/api/v4/evidencekeysreturned on 10 November 2026, at this branch's head b244e5a, so that no example or engine loses Evidence it needs..javafiles underexamples,device-detection,ip-intelligenceandlocationthat starts with a known prefix was collected withgit grep -h -o -i -E '"(header|query|cookie|server|fiftyone|location|session)\.[A-Za-z0-9_.-]+"' b244e5a -- 'examples/*.java' 'device-detection/*.java' 'ip-intelligence/*.java' 'location/*.java', giving 29 different keys in lower case.header.usage-from, whichShareUsageExamplesets for usage sharing.header.irrelevant-headerandserver.irrelevant-header, which the device detection and IP intelligenceProcessTestsadd on purpose as Evidence that no engine reads.fiftyone.pipeline.web.mvcandfiftyone.geolocation.web.examples.mvc.controller, which are Java package names in the@ComponentScanof the location MVC example'sExampleMvcConfig, not Evidence keys.Compared with Go
The Go change, 51Degrees/51degrees-go#85, is merged into Go's
rc, with its head fecae24 and the merge commit f23ad2e. It applies the same two rules inresolveEvidence, and itsengineFieldsholds the four namesresource,license,licenceandvalues.For the release notes
resource,license,licenceorvalues, in any case, is never sent. The request carries only the resource key and license key the engine was configured with.How to verify
No new test calls a live service.
CloudRequestEngineTests2f6a4de holds the tests alone, on Discover when the Cloud Request Engine is built, and retry on first use only after a failure that can pass #86's code. These are written to fail there:
process_SendsOnlyAcceptedEvidence, which listsquery.User-Agentandheader.Accept-Languageas accepted, adds three more items of Evidence and expects onlyresource,User-Agentandaccept-languagein the request. On Discover when the Cloud Request Engine is built, and retry on first use only after a failure that can pass #86's code the request also carriesX-Other,sessionandclient-ip.process_LeavesOutEvidenceNamedResourceLicenseOrValues, one with no license key and one with a license key. The accepted list holdsquery.resource,header.Resource,query.license,cookie.LICENSEandquery.values, Evidence is added under each, and the test expects exactlyresource=resourcekey, thenlicense=ABCDEFGwhere configured, thenUser-Agent=iPhone. On Discover when the Cloud Request Engine is built, and retry on first use only after a failure that can pass #86's code the request also carries the Evidence values underresource,Resource,license,LICENSEandvalues.The same commit has the existing tests that check the request answer the evidencekeys request with the keys they send, as the cloud service would, through a new
acceptEvidencehelper. These areevidencePrecidence,getFormData,getFormData_LeavesOutTheWebRequest,getFormData_KeepsDotsInEvidenceNameandprocess_PostsWholeEvidenceName, and they pass on Discover when the Cloud Request Engine is built, and retry on first use only after a failure that can pass #86's code as well as on this branch.getFormData_LeavesOutTheWebRequestnow listsserver.http-requestas accepted, so it still shows the web request is left out by name.0155c33 holds the fix.
684a837 and f522ddc merge Discover when the Cloud Request Engine is built, and retry on first use only after a failure that can pass #86's later commits into this branch.
c2a5e9c holds one more test alone, written to fail on f522ddc. Both cases of
process_LeavesOutEvidenceNamedLicencelistquery.licenceandcookie.LICENCEas accepted, add Evidence under each, and expect exactlyresource=resourcekey, thenlicense=ABCDEFGwhere configured, thenUser-Agent=iPhone. On f522ddc the request also carries the Evidence values underlicenceandLICENCE.97a73df adds
licencetoENGINE_FIELDS.f755cb5 and 38bccd2 merge Discover when the Cloud Request Engine is built, and retry on first use only after a failure that can pass #86 at 19cc0fa and at 7d4cebd into this branch.
b244e5a merges Discover when the Cloud Request Engine is built, and retry on first use only after a failure that can pass #86's head, 5a6fc84, which now holds
rcwith Show a license key only by its last 4 characters in logs and error messages #85 (license key masking) and Build a cloud engine whose product the resource key lacks, and log that once #84's head 9fced9b. It merges with no conflict and deletes no file. Against 5a6fc84 this branch changes onlyCloudRequestEngineDefault.javaandCloudRequestEngineTests.java.Run 42729045219 runs each set of tests on the code before its fix, in two jobs, and each job fails as expected.
process_SendsOnlyAcceptedEvidenceand both cases ofprocess_LeavesOutEvidenceNamedResourceLicenseOrValues.process_LeavesOutEvidenceNamedLicence, because the request also carrieslicence=evidence-licenceandLICENCE=cookie-licence.Full build
The full build and test matrix passed every job other than macOS on head b244e5a in run 42800964739 on 10 November 2026, 11 jobs succeeded, run as a dry run so nothing merged. 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.