From 8ff1f0b781c49b88da13af390e4d118ad3e77b43 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Mon, 17 Oct 2016 03:25:43 -0300 Subject: [refactor] Issues from code review --- client/src/leap/soledad/client/api.py | 8 ------- client/src/leap/soledad/client/http_target/api.py | 7 ++++-- .../src/leap/soledad/client/http_target/fetch.py | 28 ++++++++++++---------- .../soledad/client/http_target/fetch_protocol.py | 2 +- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/client/src/leap/soledad/client/api.py b/client/src/leap/soledad/client/api.py index 1f151e7d..c560f661 100644 --- a/client/src/leap/soledad/client/api.py +++ b/client/src/leap/soledad/client/api.py @@ -967,13 +967,5 @@ class VerifiedHTTPSConnection(httplib.HTTPSConnection): match_hostname(self.sock.getpeercert(), self.host) -# TODO move this to a common module - -class DocInfo: - def __init__(self, doc_id, rev): - self.doc_id = doc_id - self.rev = rev - - old__VerifiedHTTPSConnection = http_client._VerifiedHTTPSConnection http_client._VerifiedHTTPSConnection = VerifiedHTTPSConnection diff --git a/client/src/leap/soledad/client/http_target/api.py b/client/src/leap/soledad/client/http_target/api.py index 2d51d94f..0e24b37f 100644 --- a/client/src/leap/soledad/client/http_target/api.py +++ b/client/src/leap/soledad/client/http_target/api.py @@ -40,9 +40,8 @@ class SyncTargetAPI(SyncTarget): Declares public methods and implements u1db.SyncTarget. """ - @defer.inlineCallbacks def close(self): - yield self._http.close() + return self._http.close() @property def uuid(self): @@ -75,6 +74,10 @@ class SyncTargetAPI(SyncTarget): if not body_producer: d = self._http.request(url, method, body, headers, body_reader) else: + # Upload case, check send.py + # Used to lazy produce body from docs with a custom protocol + # FIXME: _agent usage to bypass timeout, there is an ongoing + # discussion on how to properly do it. d = self._http._agent.request( method, url, headers=Headers(headers), bodyProducer=body_producer(body)) diff --git a/client/src/leap/soledad/client/http_target/fetch.py b/client/src/leap/soledad/client/http_target/fetch.py index 1b4351ea..036b5b21 100644 --- a/client/src/leap/soledad/client/http_target/fetch.py +++ b/client/src/leap/soledad/client/http_target/fetch.py @@ -53,19 +53,21 @@ class HTTPDocFetcher(object): ensure_callback, sync_id): new_generation = last_known_generation new_transaction_id = last_known_trans_id + self._received_docs = 0 # Acts as a queue, ensuring line order on async processing + # as `self._insert_doc_cb` cant be run concurrently or out of order. + # DeferredSemaphore solves the concurrency and its implementation uses + # a queue, solving the ordering. + # FIXME: Find a proper solution to avoid surprises on Twisted changes self.semaphore = defer.DeferredSemaphore(1) - # we fetch the first document before fetching the rest because we need - # to know the total number of documents to be received, and this - # information comes as metadata to each request. - - self._received_docs = 0 metadata = yield self._fetch_all( last_known_generation, last_known_trans_id, sync_id, self._received_docs) - number_of_changes, ngen, ntrans =\ - self._parse_metadata(metadata) + metadata = self._parse_metadata(metadata) + number_of_changes, ngen, ntrans = metadata + + # wait for pending inserts yield self.semaphore.acquire() if ngen: @@ -106,8 +108,8 @@ class HTTPDocFetcher(object): :param total: The total number of operations. :type total: int """ - # If arriving content was symmetrically encrypted, we decrypt - # decrypt incoming document and insert into local database + # If arriving content was symmetrically encrypted, we decrypt incoming + # document and insert into local database doc = SoledadDocument(doc_info['id'], doc_info['rev'], content) @@ -117,10 +119,10 @@ class HTTPDocFetcher(object): doc.set_json(content) # TODO insert blobs here on the blob backend - # FIXME: This is wrong. Using a SQLite connection from multiple threads - # is dangerous. We should bring the dbpool here or find an alternative. - # Current fix only helps releasing the reactor for other tasks as this - # is an IO intensive call. + # FIXME: This is wrong. Using the very same SQLite connection object + # from multiple threads is dangerous. We should bring the dbpool here + # or find an alternative. Deferring to a thread only helps releasing + # the reactor for other tasks as this is an IO intensive call. yield self.semaphore.run(threads.deferToThread, self._insert_doc_cb, doc, doc_info['gen'], doc_info['trans_id']) self._received_docs += 1 diff --git a/client/src/leap/soledad/client/http_target/fetch_protocol.py b/client/src/leap/soledad/client/http_target/fetch_protocol.py index dac82d8e..29801819 100644 --- a/client/src/leap/soledad/client/http_target/fetch_protocol.py +++ b/client/src/leap/soledad/client/http_target/fetch_protocol.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -# support.py +# fetch_protocol.py # Copyright (C) 2016 LEAP # # This program is free software: you can redistribute it and/or modify -- cgit v1.2.3