Mutual friends finder written in Python

Question

I've written a server/client tool which can "check the Six degrees of separation theory" in Russian social network VK.com. Client sends task (which contains two VK IDs) and Server tries to find all connections between these two IDs. I've also written a simple HTTP client just to practice working with sockets.

HTTPClient.py:

import socket
import re


def get(host, port, address):
    s = socket.socket()
    s.connect((host,port))
    s.sendall("GET %s HTTP/1.0\nHost: %s\n\n" % (address, host))
    answer = ""
    symbol = s.recv(1024576)
    while symbol != '':
        answer += symbol
        symbol = s.recv(1024576)
    s.close()
    answer = re.search("\r\n\r\n(.+)", answer, re.DOTALL).group(1)
    return answer

VKApi.py:

import HTTPClient
import json
import re
import types
import md5
import time

class BaseAPIException(Exception):

    def __init__(self, text=''):
        Exception.__init__(self,text)


class LoginException(BaseAPIException):
    pass

class MethodExecutionException(BaseAPIException):
    pass


class VKApi:

    def __init__(self,appId,scope=[],callbackUrl='https://oauth.vk.com/blank.html'):
        self.appId = appId
        if ( not isinstance(scope,types.ListType)): raise Exception("Wrong args")
        scope.append('nohttps')
        self.scope = ','.join(scope)
        self.callbackUrl=callbackUrl
        self.isLoggedIn = False
        self.callsCount = 0

    def getAuthorizeLink(self):
        return "https://oauth.vk.com/authorize?client_id=%s&scope=%s&redirect_uri=%s&display=page&v=5.21&response_type=token" % \
               (self.appId, self.scope, self.callbackUrl)

    def login(self, link):
        tokenRegExp = re.search('access_token=(.*?)(&|$)',link)
        if tokenRegExp == None: raise LoginException('No token found')
        self.token = tokenRegExp.group(1)
        secretRegExp = re.search('secret=(.*?)(&|$)',link)
        if secretRegExp == None: raise LoginException('No secret found')
        self.secret = secretRegExp.group(1);
        self.isLoggedIn = True

    def loginByToken(self,token,secret):
        self.token=token
        self.secret=secret
        self.isLoggedIn = True

    def callMethod(self,method,args={}):
        if not self.isLoggedIn: raise MethodExecutionException('Not logged in')
        if self.callsCount == 3:
            time.sleep(1)
            self.callsCount = 0
        self.callsCount += 1
        link = '/method/%s?' % method
        link += '&'.join([ '%s=%s' % (k,v) for (k,v) in args.items()])
        if len(args)>0: link += '&'
        link += 'access_token=%s' % self.token
        signature = md5.md5(link + self.secret).hexdigest()
        link += '&sig=%s' % signature
        try:
            response = HTTPClient.get('api.vk.com',80,link)
        except Exception as e:
            raise MethodExecutionException(e.message)
        responseObj = json.loads(response)
        if 'error' in responseObj: raise MethodExecutionException(responseObj['error']['error_msg'])
        return responseObj['response']

VKHandshakeChecker.py:

import VKApi
import threading


class VKHandshakeChecker:

    @property
    def answers(self):
        return self.__answers

    @property
    def status(self):
        return self.__status    

    @property
    def currentSequence(self):
        return self.__currentSequence

    def __init__(self,vkapi,maxDepth=6,cache={}):
        isinstance(vkapi,VKApi.VKApi)
        self.vkapi = vkapi
        self.__maxDepth=maxDepth
        self.__answers = list()
        self.__alreadyBeen = dict()
        self.__status = 'IDLE'
        self.__canwork = True
        self.__cache = cache

    def startFinding(self,firstId,secondId):
        self.__thread = threading.Thread(target=VKHandshakeChecker.__findHandshakes,args=(self,firstId,secondId))
        self.__thread.start()

    def stopFinding(self):
        self.__canwork = False

    def __findHandshakes(self,firstId,secondId):
        self.__status = 'WORKING'
        self.__answers = list()   
        for depth in range(2,self.__maxDepth):
            self.__currentSequence = [firstId]           
            self.__findHandshakesRecursive(secondId,depth)
        self.__status = 'OK'

    def __findHandshakesRecursive(self,finalId,depth):
        if self.__canwork == False:
            return
        if len(self.__currentSequence) == depth:
            if (self.currentSequence[-1] == finalId):
                self.__answers.append(list(self.__currentSequence))
        else:
            for friendId in self.__getFriends(self.__currentSequence[-1]):
                self.__currentSequence.append(friendId)
                self.__findHandshakesRecursive(finalId,depth)
                self.__currentSequence.pop()
        return


    def __getFriends(self,uid):     
        if uid in self.__cache:
            return self.__cache[uid]
        try:
            response = self.vkapi.callMethod('friends.get',{'user_id':uid})
        except:
            response = []
        self.__cache[uid] = response
        return response

VKHandshakeServer.py

import VKHandshakeChecker
import VKApi
import socket
import threading
import cPickle as pickle
import sys
import random
import string

