Repository navigation
Send requested properties beside a resource key, with or without a licence key - #89
Merged
Merged
Conversation
…r without a licence key
…cence key The engine refused requested_properties whenever a resource key was set, because the cloud service was thought to ignore the list. The service adds the listed properties to those the resource key carries, so the engine now builds and sends the list as values, and the licence key too where there is one. It still refuses no key at all, and a licence key with no resource key or list, which the service refuses on every request. The message for properties an answer leaves out now says they are not covered by the keys in use, as a resource key can be the only one.
This was referenced Nov 10, 2026
… and drop history from two comments
This was referenced Nov 10, 2026
Merged
… stale claim about discovery .NET, Java, Node.js and PHP all word the warning as not covered by the keys in use, because the licence behind a resource key and a licence key beside it both decide what comes back. The name not known warning gains the same closing line and the spelling advice those languages give. The discovery comment no longer says a licence key beside a resource key widens the answer, which the corrected Accepted combinations table replaced. Tests assert both wordings.
…rite The report compares the answer with the list it asked for, and an answer that parses as JSON but is not an object has nothing to compare, so the test pins that the request still succeeds and nothing is written. The warning for a name that is not a property is asserted as .NET and Java word it, both of them carrying the same sentence.
…ET and Java's wording An answer that parses as JSON but is not an object has no sections to look in, so the report returned an AttributeError out of processing and failed a request that had succeeded. It now leaves such an answer alone, with the same guard and the same wider catch that is_cacheable uses on the same text. The warning for a name that is not a property is now word for word the sentence .NET and Java both write, so Python's two warnings match theirs exactly.
jwrosewell
marked this pull request as ready for review
November 12, 2026 15:10
jwrosewell
added a commit
that referenced
this pull request
Nov 12, 2026
Takes rc at 435274d, which now holds #69 (squashed as 8b0b624), #70, #59, #89 and #106. Three files conflict, being datafile_update_service.py, test_datafile_update_http.py and test_datafile_update_service.py, each resolved by keeping this branch's version. rc's version of each of the three is identical to #69's head e770a88, which this branch already took in at efbfb6d, so nothing rc brings to those files is lost. In __emit, rc logs the unhandled error inline and this branch calls _log_unhandled, which writes the same sentence with the same masking. Every other file rc changed takes rc's version, including the comment #70 changed in the device detection datafileupdate_console.py example.
jwrosewell
added a commit
that referenced
this pull request
Nov 12, 2026
Takes rc at 435274d, which now holds #70, #69, #59, #89 and #106. sequenceelement.py conflicts, because #70 and this branch both give the Sequence Element Element Data. It takes rc's version whole. rc's process_internal publishes the same two values under the same data key "sequence" and the same names "session-id" and "sequence", being the pair it writes to Evidence, and also declares both properties and validates each value. So this branch's 13 lines there are no longer needed, and the JavaScript Builder reads rc's Element Data unchanged. javascriptbuilder.py merges without a conflict and keeps both changes, rc's escaping of the JSON written into the script and this branch's reading of the session id and the sequence from the Element Data. test_javascriptbuilder_sequence.py also merges without a conflict and keeps rc's too_long case and this branch's module docstring.
jwrosewell
added a commit
that referenced
this pull request
Nov 12, 2026
* Point the documentation build at the example folders and package (#23)
* Point the documentation build at the example folders and package
List the cloud request engine examples and the device detection examples
source folder in EXAMPLE_PATH, so Doxygen finds the files the examples
name. Include each client hints example's template from its own folder,
and read the location package from src/fiftyone_location.
* Keep the cloud web example doc comment unbroken and name its view by folder
* Keep the onpremise web example doc comment unbroken and name its view by folder
* Point links to the archived repositories at where the code now lives (#25)
* Point links to the archived repositories at where the code now lives
The examples, readmes and example web pages linked to the archived
Python repositories, and the IP Intelligence readmes linked to the
archived IP Intelligence C repository. Each link now points at the same
file or folder in 51degrees-python, or at the IP Intelligence readme in
51degrees-cxx, so readers land on code that is still maintained.
Links to a file follow the file to its folder in this repository, for
example the Device Detection examples now sit under
device-detection/fiftyone_devicedetection_examples/src. Links that used
the old version/4.5 branch now use main.
The match metrics example closed its Markdown link with a square bracket
instead of a parenthesis. The link now closes with a parenthesis. The
cloud examples readme changes into the cloned folder after cloning, so
its cd line now follows the new clone address.
The history table in the root README still names the earlier
repositories, because their tags and releases stay there. The links to
the data repositories are unchanged, because those repositories are
still current.
* Describe the IP Intelligence performance test gap without old repository names
* Generate both SWIG wrappers again for 51degrees-cxx 4.5.102 (#27)
* Add a temporary workflow that generates the SWIG wrappers for 51degrees-cxx 4.5.102
* Name the SWIG modules in the temporary workflow instead of reading them
* Generate both SWIG wrappers again for 51degrees-cxx 4.5.102
Between 4.5.100 and 4.5.102, 51degrees-cxx removed the two
EngineHash process overloads and the three EngineIpi process
overloads that take required property indexes. The committed
wrappers still called them, so neither on-premise package compiled
against 4.5.102, which the nightly submodule update brings in #26.
This moves the cxx submodule from 4110d681, the 4.5.100 release, to
1bec7b57, the 4.5.102 release, and generates hash_python_wrap.cxx,
DeviceDetectionHashEngineModule.py, ipi_python_wrap.cxx and
IpIntelligenceEngineModule.py again with SWIG 4.4.1 built from its
v4.4.1 tag. Run against 4110d681, the same command gives files byte
for byte the same as the ones committed before, so every difference
here comes from cxx. Both engines call only process(evidence), which
cxx keeps.
* Remove the temporary SWIG wrapper workflow
* Merged Pull Request '#26 update-submodules/rc->rc: Update submodules'
Co-authored-by: Automation51D <51DCI@51Degrees.com>
* Read the data files from the data repository in cxx/data (#28)
* Say in AGENTS.md how to get the data, build and run the examples (#34)
* Point package metadata and package READMEs at the right pages (#35)
* Ask the country property for its no value message in the location example (#32)
* Describe the rc approach to releases in AGENTS.md (#29)
* Check the callback URL before the JavaScript builder writes it (#31)
* Let FlowData and pipelines release native memory, and close them in the tests and examples (#36)
* Look for the resource keys link the cloud engine now gives in the cloud tests (#38)
* Allow only http and https in the callback URL, and test the zone and port rules (#40)
* Test every package against the packages it depends on in this checkout (#41)
* Use a name in the object name test that a session id cannot contain (#44)
* Run every example on the free data files, and show what needs the Enterprise data file (#43)
* Build the released packages when a nightly publish has nothing to release (#45)
* Share only the evidence usage sharing accepts unless asked for all (#50)
* Fail the cloud tests when no resource key is set, and run the Cloud 51Did workflow (#58)
* Fail the cloud tests when no resource key is set, and run the Cloud 51Did workflow
Tests that exist to call the live cloud service skipped when no resource
key reached them, and the 51Did live tests also skipped when the answer
lacked what they checked, so a run with no key or with a key lacking a
product reported success without the cloud being called.
- Each test folder that calls the live service has the same live_cloud.py,
which reads _51DEGREES_RESOURCE_KEY, then _51DEGREES_RESOURCE_KEY_BESPOKE,
then resource_key, and gives the failure messages. Every skip for a
missing key in the 51Did, Device Detection cloud, Device Detection
example, IP Intelligence example and pipeline example tests is now a
failure naming the variables, the product the key must carry and how to
see what a key carries.
- Where the 51Did answer lacks an identifier, the failure gives the cloud's
null reason and the product. A host without the creator context fails.
Without a licence key the redeem tests check the sealed result redeems as
unreadable.
- test_did_tier_live.py checks what a Basic key gets, being a 51Did for
non-marketing and a refusal naming CloudV5FODiD for standard and
personalized.
- The live tests are marked, the unit steps deselect them through
PYTEST_ADDOPTS, and the pipeline integration step now runs the 51Did
live tests. The integration scripts fail when no TestResourceKey is
passed, and the Device Detection one keeps the cloud package in the run
when 51Degrees.csv is missing.
- cloud-51did.yml moves to the root .github/workflows, where GitHub reads
it, with its paths corrected. It fails when no key reaches it, and runs
the Basic key only through the tier test.
The licence-key tests in the pipeline examples and test_licensekey_live.py
still skip, because CI has no cloud licence key yet (#13).
* Wrap the tox.ini comment about the cloud test keys
* Make the example profile comments and the Lite file name match the code (#65)
- The IP Intelligence data file update example now names the LowMemory
profile for the pipeline that downloads and reloads the Enterprise data
file, rather than taking the engine's default.
- The Device Detection getting started example's comment said it uses the
low memory profile, but the code sets MaxPerformance. The comment now
says MaxPerformance, and why it suits the free Lite data file.
- The IP Intelligence performance example's header said worker processes
share a memory mapped data file under LowMemory, as if that were the
default. It now says the default MaxPerformance loads the whole file
into each worker, and that -p LowMemory lets the workers share the
operating system's file cache.
- The IP Intelligence package readme named the Lite data file
51Degrees-LiteIpiV41.ipi, which is CI's name for it. The data
repository's scripts save it as 51Degrees-LiteV41.ipi.
* Use the Balanced performance profile by default for IP Intelligence (#62)
The IP Intelligence on-premise specification gives Balanced as the
default performance profile, and the C library's default IP Intelligence
configuration is the Balanced one, but the Python engine defaulted to
LowMemory. The engine now defaults to "Balanced", which its profile
switch already maps to the native Balanced configuration, and the
docstrings in the engine and both builders say so.
Balanced loads some collections into memory and caches others, where
LowMemory reads everything from the data file, so a caller relying on the
default uses more memory. Naming "LowMemory" keeps the previous behaviour.
A new test, on the free ASN data file, compares the collection settings
of the configuration an engine built at its defaults gives the native
code with those of each profile, because the engine keeps no name for the
profile in use.
* Accept the specification's translation option names, and the behavior as text (#64)
* Accept the missing translation behavior as text, and refuse an unknown one
TranslationEngineBase and Translator stored the behavior they were given
without checking it, and compared it with the MissingTranslationBehavior
members. A configuration file can only give text, so a configuration
naming "EmptyString" or "flow_error" matched no member and a missing
translation kept its original value, with nothing to say the setting had
been ignored.
MissingTranslationBehavior.from_value now returns a member as it is, and
turns text naming a member into that member. It takes the
specification's names (Original, EmptyString, FlowError) and the members'
own names and values, ignoring case and underscores, and raises
ValueError for anything else. The engine and the translator use it, so
an unknown value stops the engine being built. The parameter is still
named behavior.
* Accept the specification's name for the missing translation behavior
The specification names the Translation Engine's options
SourceElementDataKey and MissingTranslationBehavior. Python already took
source_element_data_key, but named the second option behavior.
TranslationEngineBase and Translator now take missing_translation_behavior.
behavior still works as a keyword-only deprecated alias that raises a
DeprecationWarning, and when both are given missing_translation_behavior
wins. Neither given means Original, as before. A value given by position
is taken as missing_translation_behavior, as it was taken as behavior.
The IP Intelligence translation engines now pass every option by its name,
so they need a fiftyone_pipeline_translation that takes the new name. Their
package requires fiftyone_pipeline_translation 4.5.43 or later, the
version the release plan gives this commit.
* Leave the IP Intelligence translation engines as they were on rc
The IP Intelligence translation engines pass the missing translation
behavior by position. A value passed by position is taken as
missing_translation_behavior by the new translation package and as
behavior by the old one, so the engines work with either release and
raise no deprecation warning without any change. Passing it by name
needed a minimum version of fiftyone_pipeline_translation that cannot be
known until the release that carries the new name, so the engines and
their package's requirements are back to their rc content.
* Take None as not given for the missing translation behavior
rc treated None as Original, because the translator took any value that
was not EMPTY_STRING or FLOW_ERROR as Original. Refusing it would stop a
configuration holding null, or code passing an unset option on as None,
from building after a patch release. The repository's other engines use
None to mean not set, so the translation engine and Translator now do the
same. Both parameters default to None, None under either name gives
Original without a warning, and anything else that names no member is
still refused.
* Package text says what each package does in the words people search for (#68)
Name the competitor brands people search for in the remaining package text. Text only, no code change.
* Check the callback URL as the specification's six rules say (#66)
* Check the callback URL as the specification's six rules say
Refuse an IPv6 zone identifier, an '@', '?' or '#' anywhere in the host value, and a name ending in a number unless it is an IPv4 address in dotted decimal form. The logged URL shows *** in place of everything in the host before its last '@'.
* Test that a port which is present is not empty
* Refuse a port that is present but empty
A host such as https://example.com otherwise reads as the host https with an empty port. RFC 3986 section 3.2.3 allows an empty port, but section 6.2.3 treats it as equal to none, so the callback URL does not carry one.
* Test that a licence key is masked in logs and errors
* Show only the last four characters of a licence key in logs and errors
Each hidden character is an asterisk, and a key of four or fewer characters is
hidden entirely. The update service masks the URL it logs and the message of a
failed request, and the Cloud Request Engine masks the URL in its no-data
message.
* Refuse a 51Did text with white space inside it, or both base64 alphabets (#56)
* Refuse a 51Did text with white space inside it, or both base64 alphabets
The 51Did specification says a reader ignores white space before and
after the text, being any character with the Unicode White_Space
property, refuses any other character outside the alphabet, white space
inside the text included, and refuses a text holding a character from
each alphabet, checking that before it maps "-" and "_" onto "+" and "/".
FodId.to_standard_base64 now removes exactly the White_Space characters
around the text, and answers None for a text holding any other character
outside the two alphabets and the padding, or holding a character from
each alphabet. The base64 readers report InvalidBase64 for such a text.
White space alone still reports MissingInput.
Before this change the reader read a mixed text as if it were clean, and
read a text with U+001C to U+001F around it, because str.strip() removes
those characters although they are not White_Space. White space inside
the text was already refused by the OWID decoder.
The client's length check and to_base64_url trim the same characters.
* Say the alphabet check is made separately in the fod_id comment
* Use the specification's defaults for the performance profile and Update Matched User Agent (#57)
* Leave Update Matched User Agent on by default, as the native code sets it
The Device Detection on-premise specification says the option is on by
default, so the UserAgents property holds the matched characters of the
evidence. The C library sets it on, but the Python engine took
update_matched_useragent=False and always passed it on, so UserAgents was
a list of empty strings unless a caller turned the option on.
The engine now takes None and only sets the option when a value is given,
so the native default applies, as the .NET builder does. The docstrings in
the engine and both builders say the option is on by default, and the
match metrics example's comment no longer says it has to be set.
A new test builds a pipeline on the free Lite data file with the option
left unset, set on and set off, and skips when that file is not there.
* Use the Balanced performance profile by default, as the native code does
The Device Detection on-premise specification gives Balanced as the
default performance profile, and the C library's default configuration is
the Balanced one, but the Python engine defaulted to LowMemory. The engine
now defaults to "Balanced", which its profile switch already maps to the
native Balanced configuration, and the docstrings in the engine and both
builders say so.
Balanced loads some collections into memory and caches others, where
LowMemory reads everything from the data file, so a caller relying on the
default uses more memory. Naming "LowMemory" keeps the previous behaviour.
A new test compares the collection settings of the configuration an
engine built at its defaults gives the native code with those of each
profile, because the engine keeps no name for the profile in use.
* Say which profiles the default profile test cannot tell apart
MaxPerformance and LowMemory have the same collection settings, as do Balanced and BalancedTemp, so the test's comparison of collection settings cannot separate either pair. The test's docstring now says so.
* Say what the matched User-Agent length limit does, as the C header does
The max_matched_useragent_length docstrings in the engine and both
builders said the option is ignored when update_matched_useragent is
false, and named it max_matched_useragents_length. The C library's header
(src/dd/config-dd.h) says the limit applies to a single header, a pseudo
header and the matched User-Agent, that a longer value is ignored and
detection moves on, and that the limit applies whether or not the matched
User-Agent is recorded. The docstrings now say the same, give the native
default of 500, and spell the option's name correctly.
* Drop the cloud builder's false matched User-Agent settings
The cloud builder's docstring listed max_matched_useragents_length,
misspelled, as ignored when update_matched_useragent is false, which the
C header contradicts. The cloud builder reads neither setting, so both
entries are removed rather than corrected.
The match metrics example comment now says the option is set to show
where to turn it off, as the line sets it to True.
* Read a 51Did factor word the package does not list as a mismatch (#75)
* Test that an unknown 51Did factor word is a mismatch
* Read a 51Did factor word the package does not list as a mismatch
* Test that a 51Did factor word is read without regard to case
* Compare a 51Did factor word without regard to case, as the specification says
* Accept only the evidence each element reads, and none by default (#63)
* Test that the elements the builder adds accept only the evidence they read
* Accept only the evidence the sequence element, the JavaScript builder, the JSON bundler and the set header element read
* Keep the evidence the cloud request engine tests need with an element that accepts every key, and test what is no longer sent
* Describe the reserved evidence test's element without a claim about the live service
* Test that a cloud engine accepts no evidence and that a cloud pipeline sends only what the service lists
* Accept no evidence in a cloud engine, as it reads only the cloud request engine's results
* Test that an element which declares no evidence accepts none
* Accept no evidence in an element that declares none, as the specification asks
* Wrap the import and fix the blank lines in the default elements test
* Take 51degrees-cxx rc cfad8978 and generate the IP Intelligence wrapper again (#72)
* Take 51degrees-cxx rc 865cee97 and generate the IP Intelligence wrapper again
Moves the cxx submodule from e866f6d3, 4.5.103, to the 51degrees-cxx rc
commit 865cee97.
Generated hash_python_wrap.cxx, DeviceDetectionHashEngineModule.py,
ipi_python_wrap.cxx and IpIntelligenceEngineModule.py again with SWIG
4.4.1, the version in the committed files, from the staged source the
way PreBuild.sh does. Run against e866f6d3 the same command gives
files byte for byte the same as the committed ones, so every
difference comes from cxx.
Only ipi_python_wrap.cxx changes. The four ConfigIpiSwig getters that
return a CollectionConfig no longer pass SWIG_POINTER_OWN, because the
cxx interface now scopes its newobject directives to MetaData and the
configuration keeps ownership of those references. The Device
Detection wrapper and both module files are unchanged.
The setup.py files list the cxx sources by glob, so the new
src/common/date.c is picked up with no change there.
* Move cxx to the 51degrees-cxx rc commit b4f769ad
Takes the eight 51degrees-cxx pull requests merged into rc after 865cee97, among them the rename of src/common/Date.cpp to date_pp.cpp so it no longer clashes with date.c on a case-insensitive file system.
* Move cxx to the 51degrees-cxx rc commit cfad8978
Takes 51degrees-cxx #130, which writes the text of each IP Intelligence value with its weight in C, and #146, which gives a reason and a message when a property has no value in the matched results.
* Test that the cloud's errors text shows only the masked licence key
* Show only the last four characters of the licence key in the cloud's errors text
* Accept no evidence for usage sharing, and share what it finds in the flow data and the web request (#61)
* Test that usage sharing accepts no evidence and still shares what it finds
* Accept no evidence for usage sharing, and share what it finds in the flow data and the web request
* Check the tracker reads the session cookie the element no longer accepts
* Test the request view's parts and that the Cookie header is never shared
* Never share the Cookie header, and give the request view its client IP, protocol, method, path and session values
* Give the cookie in the request view tests a value Flask reads
* Read the session through the object behind Flask's proxy
* Test that session numbers and booleans reach the evidence as text, and wrap two test lines
* Add session numbers and booleans to the evidence as text
* Write the JSON Builder element and meta-property keys in lower case (#77)
* Test that the JSON Builder writes keys in lower case
* Write the JSON Builder's element and meta-property keys in lower case
* Store the lower case test element's data under the lower case key
* Set the mixed case test element's data through set_element_data, as a real element does
* Keep Element Data under the data key in lower case, which is how FlowData.get reads it
* Send only the Evidence the cloud accepts, never resource, license, licence or values (#79)
* Test that reserved names in Evidence are not sent in the request
* Send only the Evidence the cloud accepts, never resource, license, licence or values
* Keep the other evidence for the engine in the reserved evidence helper, now that no default element accepts it
* Wrap the reserved evidence helper's pipeline line
* Write a WKT value to JSON as an object holding the text, and read it back (#82)
* Test that a WKT value is an object in JSON
* Write a WKT value to JSON as an object holding the text, and read it back
The JSON Builder writes a shape in well-known text as an object with one member
value, and the Cloud Engine gives the text from such an object.
* Key the WKT test's property list by the names the cloud service gives
* Find a WKT property in the cloud property list by its name in any case
* Build a Cloud Aspect Engine whose product the key lacks, and log it once (#74)
* Test that a Cloud Aspect Engine with no product builds and logs once
* Build a Cloud Aspect Engine whose product the key lacks, and log it once
The engine builds with empty Property metadata and logs the missing product
once, where it raised and failed the Pipeline build. The Location engine's own
check that raised is removed.
* Test that the missing product is logged at warning level
* Log a Cloud Aspect Engine's missing product at warning level, as the specification says
* Refuse the Window property names the specification lists as the object name (#73)
* Test that the object name refuses the Window properties the specification lists
* Refuse the Window property names the specification lists as the object name
* Say in the object name comment why eval and arguments are refused
* Fail the JSON Builder when no Sequence Element has produced a sequence number (#76)
* Test that the JSON Builder needs a sequence number
* Fail the JSON Builder when no Sequence Element has produced a sequence number
* Build the JavaScript script tests on a pipeline with a Sequence Element
The JSON Builder fails without a sequence number, so a pipeline without a
Sequence Element no longer reaches the script. The builder's reading of text
values stays covered by the get_sequence tests.
* Test that each on-premise web example's configuration gives the JSON Builder a sequence number
* Add the Sequence Element to the User-Agent Client Hints web example's configuration
* Keep the caller's integer sequence in the JSON Builder tests with an element that accepts it, now that no default element does
* Write a list of records in the JSON Builder (#85)
* Test that the JSON Builder writes a list of records
* Write a list of records in the JSON Builder
A Property whose value is a list of Element Data is written as a list of
objects. A Property with no value is null beside its nullreason, and a null that
was not assigned no value has no entry. The list branch no longer replaces the
output with its own list, and an empty list is written as an empty list.
* Test that a top-level null not assigned no value has no entry in the JSON Builder
* Leave a top-level null not assigned no value out of the JSON, as the specification says
* Test that a null JavaScript Property is still listed and a no value with no message has a text reason
* List a null JavaScript Property, and give a no value with no message the reason Unknown
* Mark a usage sharing value with invalid characters as escaped, as the specification says (#71)
* Test the usage sharing record attributes
* Mark a value with invalid characters as escaped in the usage sharing record
* Expect escaped in the usage sharing invalid character test
* Test the usage sharing record escapes and limit
* Write invalid characters as backslash x and four hex digits, and cut values at 10,000 characters
A character XML 1.0 does not allow is written as its code in four lower-case
hexadecimal digits after a backslash and x, and the element is marked escaped.
A value is cut to 10,000 characters (code points) before the markup characters
are written, and the element is marked truncated.
* Test that escapes do not count towards the usage sharing length limit
* Test that the session id, client IP and names leave the usage record parseable, and the date and warning
* Write the session id, client IP and Name attribute through the record's escaping, leave out entries with no XML name, and give the date in UTC
* Import re apart from the imports the back-off draft adds
* Test that the session id and client IP carry no attributes
* Write the session id and client IP with no escaped or truncated attribute, as the record gives them none
* Merged Pull Request '#96 update-submodules/rc->rc: Update submodules'
Co-authored-by: Automation51D <51DCI@51Degrees.com>
* Read each on-premise property by its index in the build-time names (#93)
* Test that Element Data reads each property by its build-time index
* Test with the native engines that a property keeps its build-time index, and time a read by name and by index
* Read each on-premise property by its index in the build-time names, and build an engine given no names with the data set's names
* Make the index table from the build-time names, count reads by set, and time building with every name
* Register each engine before the native tests read it, and close each flow data they make
* Test that an engine built with names keeps each build-time index, and that an engine built with no names reads by name as before
The tests cover an engine built with a list of names in an order that
differs from the data set's, and a list that is a subset of what the data
set holds, which is the superset case. Each name keeps its place in the
list given, and reads the same value as an engine built with every name.
A read by name, a read by index and the typed read are checked to give
the same value, and their times are printed.
An engine built with no names is checked to have an empty table and to
keep reading by name, which is what it does today.
* Keep an engine built with no property names reading by name
An engine built with no names was built with every name in the data set,
read first from a low memory engine, so that its properties had fixed
indexes too. Processing was measured at 3 to 7 times slower that way,
because an engine holding every name walks each component's graph during
process where one built with no names walked a graph only when a
property of that component was first read.
The fastest approach is always used, so that part is removed. An engine
built with no names is built exactly as it is today, its table of names
to indexes is empty, and every read of its properties goes by name as
before. Nothing about it changes.
An engine built with a list of property names still reads every property
by its index in that list. That half is unchanged and is where the gain
is.
The no-names half follows once the native change is in and 51degrees-cxx
has had its second release.
* Test that restricted property names ignore case, and give the superset test the names CI has
A restricted property list given as the data set spells the names, for
example IsMobile, must still let flowdata.device.ismobile be read, as
access-to-results.md says string comparisons must be case insensitive.
The new core test gives "Boolean" and reads "boolean", and checks that a
name outside the list is refused in any case.
The IP Intelligence superset test asked for more than 6 names and the
data file in CI lists 3. It now builds with every other name in reverse
order, so the data set still holds a name the engine was not given, that
name sits between two given ones, and each given name's index differs
from its place in the data set.
* Compare restricted property names ignoring case
AspectData.get refused a property whose name was in the engine's
restricted list in another case, so an engine built with "IsMobile"
refused flowdata.device.ismobile with "Property ismobile was excluded
from device". access-to-results.md says string comparisons must be case
insensitive.
set_restricted_properties now also keeps the names in lower case, and
AspectData.get compares the key in lower case against them. The error
still names the key as the caller gave it.
* Report the client address the native code looks up, chosen as the specification says (#90)
* Read the client addresses in their own module, with no change in what is read
* Test that an IPv4-mapped address is reported as Ip and an address with spaces is not read
* Report an IPv4-mapped address as its IPv4 address, and read an address as given, as the Address forms section says
* Test that a value holding a zone index is not read
* Refuse a client IP value that holds a zone index, as the Zone indexes section says
* Test that an IPv4 address with a leading zero is not read
Reading the client IP value says the Engine must not read an IPv4
address with a leading zero in any part, such as 01.2.3.4, and must read
0.0.0.0. ipaddress refuses a leading zero, so the rule holds today, and
this test catches a Python whose ipaddress reads one.
* Let client_ips be given the engine's client IP keys
A structural change only, so the tests that follow can give the keys.
Nothing reads the parameter yet.
* Test that the client address is chosen as the native code chooses it
Choosing the IP address says the native code uses one value, from the
client IP keys the data file lists, the query keys first in the data
file's order and the server keys only when no query key supplied an
address, and that the Property values describe that one address.
Reading the client IP value reads a port, which is dropped, and an IPv6
address in brackets. evidence.md says keys are case insensitive.
The tests give client_ips the keys as an engine lists them and check
each of those, and that only the one address looked up is reported. This
commit is the tests alone, so six of them fail.
* Choose the client address as the native code does, and report only that one
The engine reported any evidence value that was an address, from a fixed
pair of keys first and then from any key at all, matched with letter
case, and it could not read a port or brackets. So the Ip or IpV6 it
reported could differ from the address the native code looked up, which
the other Property values describe.
As .NET's IpiOnPremiseEngine does, the engine now takes the client IP
keys from the native engine's own list, every query key in the data
file's order and then every server key, and reads each from the evidence
in any letter case. read_client_ip reads a whole value only, being an
address, an IPv4 address with a port, or an IPv6 address in brackets
with or without a port, and drops the port, as Reading the client IP
value says. Only the values read reach the native code, each as its
address alone under the engine's spelling of the key, so the first of
them is the address the native code looks up, and that one address is
reported, in Ip or in IpV6.
* Rename the data file variable and read every key under one _51DEGREES_ name (#97)
* Rename the data file environment variable to _51DEGREES_DD_PATH
A POSIX shell cannot set a variable whose name starts with a digit, so
51DEGREES_DD_PATH could not be exported the way the examples described.
Every name now starts with an underscore, and the old name is not read
anywhere.
A new check reads every file git tracks and fails on a name that starts
with the digit, run by its own workflow on each pull request.
* Fail the environment name check on a fallback name, in any case for a digit-initial name
Each variable has one name with nothing read in its place, so the check
now also fails on a bare RESOURCE_KEY or LICENSE_KEY read or set as an
environment variable, and on a LEGACY_..._ENV_VAR constant, the shape the
examples' fallbacks take. This is the check 51degrees-php uses, with the
forms Python reads an environment variable by.
A digit-initial name is matched in any case, since no tracked file holds
one in lower case. The fallback names are matched with case, because the
lower case resource_key and license_key are still the one name some
examples and tests read.
git lists paths unquoted, so a path holding a non-ASCII character is
still read, and the check says how many paths it read and which it
skipped as not files.
This commit is the check alone, before the fallbacks are removed, so it
fails.
* Fail the environment name check on a bare key name held in a string
The 51Did example reads its keys through env("_51DEGREES_RESOURCE_KEY",
"RESOURCE_KEY"), a fallback the read forms do not see. A string holding
RESOURCE_KEY or LICENSE_KEY and nothing else is now matched as well, and
is only ever such a name.
Still the check alone, before the fallbacks are removed, so it fails on
12 names.
* Read each key from its _51DEGREES_ name alone, with no fallback
Each variable has one name with nothing read in its place.
- The Device Detection and cloud request engine examples read the
resource key from _51DEGREES_RESOURCE_KEY and the licence key from
_51DEGREES_LICENSE_KEY, and no longer fall back to resource_key and
license_key. The LEGACY_ constants go with them.
- The 51Did example reads _51DEGREES_RESOURCE_KEY and
_51DEGREES_LICENSE_KEY, and no longer RESOURCE_KEY and LICENSE_KEY.
- The live tests' live_cloud.py, in all five copies, no longer tries
resource_key after the two _51DEGREES_ names, and the 51Did client and
licence key live tests read _51DEGREES_LICENSE_KEY alone.
- KeyUtils.get_named_key in both shared packages, and the IP
Intelligence examples' reader, read the name given and no longer its
upper case form.
common-ci sets only resource_key, so the IP Intelligence and pipeline
integration scripts now set _51DEGREES_RESOURCE_KEY from the test key,
as the Device Detection one already did. The READMEs and AGENTS.md name
the one variable each example reads.
* Name the environment name check's job for both of its checks
* Make the environment name check's matching a function its tests can call
Dot-sourcing the script now defines Find-EnvironmentNameProblem without
reading the repository, and the scan leaves out the script's own tests,
which name every form it fails on. What the check matches is unchanged.
* Test the environment name check, with names in lower case
Windows reads an environment variable name in any case, so a lower case
old name is still the old name there, and the check must fail on it. The
tests give each form the check must find, in upper, lower and mixed case,
and the lines it must leave alone, being the _51DEGREES_ names, the
builder's resource_key setting, a placeholder and a longer name. The
workflow runs them before the check.
This commit is the tests alone, so the lower case fallback forms fail.
* Match environment names in any case, and read every key under its _51DEGREES_ name
Windows reads an environment variable name in any case, so the check now
matches the fallback names in any case, as it already did the
digit-initial ones. A key name in a string is now matched only as a second
name after a _51DEGREES_ one, the shape of a fallback, because the builder
setting is also called resource_key.
Matching in any case finds the names that had no _51DEGREES_ form at all,
so each now reads its _51DEGREES_ name, as 51degrees-node does.
- The location examples and tests, the IP Intelligence examples and the
cloud request engine's own tests read _51DEGREES_RESOURCE_KEY rather
than resource_key.
- The two data file update examples read _51DEGREES_LICENSE_KEY rather
than license_key, and the tests that run the examples without settings
clear that name.
- The Device Detection integration script sets _51DEGREES_LICENSE_KEY
from the data file licence key, and the location one sets
_51DEGREES_RESOURCE_KEY, since common-ci sets only the older names.
- Every tox.ini passes the _51DEGREES_ names through in place of
resource_key and license_key, which nothing reads now.
- The READMEs, AGENTS.md and example comments name the new variables.
* Give the specification's JSON Builder error text with no sequence number (#101)
* Test the exact JSON Builder error text with no sequence number
* Give the specification's JSON Builder error text with no sequence number
Every language gives the same text when no Sequence Element has produced a sequence number, as the JSON Builder page of the specification states.
* Give IP Intelligence latitude and longitude to three decimal places (#81)
* Test that IP Intelligence latitude and longitude have three decimal places
* Give IP Intelligence latitude and longitude to three decimal places
The native code gives the coordinates in text and in the WKT of Areas to three
places, and the numbers read through the weighted and single value getters are
rounded to match. The unused sixteen place setting is removed.
* Test that a half rounds away from zero and a coordinate rounding to zero has no sign
* Round a half away from zero and give a coordinate that rounds to zero no sign
* Send every Evidence value to the cloud apart from the reserved names, under the name after the first dot, with prefixes ranked as evidence.md lists them (#99)
* Test that the Cloud Request Engine sends evidence the cloud does not list
* Send the evidence no element accepts to the cloud as well
* Test the reserved names with credentials the engine accepts
A resource key with requested properties is refused when the engine is
built, so the reserved name test now sends twice, once with a resource key
and a licence key and no property list, and once with a licence key alone
and the property list it needs.
* Test that every evidence value is sent under the name after the first dot, in its own letter case
The two rc tests that asserted a header and a cookie no element accepts
are left out now assert they are sent, under decision 1. The sent name is
everything after the first dot, as .NET's CloudRequestEngine.AddQueryData
takes it, and keeps the letter case of the evidence key.
* Send each evidence value under the name after the first dot, in its own letter case
The prefix is the text before the first dot, so query.a.license is sent as
a.license rather than dropped as the reserved name license, and the name
keeps the letter case of the evidence key, as .NET's AddQueryData does.
A name already added in other letters is the same name, so precedence
still sends it once.
* Test that the Cloud Request Engine ranks Evidence prefixes in evidence.md's order
* Rank server, fiftyone and location Evidence in evidence.md's order, then other prefixes alphabetically
* Pause usage sharing sends after repeated failures (#53)
* Test that usage sharing pauses sending after failed sends
Usage sharing in Python has no back-off. Every package is sent while the
service is failing, and is lost. These tests expect the back-off .NET is
being given, at its default settings of ten failed sends within 100
seconds and a first pause of two seconds:
- after ten failed sends nothing is sent during the pause, and the
evidence processed during it is all sent once the pause ends and the
service answers, with no more requests processed;
- closing while paused returns at once and sends nothing.
The test receiver can now answer with another status, and records every
request it gets as well as the data it accepts. The tests fail until the
back-off is added.
* Pause usage sharing sends after repeated failures
Python now has the same back-off as .NET, with the same five settings
and defaults, as constructor arguments.
- After failures_to_enter_recovery sends fail within
failures_window_seconds, sending pauses for
initial_recovery_delay_seconds. A send fails when it raises or the
answer is not 200.
- When a pause ends one send is tried. If it fails the delay is
multiplied by recovery_multiplier, up to max_recovery_delay_seconds.
If it succeeds the back-off ends.
- Processing is not affected. Evidence is queued during a pause, and new
evidence is dropped only when the queue is full. The new
max_queue_size argument sets the size, by default 1000 or ten times the
requested package size if that is larger, as in .NET. The queue had no
limit before, which a pause would have needed.
- One thread sends at a time, as in .NET and Java. It sends until fewer
than requested_package_size entries are queued. A timer that does not
keep the process running starts sending when a pause ends, so the queue
drains without more requests being processed.
- Closing cancels the timer and sends nothing while paused, logging how
many entries were not sent.
ShareUsageBackOff holds the rules and takes the time from the caller, so
its tests set the time. send_thread now returns whether the service
accepted the message.
* Test the full queue log, the failed answer log, message size, close and the repeat boundary
* Log a full queue and the content of a failed answer, send up to twice the package size, send what is queued on close and share a repeat at the interval
* Build the Devices message in one place, say the message size in the docstring, and give the constants their units
* Guard the failure deque against reading from an empty queue
* Take the message inside the lock so the name is always bound
* Show the root logo in every README, add one contributing guide, and fix the README links (#46)
* Show the root logo in every README, add one contributing guide, and fix the README links
Twenty READMEs showed an older 51Degrees wordmark from 51degrees.com rather
than the logo the root README shows, and the fiftyone_pipeline_did package
README had none. Each now shows the root logo as a Markdown image from
raw.githubusercontent.com, using .github/images/logo-160.png, a 160 pixel
copy of .github/images/logo.png, the size the root README shows it at.
The repository had no contributing guide of its own. One CONTRIBUTING.md
at the root now links the 51Degrees contributing guide.
The Developer Documentation link at the top of thirteen READMEs went to the
main page of the product's generated API reference, which has no content,
and the translation package's went to the documentation home page. They
now go to the product's documentation. The dependencies page that four
READMEs linked is gone, and the next sentence in each already links the
tested versions page, so the sentence is removed.
* Link each product's quick start from the READMEs
The documentation links went to section pages of the 51Degrees
documentation, such as _device_detection__index.html, which only list the
pages in the section. They now go to the page in each section that is most
helpful to a developer and the most recently kept up to date. Those are the
Device Detection, IP Intelligence and Reverse Geocoding quick starts, which
show how to choose between the cloud and on-premise and how to start in
each language, and Pipeline Basics for the Pipeline API, which has no quick
start. The campaign tags of each link are unchanged.
* Point each Find out more documentation line at the documentation home
* Move cxx to 7c4ac436 and read by index on an engine built with no names (#102)
* Move cxx to the 51degrees-cxx release commit 7c4ac436
7c4ac436 is the second cxx release of 11 November, from 51Degrees/51degrees-cxx#150. It carries 51Degrees/51degrees-cxx#148, which keeps the first load's property order for an engine built with no names. No SWIG interface changes beyond a comment, so the committed wrappers stay as they are.
* Test that an engine built with no names reads by index
An engine built with no names takes the names the native code gives after the first load as its list. Its table holds every one of them, each index names its property after a reload, and a read through the table gives what an engine built with the same names gives. The Device Detection test also times a read by name, a read by index, the typed read and a build of the table.
* Read by index on an engine built with no names
An engine built with no names now builds its index table from the names the native code gives after the first load, through getRequiredProperties. Since 51Degrees/51degrees-cxx#148 those names are every property of the first data set, sorted by name, and a reload does not change them, so the index of each one is fixed in the same way as for an engine built with names.
* Bound usage sharing cleanup at five seconds and take fifty as the minimum entries per message (#103)
* Test that closing stops after five seconds and fifty entries start a message
* Bound usage sharing cleanup at five seconds and take fifty as the minimum entries per message
* Write the specification's numbers in the tests rather than reading them from the element
* Give every environment variable one _51DEGREES_ name (#100)
* Make the name check fail on the names that remain, red
The check already failed on a name starting with the digit and on a
RESOURCE_KEY or LICENSE_KEY read behind a _51DEGREES_ one. It now also
fails on FOD_CLOUD_API_URL, cloud_endpoint, IPINTELLIGENCEDATAFILE,
ipintelligencetestkey and run_performance_tests, read or set as an
environment variable or held in a constant to read later.
A pipeline setting carries the same word as an old variable name, so
only the environment forms are matched and a setting written by its key
is left alone, which the tests state.
This commit is the check alone, so it fails over the repository as it
stands and names every place the next commit changes.
* Make the name check fail on cloud_request_origin too
The examples read it for a resource key limited to particular domains,
and it was missed when the other old names were added.
* Read every environment variable by its one _51DEGREES_ name
The cloud endpoint was read under two names, FOD_CLOUD_API_URL by the
cloud request engine and the 51Did client, and cloud_endpoint by the
examples. Both become _51DEGREES_CLOUD_ENDPOINT, which the Rust library
already reads, and the constants that hold the name move with it.
cloud_request_origin becomes _51DEGREES_CLOUD_REQUEST_ORIGIN,
IPINTELLIGENCEDATAFILE becomes _51DEGREES_IPI_PATH, and
run_performance_tests becomes _51DEGREES_RUN_PERFORMANCE_TESTS.
The shared key helper test set a variable called ipintelligencetestkey
as scaffolding rather than as a 51Degrees setting, so it takes
_51DEGREES_TEST_NAMED_KEY rather than the real resource key name, which
the test would otherwise overwrite for whatever ran next.
A pipeline setting and a function parameter carry the same words as
some of these names and are left alone, because only the environment
variable moves.
* Leave a fake resource key in a test fixture alone
The constant clause matched any assignment of an old name, so it also
matched resourceKey = "resource_key" in the cloud request engine's and
the IP Intelligence cloud tests, where the name is the fake key sent on
the wire rather than a variable.
It now matches only a constant whose own name says it holds a variable
name, and the names that are never anything but a variable are matched
wherever they appear. The tests state both.
* Treat cloud_request_origin as a setting as well, and stop the check naming names
cloud_request_origin is a pipeline builder parameter and a setting key
across the device detection and IP Intelligence builders, so it joins the
names matched only where a variable is read, set or named in a constant.
The names matched wherever they appear were written out in the pattern
and in the comment above it, so the check failed on its own source. Each
is now built from two pieces, as the digit-initial start already was.
* Leave the IpIntelligenceDataFile class alone
The names matched wherever they appear were matched in any case, so the
IP Intelligence data file's old name also matched the class name
IpIntelligenceDataFile, which differs from it by case alone. Those four
names now keep their case, and the tests state it.
* Say which group of old names keeps its case
The paragraph about keeping case sat under the paragraph about
RESOURCE_KEY, LICENSE_KEY, cloud_endpoint and cloud_request_origin, which
are matched in any case, so it read as though it described them. The
(?-i:) is on the other group. The paragraph now sits with the group it
describes and each paragraph names the variable that holds its four.
Comment only, and the two behaviours it describes are already pinned by
the Pester cases for the IpIntelligenceDataFile class and for a lower case
read of the shared names.
* See an old name braced as a PowerShell variable
The check misses an old name written as ${env:NAME}, as Env:\NAME or as a
workflow ${{ env.NAME }}, because its prefix wants the dollar and the env
token adjacent. The CI scripts here are PowerShell, where a name is braced
whenever it sits beside other text, so the check cannot see a form the
files it watches really use. The Go, Node, Java and PHP checks all match
the token on its own.
These cases fail until this one does the same.
* Match the env token on its own, so a braced old name is seen
The prefix wanted the dollar sign and the env token next to each other, so
${env:NAME}, Env:\NAME and a workflow ${{ env.NAME }} went past it.
Matching the token on its own, with a following backslash allowed, closes
all three and brings this check into line with the Go, Node, Java and PHP
ones.
* Use the standard licence text in one LICENSE, and the standard header in every file (#104)
* Use the standard licence text once, with its additional terms in NOTICE
The root LICENSE and ip-intelligence/LICENSE held the EUPL v1.2 text and
the two additional terms in Windows-1252, and pipeline/LICENSE held the
51Degrees header form. The root LICENSE now holds the text byte for byte
as 51degrees-cxx has it, NOTICE holds the two additional terms as
51degrees-cxx has them, and the two other copies are removed. Neither
copy sat in a package folder, and every setup.py declares EUPL-1.2.
Nine test files had no licence header and get the standard one. Three
headers had a closing line missing or written in the C comment form, and
now end as the standard header does.
* Keep the additional terms in the file headers rather than in NOTICE
The two additional terms apply where each file's header states them, so
the repository has LICENSE alone, without a NOTICE file. The README's
licence line goes back to what it said before.
* Do not cache a Cloud Request Engine response that holds a fodid (#83)
* Test that a Cloud Request Engine response with a fodid is not cached
* Do not cache a Cloud Request Engine response that holds a fodid
An engine can say a result must not be stored in its cache. The Cloud Request
Engine says so for a response with a top-level fodid, in any case, which holds
a 51Did created for the one request. Such a response is therefore never served
from the cache either.
* Remove a statement the fodid cache test does not need
* Test that a cached response holding a fodid is not served
The cache tests covered the store half only. Two tests now reach an entry already in the cache, one ordinary and one holding a fodid, through a cache that answers every lookup with the entry it holds.
* Do not serve a cached response that holds a fodid
is_cacheable now takes the result rather than the flowdata, so in_cache can ask it about an entry it has found, as .NET, Node.js and PHP do. An entry the engine may not serve counts as a miss.
* Treat evidence keys that differ only in letter case as one key (#95)
* Test that evidence keys ignore letter case
* Treat evidence keys that differ only in letter case as one key, as the specification says
* Test that the elements reading evidence keys ignore letter case
get_all gives each key in the letters it was added with, so the Sequence Element's key filter and the JavaScript Builder's reads are tested with keys in other letters.
* Match evidence keys in lower case where an element reads them
The Sequence Element's key filter and the JavaScript Builder's key filter, session id, sequence and query parameters all matched the keys get_all gives with letter case, so a key added in other letters was missed. Each now matches in lower case, and a name sent on keeps the letters it was added with.
* Take the free data files from the run's asset cache (#106)
Each test job downloaded the Asn data file again with the data repository's script, outside the asset cache that the run fills once and every test job restores. They are now fetched as assets with the others, restored from the cache in each test job, and copied or linked to where the tests and examples look for them, so no test job downloads a data file. The prefetch in Configure fetches every product's files, because the cache key does not depend on the products a change touches. The notes say CI takes every data file from the assets, and that a new data file needs an entry in common-ci's steps/fetch-assets.ps1. Invoke-ProductStep takes the products to run when it is given them. The Device Detection script's call to the data repository's script downloaded nothing, so it is removed.
* Align the data update service to the specification: HTTP date, expected time, update from memory and logging (#69)
* Align the data update service to the specification
Send If-Modified-Since as an HTTP date, wait for the data update expected
time before the first poll, add update_from_memory, and log the actions
with the messages and levels in the Logging section of the data updates
specification.
* Treat a data file that is already current as a finished update in the update examples
With If-Modified-Since sent as an HTTP date, the Distributor answers 304 when
the file is current, which the examples reported as a failure.
* Test that no update message holds the licence key, the watcher needs automatic updates, a file data file is written, and content checks carry on
* Register the watched data files in the watcher tests with automatic updates on
* Mask the licence key in every update message, create the watcher only with automatic updates, write update_from_memory's bytes to a data file kept on disk, and carry on without an MD5 header or with content that is not compressed
* Test that a callback that raises does not stop the checks, and a connection failure keeps polling
* Log a callback that raises, so the other callbacks and the next check still run
* Test that the watcher refresh is routed as the polling refresh is
* Route the watcher refresh through the polling refresh
* Test that a callback's error is logged with .NET's message for any other error
DataUpdateService.cs logs an error from an update complete callback with
MessageAutoUpdateUnhandledError at Error level. The test now asserts that
text, so it fails while Python writes its own sentence.
* Test that a callback that raises stops the callbacks after it
data-updates.md at e89679e9 says the callbacks after the one that raised
are not called. On update polling the error is logged with the last
row's message, which is .NET's MessageAutoUpdateUnhandledError, and on a
programmatic check it passes to the caller. The tests cover polling, a
programmatic check and an update from memory, and that polling still
schedules its next check. This commit is the tests alone, so they fail.
* Test that a callback that raises is logged and stops the callbacks after it
data-updates.md at e89679e9 says the callbacks after the one that raised
are not called, and that on update polling and the file watcher the error
is logged with the last row's message, .NET's
MessageAutoUpdateUnhandledError. Passing the error to the caller of a
programmatic check, an update from memory or the update on start-up waits
for next week, so on those routes it is logged in the same way for now.
This commit is the tests alone, so they fail.
* Stop at the first callback that raises, and log it with .NET's message
DataUpdateService.cs wraps the callback loop in one try, so the first
callback that raises stops the rest, and LogUpdateException logs it with
MessageAutoUpdateUnhandledError, "An unhandled error occurred while
checking for automatic updates for engine '{0}'", at Error.
data-updates.md says the same. Python now does both, on every route, and
the next check is still scheduled.
* Attach the error a callback raised to the log entry
The errors table in the data updates specification says an error of any
other kind is logged with the error attached, and that a callback's
error is logged in the same way. __emit wrote the message and bound no
variable, so the error the callback raised was lost. It is now written
after the message, which is what .NET, Java, Node.js and Go each do.
The new test file takes the licence header every other file in the
repository carries.
* Name only the languages that attach a callback's error
The comment on the expected message said .NET, Node.js and Go write the
error the callback raised, and Java as well. Java's data update service
calls each onUpdateComplete in a bare loop with no catch
(DataUpdateServiceDefault.java on its rc, the loops after the update and
after the file watcher's refresh), so a callback that raises there is not
caught and nothing is logged. The comment now names the three that do.
* Test that a callback's error is masked in the log entry
* Mask every licence key in a callback's error before logging it
* Give each web integration element its own switch, put Sequence first and give it Element Data (#70)
* Give each web integration element its own switch and put Sequence first
The Sequence, JSON Builder, JavaScript Builder and Set Headers elements each
have a switch that defaults to on, and add_javascript_builder and
use_setheader_properties work as before. The Sequence Element is the first
element of the pipeline and also writes the session-id and sequence Element
Data, as well as its Evidence.
* Include the Sequence Element in the JSON, declare its properties and say the Lite file has few Properties
The JSON Builder specification requires a top-level entry for each Element
Data and excludes only Set Headers, the Cloud Request Engine and Usage
Sharing, so the JSON Builder no longer skips the Sequence Element. The
Sequence Element declares session-id and sequence as the Sequence Element
specification lists them. The Device Detection example comments now say what
the required examples specification says of the Lite data file.
* Write an empty session id to the Sequence Element Data when the script cannot be given it
The Sequence Element's session id now goes into the JSON that is written into
the script, so an id the script cannot take is empty there, as it is in the
script's own sessionId.
* Expect the incremented sequence in the usage sharing record now that Sequence is first
* Give the native engines evidence as text, as the Sequence Element now adds a number before them
* Read the session id and the sequence independently in the Sequence Element
A sequence with no session id now counts up with a new id. A session id that is
not a non-empty string is absent, a sequence in text must be decimal digits, and
the Evidence and the Element Data are written from the same values. The JSON
written into the script escapes the characters that could end the script.
* Check the escaped script end in the session id test
* Take a sequence of 0 to 2147483646, as the specification says
The Sequence Element took a sequence of 1 to 2147483647 and added one, so
2147483647 gave an output of 2147483648, which leaves a 32 bit integer.
The specification sets the top at 2,147,483,646 so that adding one stays
within one, which is the range 51degrees-node and 51degrees-java take.
* Say the Lite data file has a small number of Properties in the last example too
datafileupdate_console.py still said "limited capabilities", which is
the wording 51degrees-node also replaced in its own update example.
* Say plainly in the docstring what add_set_header_element does
* Set the client-side cache headers, with a weak ETag that is the same in every process and none after errors (#59)
* Test the cache headers of the client-side endpoints
* Set the client-side cache headers, with an ETag that is the same in every process and none after errors
* Test that a POST with a matching ETag gets 412, and a GET or HEAD 304
* Answer a POST whose If-None-Match matches with 412, as RFC 9110 section 13.1.2 asks
* Test that the web request value added for usage sharing is not in the ETag
* Leave the web request value out of the ETag
* Test that the client-side ETag is weak and If-None-Match is read as RFC 9110 says
* Send the client-side ETag as a weak entity tag, and match If-None-Match only when it is "*" or a list of entity tags
* Test that the client-side max age can be set
* Let the caller set the client-side max age, which the specification says should be configurable
* Read the session id from the minified script in the ETag test
* Name every header in the evidence key filter in Vary, as the other languages do
The Vary header named only the headers the request carried, so a cache
would not re-fetch when a header the pipeline accepts appeared for the
first time. The web integration specification's cache header detail says
Vary carries all headers in the Pipeline Evidence key filter, and
51degrees-node, 51degrees-php and 51degrees-go all build it from each
element's filter list.
_vary now reads each element's evidence key filter list, and asks a
filter that cannot list its keys about the request's own headers one by
one, which is what PHP does. A key that is not a field name is left out,
and the revalidation headers stay out as before.
Three tests cover it.
* Name Host in the Vary header expectations, which the JavaScript Builder adds
The three new Vary tests expected only the headers their own element
accepts. Run 43175480017 showed Host beside them, which is right, because
the JavaScript Builder's evidence key filter accepts header.host to build
the callback URL the client script posts to, and its filter is not a list
so the request's own headers are asked of it one by one.
* Test that Content-Length is the length of what was sent
* Send requested properties beside a resource key, with or without a licence key (#89)
* Test that requested properties beside a resource key are sent, with or without a licence key
* Send requested properties beside a resource key, with or without a licence key
The engine refused requested_properties whenever a resource key was set,
because the cloud service was thought to ignore the list. The service
adds the listed properties to those the resource key carries, so the
engine now builds and sends the list as values, and the licence key too
where there is one. It still refuses no key at all, and a licence key
with no resource key or list, which the service refuses on every request.
The message for properties an answer leaves out now says they are not
covered by the keys in use, as a resource key can be the only one.
* Test that a name left out beside a resource key alone is reported as not a property
* Report a name left out beside a resource key alone as not a property, and drop history from two comments
* Say the keys in use in the properties not covered warning, and drop a stale claim about discovery
.NET, Java, Node.js and PHP all word the warning as not covered by the
keys in use, because the licence behind a resource key and a licence key
beside it both decide what comes back. The name not known warning gains
the same closing line and the spelling advice those languages give.
The discovery comment no longer says a licence key beside a resource key
widens the answer, which the corrected Accepted combinations table
replaced.
Tests assert both wordings.
* Test an answer that is not an object, and the message .NET and Java write
The report compares the answer with the list it asked for, and an answer
that parses as JSON but is not an object has nothing to compare, so the
test pins that the request still succeeds and nothing is written.
The warning for a name that is not a property is asserted as .NET and
Java word it, both of them carrying the same sentence.
* Guard the report against an answer that is not an obje…
Merged
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.
Problem
Python is the only language that refuses
requested_propertiesbeside aresource key. On
rc8ac7236 the engine raisesValueErrorwhenever aresource key and a list are both set, with or without a licence key
(
cloudrequestengine.pylines 216 to 218,carrying
MESSAGE_PROPERTIES_IGNORED_WITH_RESOURCE_KEYfromconstants.pylines 118 to 124).The reason the message gives is that the cloud service ignores the list
whenever a resource key is present, which is not what the service does.
So a deployment that holds a resource key and wants one extra property
cannot build the engine at all in Python, where in every other language it
builds and the extra property comes back.
What the specification says
Quoted from
cloud-request-engine.md at 6570727d,
which is the head of
mainin51Degrees/specifications and
is pinned here rather than
rc, which moves.Requested Properties, lines 92 to 100:
Accepted combinations, lines 150 to 158,
with what Python
rcdoes beside each row:rcMESSAGE_LICENSE_KEY_NEEDS_PROPERTIESMESSAGE_NO_CREDENTIALSo two of the seven rows are wrong on
rcand the other five alreadymatch.
Properties the remote server did not return, lines 420 to 433:
and
What the other languages merged
Each of these merged the same behaviour into its own
rcon 10 and 11November, which is why Python is now the one language short.
None of the five removes a refusal, because none of them ever had one. The
refusal is Python's alone, so this draft is Python catching up rather than
a change the set is making together.
.NET's
CheckCredentialskeeps exactly the two refusals this draft keeps,being no credential at all and a licence key with no list, and nothing
else.
Both warnings now say word for word what .NET and Java say. I read the two
constants on each language's
rcrather than reading the pull requests, in.NET
Messages.resxand
Java
Constants.java,and the two are identical to each other.
%sin its answer. They are not covered by the keys in use, so they will be missing from every result. The other requested properties came back and are usable."%sin its answer. Every property is answered, with a reason when it has no value, so these names are not properties. Check the spelling of each one. The other requested properties came back and are usable."Python takes the .NET and Java column for both, so Python, .NET and Java
are now one wording and PHP and Node.js are another, with Go different
again. The specification sets the meaning rather than the sentence, so one
wording across all six is worth a change next week rather than now.
What changed
Four files, 155 lines added and 64 removed.
check_credentials(
cloudrequestengine.pyline 193)no longer refuses a list beside a resource key, and
MESSAGE_PROPERTIES_IGNORED_WITH_RESOURCE_KEYis deleted with it, so nomessage outlives the rule it stated. The two refusals the specification
does ask for stay, being no resource key and no licence key, and no
resource key with no list.
add_requested_propertiesno longer callscheck_credentialsagain.Names added after the engine is built can no longer turn a valid engine
into an invalid one, because the only combination the list took part in
has gone.
process_internal(line 435)
already sent
valueswhenever the list was not empty, andget_credentialsalready sentresourceandlicensewherever each wasset, with the licence key in the body and never in a URL. Taking the
refusal away is what lets that path run.
report_properties_not_covered(line 475)
now picks between two messages. With a licence key,
MESSAGE_PROPERTIES_NOT_COVEREDsays the properties are not covered bythe keys in use, naming the keys rather than one key because the licence
behind the resource key and a licence key beside it both decide what
comes back. With a resource key and no licence key, the new
MESSAGE_PROPERTIES_NOT_KNOWNsays every property is answered, with areason when it has no value, so these names are not properties, and asks
the reader to check the spelling of each one. That is what the
specification requires rather than naming a licence key that was never
given, and it is .NET's and Java's sentence word for word.
not an object
(line 511).
Such an answer reaches here, because
validate_responsetests"errors" in json_response, which is a membership test that an array ora string answers without raising, and the report then called
answer.get(section)and threwAttributeErrorout of processing. Thespecification says the report "MUST NOT fail the request", and the
request had succeeded, so there is nothing to say and nothing to fail.
The guard and the wider catch are the ones
is_cacheablealready useson the same text a few lines above, so the two readers of a cloud answer
now treat a malformed one the same way.
readme.mdtable now has all seven rows of the specification's table, where it had
six. The row that refused a list beside a resource key has gone and the
two rows the specification gives for that case take its place, and the
resource key with licence key row is reworded, because a licence key
adds no property when no list is sent. The paragraph below the table
says what the warning says in each of the two cases.
than left behind, in the constructor, in
add_requested_properties, inget_engine_propertiesand inreport_properties_not_covered.Effect on existing callers
None for any configuration that built before, because no accepted
combination changes and no message a caller already saw changes except the
one warning sentence. A configuration that was refused, being a list beside
a resource key, now builds and sends the list.
Tests
test_credentials.pygoes from 17 tests to 19. Each was written before the change it covers, as
the two red runs below show.
test_properties_with_resource_key_are_sentreplacestest_properties_with_resource_key_are_refused. It builds the enginewith and without a licence key beside the resource key, and checks the
data request goes to the resource key's own route, carries
resource,carries
valuesas the comma separated list, carrieslicenseonly whenone was given, and that no request anywhere puts the licence key in a
URL.
test_a_name_left_out_beside_a_resource_key_alone_is_not_a_propertyisnew. With a resource key, a list and no licence key it processes twice
and checks exactly one warning names the missing name, that the warning
carries .NET's and Java's sentence about every property being answered
and about checking the spelling, that it never says "licence key", and
that the property that did come back is not named.
test_an_answer_that_is_not_an_object_does_not_fail_the_requestis new.It answers a licence key request with a JSON array and checks that
processing completes and that nothing is written, which pins the
specification's "MUST NOT fail the request".
test_properties_added_later_are_normalised_and_sentreplacestest_properties_added_later_are_normalised_and_checked, and now checksthat names added after the engine is built reach the request as
valuesbeside a resource key.
test_properties_the_answer_leaves_out_are_reported_oncegains anassertion on the new "not covered by the keys in use" wording.
test_resource_key_with_licence_key_is_acceptedkeeps its behaviour andits docstring is corrected, because the licence key adds no property when
no list is sent.
Runs
Each temporary branch is the branch it names plus
.github/workflows/tmp-fixpy.ymlandtmp-tests.txt, which carry thethrowaway workflow and the one test path it runs, and nothing else.
tmp/fixpy-89b-red, 6b2298a, beingrc8ac7236 with this draft's test file and no source changetmp/fixpy-89b-green, 60dd711, being the earlier head cb86932tmp/fixpy-89c-red, edcc100, being that same head cb86932 with the two later tests and no source changetmp/fixpy-89c-green, 5ec4fef, being the head 039c128tmp/pr-matrix-89c, built fromrc8ac7236The first red is the main change. Its four failures each have their own
cause rather than a collection error:
rctest_properties_with_resource_key_are_sentValueErrorfrom the refusaltest_properties_added_later_are_normalised_and_sentValueErrorfrom the refusaltest_a_name_left_out_beside_a_resource_key_alone_is_not_a_propertyValueErrorfrom the refusaltest_properties_the_answer_leaves_out_are_reported_onceThe second red covers the two later changes, and its two failures are the
faults they fix:
test_an_answer_that_is_not_an_object_does_not_fail_the_requestAttributeError: 'list' object has no attribute 'get', which is the request failingtest_a_name_left_out_beside_a_resource_key_alone_is_not_a_propertyThe full build has 11
Setting 'PullRequestSha'lines and every one ofthem is
039c1282fcb51e8b17484b4b64364fa31baf3a00, the head. Configureprinted "Left out 5 macOS entries". All ten Build and Test jobs ran, being
Ubuntu and Windows on Python 3.10 to 3.14, and
test_credentials.pyran 19tests and passed all 19 in every one of the ten.
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.
The three checks on the head also pass, being
the variable names check,
only rc into main
and utm-link-lint.
Merging rc
rc8ac7236 is merged into the branch at 121ae0d and is an ancestor ofthe head, so nothing
rcadded is undone. The whole difference betweenrcand the head is the four files above.rctook#63,
#72 and
#79, each of which
added tests at the same place in
test_credentials.py, and the merges at247eef9, 647fa74 and 121ae0d keep every one of them, which the count of 19
tests on the head bears out against 17 on
rc, being two renamed in placeand two added.
Other drafts into Python's
rcare being worked on at the same time. Ifrcmoves before this draft merges, it needs merging again and buildingagain.
Issue
For #12. The
table in that issue predates the corrected specification section, and this
draft follows the specification.
What remains
Accepted combinations table in all seven rows.
get_engine_propertiesasksaccessiblePropertieswith the keys aloneand sends no
values, which is right, becauseDiscovery under a License Key
says property metadata must come from that answer and must not be
manufactured from the Requested Properties list. A property added by the
list alone therefore has no metadata until the key carries it, in Python
as in the other languages.
as the table above sets out. Python, .NET and Java are one wording, PHP
and Node.js are a second and differ from each other in whether they give
any advice at all, and Go is a third. Python now matches the largest
group and is no longer alone, which is as far as this draft should
reach, because changing PHP, Node.js or Go means changing their tests
and they are not this draft's to change. One wording across all six
wants its own change next week, and PHP's "Check each name against the
property dictionary." is worth considering for it, because it names a
real 51Degrees resource where the other five name none.