diff --git a/pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py b/pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py index 943674b8..3a91a980 100644 --- a/pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py +++ b/pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py @@ -435,9 +435,10 @@ def found_evidence(self, flow_data): The evidence the element finds to share. These are the values of the web request, when the web integration added it as server.http-request, and every evidence value in the flow data, - whether an element accepted it or not. A key found in both places - takes the evidence value. The web request itself is never one of - them. + whether an element accepted it or not. Keys are compared without + regard to case, so a key found in more than one place is found + once, with the evidence value and the evidence's spelling. The web + request itself is never one of them. @type flow_data: FlowData @param flow_data: The flow data being processed @@ -450,12 +451,15 @@ def found_evidence(self, flow_data): unaccepted = getattr(evidence, "get_unaccepted", dict)() web_request = unaccepted.get(HTTP_REQUEST_KEY) or \ evidence.get(HTTP_REQUEST_KEY) or {} + # The key of each value found, by its lower case form. A later + # source replaces both the value and the spelling of an earlier one. found = {} for source in (web_request, unaccepted, evidence.get_all()): for key, value in source.items(): - if str(key).lower() != HTTP_REQUEST_KEY: - found[key] = value - return found + folded = str(key).lower() + if folded != HTTP_REQUEST_KEY: + found[folded] = (key, value) + return dict(found.values()) def share_send_usage(self): """! diff --git a/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_accepts_no_evidence.py b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_accepts_no_evidence.py index e683f856..503abff0 100644 --- a/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_accepts_no_evidence.py +++ b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_accepts_no_evidence.py @@ -25,8 +25,13 @@ depend on it. It shares what it finds in the flow data and in the web request instead, by the same rules as before.""" +import gzip +import re import unittest +from types import MappingProxyType +from unittest import mock +import requests from flask import Flask, request from fiftyone_pipeline_core.basiclist_evidence_keyfilter import \ @@ -82,6 +87,22 @@ def request_evidence(headers, path="/"): return webevidence(request) +def sent_on_close(usage): + """Closes the usage sharing element, which sends what it has queued, + and returns the text of each message it posted. The post is answered + with a 200 without leaving the process.""" + + sent = [] + + def post(url, headers=None, data=None, **kwargs): + sent.append(gzip.decompress(data).decode("utf8")) + return mock.Mock(status_code=200, text="") + + with mock.patch.object(requests, "post", post): + usage.close() + return sent + + class ShareUsageAcceptsNoEvidenceTests(unittest.TestCase): def test_the_pipeline_accepts_no_header_for_usage_sharing(self): @@ -171,6 +192,40 @@ def test_a_key_in_both_places_is_shared_once_with_the_evidence_value(self): self.assertIn( '
from the evidence
', shared) + def test_a_header_no_element_reads_is_in_the_message_sent(self): + usage = usage_element() + flowdata = pipeline_with(usage).create_flowdata() + flowdata.evidence.add_from_dict(request_evidence({ + "User-Agent": CHROME, + "X-Test": "only usage sharing reads this", + })) + flowdata.process() + + sent = sent_on_close(usage) + self.assertEqual(1, len(sent)) + self.assertIn( + '
only usage sharing reads this
', + sent[0]) + self.assertNotIn("http-request", sent[0]) + + def test_a_key_in_both_places_whose_letters_differ_is_shared_once(self): + usage = usage_element() + flowdata = pipeline_with(usage).create_flowdata() + # Added before the request, so the evidence keeps this spelling and + # the request carries the other one. + flowdata.evidence.add("HEADER.User-Agent", "evidence agent") + flowdata.evidence.add("server.http-request", MappingProxyType({ + "header.user-agent": "request agent"})) + flowdata.process() + + sent = sent_on_close(usage) + self.assertEqual(1, len(sent)) + names = re.findall( + r'