def handle(sock):
    files = sock.makefile('rb+')
    request = pickle.load(files)
    response = {'header': 'ERROR', 'body':'Non-existent command'}
    if request['header'] == 'START':
        response = start(request['body'])
    elif request['header'] == 'STOP':
        response = stop(request['body'])
    elif request['header'] == 'GETCURRENTSEQUENCE':
        response = getCurrentSequence(request['body'])
    elif request['header'] == 'GETANSWERS':
        response = getAnswers(request['body']) 
    elif request['header'] == 'GETSTATUS':
        response = getStatus(request['body'])    
    pickle.dump(response,files)
    files.flush()
    sock.close()

def start(request):
    try:
        objectId = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(5))
        checker = VKHandshakeChecker.VKHandshakeChecker(request['api'], cache=cache)
        checker.startFinding(request['firstId'],request['secondId'])
        checkers[objectId] = checker
        return {'header': 'OK', 'body': {'id': objectId}}
    except Exception as e:
        return {'header': 'ERROR', 'body': repr(e)}

def stop(request):
    try:
        checkers[request['id']].stopFinding()
        del checkers[request['id']]
    finally:
        return {'header': 'OK', 'body': 'OK'}

def getCurrentSequence(request):
    try:
        checker = checkers[request['id']]
        curSeq = checker.currentSequence
        return {'header': 'OK', 'body': curSeq}
    except Exception, e:
        return {'header': 'ERROR', 'body': repr(e)}

def getAnswers(request):
    try:
        checker = checkers[request['id']]
        answers = checker.answers
        return {'header': 'OK', 'body': answers}
    except Exception, e:
        return {'header': 'ERROR', 'body': repr(e)}    

def getStatus(request):
    try:
        checker = checkers[request['id']]
        status = checker.status
        return {'header': 'OK', 'body': status}
    except Exception, e:
        return {'header': 'ERROR', 'body': repr(e)}    


checkers = dict()
cache = dict()
host, port = sys.argv[1:3]
port=int(port)
s = socket.socket()
s.bind((host,port))
s.listen(5)
while True:
    conn, addr = s.accept()
    threading.Thread(target=handle,args=(conn,)).start()

VKHandshakeClient.py:

import socket
import cPickle as pickle

def sendCommand(server, port, header, body=''):
    s = socket.socket()
    s.connect((server, int(port)))
    request = {'header':  header, 'body': body}
    files = s.makefile('rb+')
    pickle.dump(request,files)
    files.flush()
    response = pickle.load(files)
    if response['header'] == 'ERROR':
        raise Exception(response['body'])
    return response['body']

Answer 1

That's quite a lot of code to review. I'll review only some parts.

HTTPClient.py

Instead of using magic number 1024576 twice, it's better to give it a meaningful name and put it near the top of the file where it's easy to see, for example:

BUFSIZE = 1024576

In this piece of code:

symbol = s.recv(1024576)
while symbol != '':
    answer += symbol
    symbol = s.recv(1024576)

The name "symbol" is not great. A symbol usually suggests a single character, but here it's an entire buffer. And instead of checking for symbol != '', a more Pythonic way is to do not symbol. Finally, the statement symbol = s.recv(1024576) is repeated twice. A better way to write the same thing:

while True:
    buf = s.recv(BUFSIZE)
    if not buf:
        break
    answer += buf

At the end of the method you strip the header from the response like this:

answer = re.search("\r\n\r\n(.+)", answer, re.DOTALL).group(1)
return answer

In my tests, some sites don't actually return a proper header, the expression will match nothing, and so the .group(1) will raise an error. To make it more robust, I recommend to rewrite like this:

return re.sub(".*\r\n\r\n", '', answer, flags=re.DOTALL)

Exception handling

This exception class can be simplified:

class BaseAPIException(Exception):
    def __init__(self, text=''):
        Exception.__init__(self,text)

This is equivalent:

class BaseAPIException(Exception):
    pass

Non-standard formatting

There's an official coding style guide called PEP8. You are violating it everywhere, making your code harder to read than it needs to be. For example:

def __init__(self,appId,scope=[],callbackUrl='https://oauth.vk.com/blank.html'):
    self.appId = appId
    if ( not isinstance(scope,types.ListType)): raise Exception("Wrong args")
    scope.append('nohttps')
    self.scope = ','.join(scope)
    self.callbackUrl=callbackUrl
    self.isLoggedIn = False
    self.callsCount = 0

This should be formatted like this to conform to the standard:

def __init__(self, appId, scope=[], callbackUrl='https://oauth.vk.com/blank.html'):
    self.appId = appId
    if not isinstance(scope, types.ListType):
        raise Exception("Wrong args")
    scope.append('nohttps')
    self.scope = ','.join(scope)
    self.callbackUrl = callbackUrl
    self.isLoggedIn = False
    self.callsCount = 0

Your code is full of this kind of violations. There is a command line utility called pep8, which you can install easily with pip install pep8, which can tell you all the violations.

Checking for None

Instead of this:

if tokenRegExp == None:

The recommended way is this:

if tokenRegExp is None:

Or even simpler:

if not tokenRegExp:

Answer 2

A six degrees of separation checker is a generic algorithm; it should be applicable to any graph. Yet the solution heavily depends on VKontakte internals. Do you want to rewrite the whole thing to test Odnoklassniki?

Distilling VKApi is a step in a right direction. Next step would be to isolate VKHandshakeChecker from VKApi (that is make it into a HandshakeChecker)