commit da51f6d069ac55ebf430ebb0adfa6a70110fda68 Author: Arturo Filastò <arturo@filasto.net> Date: Tue Apr 12 17:09:51 2016 +0200 Clean up how we handle the reportId and remove unused functions --- ooni/deck.py | 52 ++++------------------------------- ooni/director.py | 8 +----- ooni/reporter.py | 66 +++++++++++++++++++-------------------------- ooni/tasks.py | 3 --- ooni/tests/mocks.py | 19 +++++++++++++ ooni/tests/test_deck.py | 4 +-- ooni/tests/test_reporter.py | 2 +- 7 files changed, 55 insertions(+), 99 deletions(-) diff --git a/ooni/deck.py b/ooni/deck.py index 4b6d894..b13385d 100644 --- a/ooni/deck.py +++ b/ooni/deck.py @@ -171,10 +171,10 @@ class Deck(InputFile): if self.bouncer: log.msg("Looking up collector and test helpers") - yield self.lookupCollector() + yield self.lookupCollectorAndTestHelpers() @defer.inlineCallbacks - def lookupCollector(self): + def lookupCollectorAndTestHelpers(self): self.oonibclient.address = self.bouncer required_nettests = [] @@ -221,9 +221,9 @@ class Deck(InputFile): net_test_loader.testName) collector, test_helpers = \ - find_collector_and_test_helpers(net_test_loader.testName, - net_test_loader.testVersion, - net_test_loader.inputFiles) + find_collector_and_test_helpers(test_name=net_test_loader.testName, + test_version=net_test_loader.testVersion, + input_files=net_test_loader.inputFiles) for option, name in net_test_loader.missingTestHelpers: test_helper_address = test_helpers[name].encode('utf-8') @@ -234,48 +234,6 @@ class Deck(InputFile): net_test_loader.collector = collector.encode('utf-8') @defer.inlineCallbacks - def lookupTestHelpers(self): - self.oonibclient.address = self.bouncer - - required_test_helpers = [] - requires_collector = [] - for net_test_loader in self.netTestLoaders: - if not net_test_loader.collector and not self.no_collector: - requires_collector.append(net_test_loader) - - required_test_helpers += map(lambda x: x[1], - net_test_loader.missingTestHelpers) - - if not required_test_helpers and not requires_collector: - defer.returnValue(None) - - response = yield self.oonibclient.lookupTestHelpers(required_test_helpers) - - for net_test_loader in self.netTestLoaders: - log.msg("Setting collector and test helpers for %s" % - net_test_loader.testName) - - # Only set the collector if the no collector has been specified - # from the command line or via the test deck. - if len(net_test_loader.missingTestHelpers) == 0 and \ - net_test_loader in requires_collector: - log.msg("Using the default collector: %s" % - response['default']['collector']) - net_test_loader.collector = response['default']['collector'].encode('utf-8') - continue - - for option, name in net_test_loader.missingTestHelpers: - test_helper_address = response[name]['address'].encode('utf-8') - test_helper_collector = \ - response[name]['collector'].encode('utf-8') - - log.msg("Using this helper: %s" % test_helper_address) - net_test_loader.localOptions[option] = test_helper_address - net_test_loader.testHelpers[option] = test_helper_address - if net_test_loader in requires_collector: - net_test_loader.collector = test_helper_collector - - @defer.inlineCallbacks def fetchAndVerifyNetTestInput(self, net_test_loader): """ fetch and verify a single NetTest's inputs """ log.debug("Fetching and verifying inputs") diff --git a/ooni/director.py b/ooni/director.py index 13a4503..64b5a98 100644 --- a/ooni/director.py +++ b/ooni/director.py @@ -253,16 +253,10 @@ class Director(object): self.reportEntryManager, collector_address, no_yamloo) + yield report.open() net_test = NetTest(test_cases, test_details, report) net_test.director = self - yield net_test.report.open() - - # XXX this needs some serious refactoring - net_test_loader.reportID = report.reportID - net_test.reportID = report.reportID - net_test.testDetails['report_id'] = report.reportID - yield net_test.initializeInputProcessor() try: self.activeNetTests.append(net_test) diff --git a/ooni/reporter.py b/ooni/reporter.py index 4318fd5..aa6f5ee 100644 --- a/ooni/reporter.py +++ b/ooni/reporter.py @@ -163,7 +163,10 @@ class YAMLReporter(OReporter): if not os.path.isdir(report_destination): raise errors.InvalidDestination - report_filename = generate_filename(test_details, filename=report_filename, prefix='report', extension='yamloo') + report_filename = generate_filename(test_details, + filename=report_filename, + prefix='report', + extension='yamloo') report_path = os.path.join(self.reportDestination, report_filename) @@ -239,7 +242,7 @@ class OONIBReporter(OReporter): self.collectorAddress = collector_address self.validateCollectorAddress() - self.reportID = None + self.reportId = None self.supportedFormats = ["yaml"] if self.collectorAddress.startswith('https://'): @@ -302,7 +305,7 @@ class OONIBReporter(OReporter): def writeReportEntry(self, entry): log.debug("Writing report with OONIB reporter") - url = self.collectorAddress + '/report/' + self.reportID + url = self.collectorAddress + '/report/' + self.reportId if "json" in self.supportedFormats: serialisation_format = 'json' @@ -314,7 +317,7 @@ class OONIBReporter(OReporter): 'content': self.serializeEntry(entry, serialisation_format) } - log.debug("Updating report with id %s (%s)" % (self.reportID, url)) + log.debug("Updating report with id %s (%s)" % (self.reportId, url)) request_json = json.dumps(request) log.debug("Sending %s" % request_json) @@ -414,7 +417,7 @@ class OONIBReporter(OReporter): "different collector.") raise errors.OONIBReportCreationError - self.reportID = parsed_response['report_id'] + self.reportId = parsed_response['report_id'] self.backendVersion = parsed_response['backend_version'] self.supportedFormats = parsed_response.get('supported_formats', ["yaml"]) @@ -423,7 +426,7 @@ class OONIBReporter(OReporter): defer.returnValue(parsed_response['report_id']) def finish(self): - url = self.collectorAddress + '/report/' + self.reportID + '/close' + url = self.collectorAddress + '/report/' + self.reportId + '/close' log.debug("Closing the report %s" % url) return self.agent.request("POST", str(url)) @@ -538,6 +541,7 @@ class OONIBReportLog(object): 'collector': collector_address, 'report_id': report_id } + return report_id def created(self, report_file, collector_address, report_id): return self.run(self._created, report_file, @@ -576,7 +580,7 @@ class OONIBReportLog(object): class Report(object): - reportID = None + reportId = None def __init__(self, test_details, report_filename, reportEntryManager, collector_address=None, @@ -605,19 +609,13 @@ class Report(object): """ self.test_details = test_details self.collector_address = collector_address + self.report_filename = report_filename self.report_log = OONIBReportLog() self.yaml_reporter = None - self.report_filename = None - if not no_yamloo: - self.yaml_reporter = YAMLReporter(test_details, report_filename=report_filename) - self.report_filename = self.yaml_reporter.report_path - self.oonib_reporter = None - if collector_address: - self.oonib_reporter = OONIBReporter(test_details, - collector_address) + self.no_yamloo = no_yamloo self.done = defer.Deferred() self.reportEntryManager = reportEntryManager @@ -641,36 +639,26 @@ class Report(object): d.addCallback(created) return d + @defer.inlineCallbacks def open(self): """ This will create all the reports that need to be created and fires the created callback of the reporter whose report got created. """ - d = defer.Deferred() - deferreds = [] - - def yaml_report_failed(failure): - d.errback(failure) - - def all_reports_openned(result): - if not d.called: - d.callback(None) - - if self.oonib_reporter: - deferreds.append(self.open_oonib_reporter()) - else: - if self.yaml_reporter: - deferreds.append(self.report_log.not_created(self.report_filename)) - - if self.yaml_reporter: - yaml_report_created = \ - defer.maybeDeferred(self.yaml_reporter.createReport) - yaml_report_created.addErrback(yaml_report_failed) - - dl = defer.DeferredList(deferreds) - dl.addCallback(all_reports_openned) + if self.collector_address: + self.oonib_reporter = OONIBReporter(self.net_test_details, + self.collector_address) + self.net_test_details['report_id'] = yield self.open_oonib_reporter() + + if not self.no_yamloo: + self.yaml_reporter = YAMLReporter(self.net_test_details, + report_filename=self.report_filename) + self.report_filename = self.yaml_reporter.report_path + if not self.oonib_reporter: + yield self.report_log.not_created(self.report_filename) + yield defer.maybeDeferred(self.yaml_reporter.createReport) - return d + defer.returnValue(self.reportId) def write(self, measurement): """ diff --git a/ooni/tasks.py b/ooni/tasks.py index 2c4f07d..72e211f 100644 --- a/ooni/tasks.py +++ b/ooni/tasks.py @@ -115,9 +115,6 @@ class Measurement(TaskWithTimeout): if 'input' not in self.testInstance.report.keys(): self.testInstance.report['input'] = test_input - if 'test_start_time' not in self.testInstance.report.keys(): - start_time = otime.epochToNewTimestamp(self.testInstance._start_time) - self.testInstance.report['test_start_time'] = start_time self.testInstance.setUp() diff --git a/ooni/tests/mocks.py b/ooni/tests/mocks.py index 42f548e..4f51f32 100644 --- a/ooni/tests/mocks.py +++ b/ooni/tests/mocks.py @@ -203,3 +203,22 @@ class MockOONIBClient(object): 'collector': 'httpo://thirteenchars1234.onion' } return defer.succeed(ret) + + def lookupTestCollector(self, net_tests): + ret = { + 'net-tests': [ + ] + } + for net_test in net_tests: + test_helpers ={} + for test_helper in net_test['test-helpers']: + test_helpers[test_helper] = '127.0.0.1' + + ret['net-tests'].append({ + 'name': net_test['name'], + 'version': net_test['version'], + 'input-hashes': net_test['input-hashes'], + 'collector': 'httpo://thirteenchars1234.onion', + 'test-helpers': test_helpers + }) + return defer.succeed(ret) diff --git a/ooni/tests/test_deck.py b/ooni/tests/test_deck.py index 7b423af..2c3bc04 100644 --- a/ooni/tests/test_deck.py +++ b/ooni/tests/test_deck.py @@ -148,7 +148,7 @@ class TestDeck(BaseTestCase): deck.verify() @defer.inlineCallbacks - def test_lookuptest_helpers(self): + def test_lookup_test_helpers_and_collector(self): deck = Deck(decks_directory=".") deck.bouncer = "httpo://foo.onion" deck.oonibclient = MockOONIBClient() @@ -156,7 +156,7 @@ class TestDeck(BaseTestCase): self.assertEqual(len(deck.netTestLoaders[0].missingTestHelpers), 1) - yield deck.lookupTestHelpers() + yield deck.lookupCollectorAndTestHelpers() self.assertEqual(deck.netTestLoaders[0].collector, 'httpo://thirteenchars1234.onion') diff --git a/ooni/tests/test_reporter.py b/ooni/tests/test_reporter.py index 9774449..b4e7592 100644 --- a/ooni/tests/test_reporter.py +++ b/ooni/tests/test_reporter.py @@ -92,7 +92,7 @@ class TestOONIBReporter(unittest.TestCase): def test_create_report(self): self.mock_response = oonib_new_report_message yield self.oonib_reporter.createReport() - assert self.oonib_reporter.reportID == oonib_new_report_message[ + assert self.oonib_reporter.reportId == oonib_new_report_message[ 'report_id'] @defer.inlineCallbacks