From 2685fe29b19bbf6d5a75a0d640d2baef173f18f6 Mon Sep 17 00:00:00 2001 From: Peter Surda Date: Sat, 24 Jun 2017 12:13:35 +0200 Subject: [PATCH] Code quality improvements --- src/bitmessagemain.py | 1 - src/bitmessageqt/safehtmlparser.py | 6 +++--- src/bmconfigparser.py | 13 ++++++------- src/network/advanceddispatcher.py | 8 +++----- src/network/announcethread.py | 5 ----- src/network/asyncore_pollchoose.py | 5 ----- src/network/bmobject.py | 6 +++--- src/network/bmproto.py | 26 +++++++++++--------------- src/network/connectionchooser.py | 17 ++++++++--------- src/network/connectionpool.py | 1 - src/network/downloadthread.py | 6 +----- src/network/invthread.py | 16 ++++++++-------- src/network/stats.py | 23 ++++++++--------------- src/network/tls.py | 18 ++++++++---------- src/network/udp.py | 25 ++++--------------------- src/storage/sqlite.py | 2 +- src/storage/storage.py | 2 +- 17 files changed, 65 insertions(+), 115 deletions(-) diff --git a/src/bitmessagemain.py b/src/bitmessagemain.py index 40ccdee3..a2d951ed 100755 --- a/src/bitmessagemain.py +++ b/src/bitmessagemain.py @@ -27,7 +27,6 @@ import socket import ctypes from struct import pack from subprocess import call -import time from api import MySimpleXMLRPCRequestHandler, StoppableXMLRPCServer from helper_startup import isOurOperatingSystemLimitedToHavingVeryFewHalfOpenConnections diff --git a/src/bitmessageqt/safehtmlparser.py b/src/bitmessageqt/safehtmlparser.py index a78991d3..88431855 100644 --- a/src/bitmessageqt/safehtmlparser.py +++ b/src/bitmessageqt/safehtmlparser.py @@ -53,20 +53,20 @@ class SafeHTMLParser(HTMLParser): self.allow_external_src = False def add_if_acceptable(self, tag, attrs = None): - if not tag in SafeHTMLParser.acceptable_elements: + if tag not in SafeHTMLParser.acceptable_elements: return self.sanitised += "<" if inspect.stack()[1][3] == "handle_endtag": self.sanitised += "/" self.sanitised += tag - if not attrs is None: + if attrs is not None: for attr, val in attrs: if tag == "img" and attr == "src" and not self.allow_picture: val = "" elif attr == "src" and not self.allow_external_src: url = urlparse(val) if url.scheme not in SafeHTMLParser.src_schemes: - val == "" + val = "" self.sanitised += " " + quote_plus(attr) if not (val is None): self.sanitised += "=\"" + val + "\"" diff --git a/src/bmconfigparser.py b/src/bmconfigparser.py index bfa7e396..0db58103 100644 --- a/src/bmconfigparser.py +++ b/src/bmconfigparser.py @@ -36,14 +36,13 @@ class BMConfigParser(ConfigParser.SafeConfigParser): raise TypeError("option values must be strings") return ConfigParser.ConfigParser.set(self, section, option, value) - def get(self, section, option, raw=False, vars=None): + def get(self, section, option, raw=False, variables=None): try: if section == "bitmessagesettings" and option == "timeformat": - return ConfigParser.ConfigParser.get(self, section, option, raw, vars) - else: - return ConfigParser.ConfigParser.get(self, section, option, True, vars) + return ConfigParser.ConfigParser.get(self, section, option, raw, variables) + return ConfigParser.ConfigParser.get(self, section, option, True, variables) except ConfigParser.InterpolationError: - return ConfigParser.ConfigParser.get(self, section, option, True, vars) + return ConfigParser.ConfigParser.get(self, section, option, True, variables) except (ConfigParser.NoSectionError, ConfigParser.NoOptionError) as e: try: return BMConfigDefaults[section][option] @@ -68,8 +67,8 @@ class BMConfigParser(ConfigParser.SafeConfigParser): except (ConfigParser.NoSectionError, ConfigParser.NoOptionError, ValueError, AttributeError): return default - def items(self, section, raw=False, vars=None): - return ConfigParser.ConfigParser.items(self, section, True, vars) + def items(self, section, raw=False, variables=None): + return ConfigParser.ConfigParser.items(self, section, True, variables) def addresses(self): return filter(lambda x: x.startswith('BM-'), BMConfigParser().sections()) diff --git a/src/network/advanceddispatcher.py b/src/network/advanceddispatcher.py index 80762aa5..61dd197b 100644 --- a/src/network/advanceddispatcher.py +++ b/src/network/advanceddispatcher.py @@ -5,7 +5,6 @@ import time import asyncore_pollchoose as asyncore from debug import logger -from bmconfigparser import BMConfigParser class AdvancedDispatcher(asyncore.dispatcher): _buf_len = 2097152 # 2MB @@ -34,11 +33,10 @@ class AdvancedDispatcher(asyncore.dispatcher): def read_buf_sufficient(self, length=0): if len(self.read_buf) < length: return False - else: - return True + return True def process(self): - if self.state != "tls_handshake" and len(self.read_buf) == 0: + if self.state != "tls_handshake" and not self.read_buf: return if not self.connected: return @@ -100,7 +98,7 @@ class AdvancedDispatcher(asyncore.dispatcher): break if bufSize <= 0: return - if len(self.write_buf) > 0: + if self.write_buf: written = self.send(self.write_buf[0:bufSize]) asyncore.update_sent(written) self.sentBytes += written diff --git a/src/network/announcethread.py b/src/network/announcethread.py index 37b7e628..3adcae48 100644 --- a/src/network/announcethread.py +++ b/src/network/announcethread.py @@ -1,4 +1,3 @@ -import Queue import threading import time @@ -8,7 +7,6 @@ from helper_threading import StoppableThread from network.bmproto import BMProto from network.connectionpool import BMConnectionPool from network.udp import UDPSocket -import protocol import state class AnnounceThread(threading.Thread, StoppableThread): @@ -33,6 +31,3 @@ class AnnounceThread(threading.Thread, StoppableThread): for stream in state.streamsInWhichIAmParticipating: addr = (stream, state.Peer('127.0.0.1', BMConfigParser().safeGetInt("bitmessagesettings", "port")), time.time()) connection.writeQueue.put(BMProto.assembleAddr([addr])) - - def stopThread(self): - super(AnnounceThread, self).stopThread() diff --git a/src/network/asyncore_pollchoose.py b/src/network/asyncore_pollchoose.py index 94cbfaf3..3f188812 100644 --- a/src/network/asyncore_pollchoose.py +++ b/src/network/asyncore_pollchoose.py @@ -604,11 +604,6 @@ class dispatcher: # cheap inheritance, used to pass all other attribute # references to the underlying socket object. def __getattr__(self, attr): - try: - sys._getframe(200) - logger.error("Stack depth warning") - except ValueError: - pass try: retattr = getattr(self.socket, attr) except AttributeError: diff --git a/src/network/bmobject.py b/src/network/bmobject.py index e16a6937..318cbfd1 100644 --- a/src/network/bmobject.py +++ b/src/network/bmobject.py @@ -51,18 +51,18 @@ class BMObject(object): def checkEOLSanity(self): # EOL sanity check if self.expiresTime - int(time.time()) > BMObject.maxTTL: - logger.info('This object\'s End of Life time is too far in the future. Ignoring it. Time is %s' % self.expiresTime) + logger.info('This object\'s End of Life time is too far in the future. Ignoring it. Time is %i', self.expiresTime) # TODO: remove from download queue raise BMObjectExpiredError() if self.expiresTime - int(time.time()) < BMObject.minTTL: - logger.info('This object\'s End of Life time was too long ago. Ignoring the object. Time is %s' % self.expiresTime) + logger.info('This object\'s End of Life time was too long ago. Ignoring the object. Time is %i', self.expiresTime) # TODO: remove from download queue raise BMObjectExpiredError() def checkStream(self): if self.streamNumber not in state.streamsInWhichIAmParticipating: - logger.debug('The streamNumber %s isn\'t one we are interested in.' % self.streamNumber) + logger.debug('The streamNumber %i isn\'t one we are interested in.', self.streamNumber) raise BMObjectUnwantedStreamError() def checkAlreadyHave(self): diff --git a/src/network/bmproto.py b/src/network/bmproto.py index 39464845..32bf038c 100644 --- a/src/network/bmproto.py +++ b/src/network/bmproto.py @@ -15,14 +15,12 @@ from network.bmobject import BMObject, BMObjectInsufficientPOWError, BMObjectInv import network.connectionpool from network.downloadqueue import DownloadQueue from network.node import Node -import network.asyncore_pollchoose as asyncore from network.objectracker import ObjectTracker from network.proxy import Proxy, ProxyError, GeneralProxyError -from network.uploadqueue import UploadQueue, UploadElem, AddrUploadQueue, ObjUploadQueue import addresses from bmconfigparser import BMConfigParser -from queues import objectProcessorQueue, portCheckerQueue, UISignalQueue, invQueue +from queues import objectProcessorQueue, portCheckerQueue, invQueue import shared import state import protocol @@ -173,7 +171,6 @@ class BMProto(AdvancedDispatcher, ObjectTracker): retval = [] size = None - insideDigit = False i = 0 while i < len(pattern): @@ -353,14 +350,13 @@ class BMProto(AdvancedDispatcher, ObjectTracker): self.set_state("tls_init", self.payloadLength) self.bm_proto_reset() return False - else: - self.set_connection_fully_established() - return True + self.set_connection_fully_established() + return True return True def bm_command_version(self): - #self.remoteProtocolVersion, self.services, self.timestamp, padding1, self.myExternalIP, padding2, self.remoteNodeIncomingPort = protocol.VersionPacket.unpack(self.payload[:protocol.VersionPacket.size]) - self.remoteProtocolVersion, self.services, self.timestamp, self.sockNode, self.peerNode, self.nonce, self.userAgent, self.streams = self.decode_payload_content("IQQiiQlslv") + self.remoteProtocolVersion, self.services, self.timestamp, self.sockNode, self.peerNode, self.nonce, \ + self.userAgent, self.streams = self.decode_payload_content("IQQiiQlslv") self.nonce = struct.pack('>Q', self.nonce) self.timeOffset = self.timestamp - int(time.time()) logger.debug("remoteProtocolVersion: %i", self.remoteProtocolVersion) @@ -377,7 +373,8 @@ class BMProto(AdvancedDispatcher, ObjectTracker): self.writeQueue.put(protocol.CreatePacket('verack')) self.verackSent = True if not self.isOutbound: - self.writeQueue.put(protocol.assembleVersionMessage(self.destination.host, self.destination.port, network.connectionpool.BMConnectionPool().streams, True)) + self.writeQueue.put(protocol.assembleVersionMessage(self.destination.host, self.destination.port, \ + network.connectionpool.BMConnectionPool().streams, True)) #print "%s:%i: Sending version" % (self.destination.host, self.destination.port) if ((self.services & protocol.NODE_SSL == protocol.NODE_SSL) and protocol.haveSSL(not self.isOutbound)): @@ -387,9 +384,8 @@ class BMProto(AdvancedDispatcher, ObjectTracker): self.set_state("tls_init", self.payloadLength) self.bm_proto_reset() return False - else: - self.set_connection_fully_established() - return True + self.set_connection_fully_established() + return True return True def peerValidityChecks(self): @@ -415,7 +411,7 @@ class BMProto(AdvancedDispatcher, ObjectTracker): return False else: shared.timeOffsetWrongCount = 0 - if len(self.streams) == 0: + if not self.streams: self.writeQueue.put(protocol.assembleErrorMessage(fatal=2, errorText="We don't have shared stream interests. Closing connection.")) logger.debug ('Closed connection to %s because there is no overlapping interest in streams.', @@ -442,7 +438,7 @@ class BMProto(AdvancedDispatcher, ObjectTracker): @staticmethod def assembleAddr(peerList): - if type(peerList) is state.Peer: + if isinstance(peerList, state.Peer): peerList = (peerList) # TODO handle max length, now it's done by upper layers payload = addresses.encodeVarint(len(peerList)) diff --git a/src/network/connectionchooser.py b/src/network/connectionchooser.py index 1e26b994..0770bfa6 100644 --- a/src/network/connectionchooser.py +++ b/src/network/connectionchooser.py @@ -9,14 +9,13 @@ import state def chooseConnection(stream): if state.trustedPeer: return state.trustedPeer - else: + try: + retval = portCheckerQueue.get(False) + portCheckerQueue.task_done() + except Queue.Empty: try: - retval = portCheckerQueue.get(False) - portCheckerQueue.task_done() + retval = peerDiscoveryQueue.get(False) + peerDiscoveryQueue.task_done() except Queue.Empty: - try: - retval = peerDiscoveryQueue.get(False) - peerDiscoveryQueue.task_done() - except Queue.Empty: - return random.choice(knownnodes.knownNodes[stream].keys()) - return retval + return random.choice(knownnodes.knownNodes[stream].keys()) + return retval diff --git a/src/network/connectionpool.py b/src/network/connectionpool.py index 41fb97be..67778b46 100644 --- a/src/network/connectionpool.py +++ b/src/network/connectionpool.py @@ -15,7 +15,6 @@ from network.connectionchooser import chooseConnection import network.asyncore_pollchoose as asyncore import protocol from singleton import Singleton -import shared import state @Singleton diff --git a/src/network/downloadthread.py b/src/network/downloadthread.py index e8bd44a7..a8d3e1f7 100644 --- a/src/network/downloadthread.py +++ b/src/network/downloadthread.py @@ -1,4 +1,3 @@ -import Queue import threading import addresses @@ -30,7 +29,7 @@ class DownloadThread(threading.Thread, StoppableThread): continue # keys with True values in the dict request = list((k for k, v in i.objectsNewToMe.iteritems() if v)) - if len(request) == 0: + if not request: continue if len(request) > DownloadThread.requestChunk - downloadPending: request = request[:DownloadThread.requestChunk - downloadPending] @@ -43,6 +42,3 @@ class DownloadThread(threading.Thread, StoppableThread): logger.debug("%s:%i Requesting %i objects", i.destination.host, i.destination.port, len(request)) requested += len(request) self.stop.wait(1) - - def stopThread(self): - super(DownloadThread, self).stopThread() diff --git a/src/network/invthread.py b/src/network/invthread.py index 63107a1f..9d05aec4 100644 --- a/src/network/invthread.py +++ b/src/network/invthread.py @@ -36,14 +36,14 @@ class InvThread(threading.Thread, StoppableThread): try: data = invQueue.get(False) if len(data) == 2: - BMConnectionPool().handleReceivedObject(data[0], data[1]) + BMConnectionPool().handleReceivedObject(data[0], data[1]) else: - BMConnectionPool().handleReceivedObject(data[0], data[1], data[2]) + BMConnectionPool().handleReceivedObject(data[0], data[1], data[2]) self.holdHash (data[0], data[1]) except Queue.Empty: break - if len(self.collectionOfInvs[iterator]) > 0: + if self.collectionOfInvs[iterator]: for connection in BMConnectionPool().inboundConnections.values() + BMConnectionPool().outboundConnections.values(): hashes = [] for stream in connection.streams: @@ -57,23 +57,23 @@ class InvThread(threading.Thread, StoppableThread): pass except KeyError: continue - if len(hashes) > 0: + if hashes: connection.writeQueue.put(protocol.CreatePacket('inv', addresses.encodeVarint(len(hashes)) + "".join(hashes))) self.collectionOfInvs[iterator] = {} iterator += 1 iterator %= InvThread.size self.stop.wait(1) - def holdHash(self, stream, hash): + def holdHash(self, stream, hashId): i = random.randrange(0, InvThread.size) if stream not in self.collectionOfInvs[i]: self.collectionOfInvs[i][stream] = [] - self.collectionOfInvs[i][stream].append(hash) + self.collectionOfInvs[i][stream].append(hashId) - def hasHash(self, hash): + def hasHash(self, hashId): for streamlist in self.collectionOfInvs: for stream in streamlist: - if hash in streamlist[stream]: + if hashId in streamlist[stream]: return True return False diff --git a/src/network/stats.py b/src/network/stats.py index b348098c..e17d1041 100644 --- a/src/network/stats.py +++ b/src/network/stats.py @@ -18,21 +18,19 @@ def connectedHostsList(): if BMConfigParser().get("network", "asyncore"): retval = [] for i in BMConnectionPool().inboundConnections.values() + BMConnectionPool().outboundConnections.values(): - if not i.connected: + if not i.fullyEstablished: continue try: retval.append((i.destination, i.streams[0])) except AttributeError: pass return retval - else: - return shared.connectedHostsList.items() + return shared.connectedHostsList.items() def sentBytes(): if BMConfigParser().get("network", "asyncore"): return asyncore.sentBytes - else: - return throttle.SendThrottle().total + return throttle.SendThrottle().total def uploadSpeed(): global lastSentTimestamp, lastSentBytes, currentSentSpeed @@ -44,14 +42,12 @@ def uploadSpeed(): lastSentBytes = currentSentBytes lastSentTimestamp = currentTimestamp return currentSentSpeed - else: - return throttle.sendThrottle().getSpeed() + return throttle.sendThrottle().getSpeed() def receivedBytes(): if BMConfigParser().get("network", "asyncore"): return asyncore.receivedBytes - else: - return throttle.ReceiveThrottle().total + return throttle.ReceiveThrottle().total def downloadSpeed(): global lastReceivedTimestamp, lastReceivedBytes, currentReceivedSpeed @@ -63,8 +59,7 @@ def downloadSpeed(): lastReceivedBytes = currentReceivedBytes lastReceivedTimestamp = currentTimestamp return currentReceivedSpeed - else: - return throttle.ReceiveThrottle().getSpeed() + return throttle.ReceiveThrottle().getSpeed() def pendingDownload(): if BMConfigParser().get("network", "asyncore"): @@ -73,8 +68,7 @@ def pendingDownload(): for k in connection.objectsNewToMe.keys(): tmp[k] = True return len(tmp) - else: - return PendingDownloadQueue.totalSize() + return PendingDownloadQueue.totalSize() def pendingUpload(): if BMConfigParser().get("network", "asyncore"): @@ -84,5 +78,4 @@ def pendingUpload(): for k in connection.objectsNewToThem.keys(): tmp[k] = True return len(tmp) - else: - return PendingUpload().len() + return PendingUpload().len() diff --git a/src/network/tls.py b/src/network/tls.py index 9694b4b9..5a8a5a59 100644 --- a/src/network/tls.py +++ b/src/network/tls.py @@ -63,28 +63,26 @@ class TLSDispatcher(AdvancedDispatcher): def writable(self): try: - if self.tlsStarted and not self.tlsDone and len(self.write_buf) == 0 and self.writeQueue.empty(): + if self.tlsStarted and not self.tlsDone and not self.write_buf and self.writeQueue.empty(): #print "tls writable, %r" % (self.want_write) return self.want_write - else: - return AdvancedDispatcher.writable(self) + return AdvancedDispatcher.writable(self) except AttributeError: return AdvancedDispatcher.writable(self) def readable(self): try: - if self.tlsStarted and not self.tlsDone and len(self.write_buf) == 0 and self.writeQueue.empty(): + if self.tlsStarted and not self.tlsDone and not self.write_buf and self.writeQueue.empty(): #print "tls readable, %r" % (self.want_read) return self.want_read - else: - return AdvancedDispatcher.readable(self) + return AdvancedDispatcher.readable(self) except AttributeError: return AdvancedDispatcher.readable(self) def handle_read(self): try: # wait for write buffer flush - if self.tlsStarted and not self.tlsDone and len(self.write_buf) == 0 and self.writeQueue.empty(): + if self.tlsStarted and not self.tlsDone and not self.write_buf and self.writeQueue.empty(): #print "handshaking (read)" self.tls_handshake() else: @@ -104,7 +102,7 @@ class TLSDispatcher(AdvancedDispatcher): def handle_write(self): try: # wait for write buffer flush - if self.tlsStarted and not self.tlsDone and len(self.write_buf) == 0 and self.writeQueue.empty(): + if self.tlsStarted and not self.tlsDone and not self.write_buf and self.writeQueue.empty(): #print "handshaking (write)" self.tls_handshake() else: @@ -123,13 +121,13 @@ class TLSDispatcher(AdvancedDispatcher): def tls_handshake(self): # wait for flush - if len(self.write_buf) > 0: + if self.write_buf: return False # Perform the handshake. try: #print "handshaking (internal)" self.sslSocket.do_handshake() - except ssl.SSLError, err: + except ssl.SSLError as err: #print "%s:%i: handshake fail" % (self.destination.host, self.destination.port) self.want_read = self.want_write = False if err.args[0] == ssl.SSL_ERROR_WANT_READ: diff --git a/src/network/udp.py b/src/network/udp.py index 9eccbf67..6770e5a0 100644 --- a/src/network/udp.py +++ b/src/network/udp.py @@ -1,32 +1,15 @@ -import base64 -from binascii import hexlify -import hashlib -import math import time import Queue import socket -import struct -import random -import traceback -from addresses import calculateInventoryHash from debug import logger -from inventory import Inventory -import knownnodes from network.advanceddispatcher import AdvancedDispatcher -from network.bmproto import BMProtoError, BMProtoInsufficientDataError, BMProtoExcessiveDataError, BMProto -from network.bmobject import BMObject, BMObjectInsufficientPOWError, BMObjectInvalidDataError, BMObjectExpiredError, BMObjectUnwantedStreamError, BMObjectInvalidError, BMObjectAlreadyHaveError -import network.connectionpool -from network.downloadqueue import DownloadQueue -from network.node import Node +from network.bmproto import BMProtoError, BMProtoInsufficientDataError, BMProto +from network.bmobject import BMObject, BMObjectInsufficientPOWError, BMObjectInvalidDataError, BMObjectExpiredError, BMObjectInvalidError, BMObjectAlreadyHaveError import network.asyncore_pollchoose as asyncore from network.objectracker import ObjectTracker -from network.uploadqueue import UploadQueue, UploadElem, AddrUploadQueue, ObjUploadQueue -import addresses -from bmconfigparser import BMConfigParser -from queues import objectProcessorQueue, peerDiscoveryQueue, portCheckerQueue, UISignalQueue -import shared +from queues import objectProcessorQueue, peerDiscoveryQueue, UISignalQueue import state import protocol @@ -35,7 +18,7 @@ class UDPSocket(BMProto): announceInterval = 60 def __init__(self, host=None, sock=None): - BMProto.__init__(self, sock) + super(BMProto, self).__init__(sock=sock) self.verackReceived = True self.verackSent = True # TODO sort out streams diff --git a/src/storage/sqlite.py b/src/storage/sqlite.py index 96733a36..03c80e49 100644 --- a/src/storage/sqlite.py +++ b/src/storage/sqlite.py @@ -47,7 +47,7 @@ class SqliteInventory(InventoryStorage): with self.lock: return len(self._inventory) + sqlQuery('SELECT count(*) FROM inventory')[0][0] - def by_type_and_tag(self, type, tag): + def by_type_and_tag(self, objectType, tag): with self.lock: values = [value for value in self._inventory.values() if value.type == type and value.tag == tag] values += (InventoryItem(*value) for value in sqlQuery('SELECT objecttype, streamnumber, payload, expirestime, tag FROM inventory WHERE objecttype=? AND tag=?', type, tag)) diff --git a/src/storage/storage.py b/src/storage/storage.py index 22dfbef5..302c84a0 100644 --- a/src/storage/storage.py +++ b/src/storage/storage.py @@ -30,7 +30,7 @@ class InventoryStorage(Storage, collections.MutableMapping): def __len__(self): raise NotImplementedError - def by_type_and_tag(self, type, tag): + def by_type_and_tag(self, objectType, tag): raise NotImplementedError def hashes_by_stream(self, stream):