dbpedia / ontology-time-machine

5 stars 2 forks source link

abstract all logic from the underlying proxy system and proxy data objects using wrappers #85

Closed JJ-Author closed 1 month ago

JJ-Author commented 3 months ago

the code is still not abstracted from the proxy.py internals as we agreed to upon the beginning once it was proposed to use mitmproxy but Jenifer wanted to stick/use proxy.py

one example is this here if (request.method == b'GET' or request.method == b'HEAD') and not request.host: that is a highly implementation-specific condition that only works in proxy.py (maybe even limited to work until a bug is fixed) also these binary comparisons might not work with another framework.

so what needs to be done is writing wrappers for the objects that are read or modified or functions that are called. e.g. a wrapper like this for the http request object with getters and setters that are needed from the code. as consequence all code that is specific to proxy.py should be located at "one place" and not spread over all the code furthermore the logic of the proxy only works on logical conditions via wrapper functions.

class PPRequestWrapper(AbstractRequestWrapper):
    def __init__(self, request):
        self.request = request

    def is_get(self):
        return self.request.method == b'GET'

    def is_head(self):
        return self.request.method == b'HEAD'

    def is_https(self):
        return request.host: == None

from abc import ABC, abstractmethod
from typing import Any

class AbstractRequestWrapper(ABC):
    def __init__(self, request: Any):
        self.request = request

    @abstractmethod
    def is_get(self) -> bool:
        pass

    @abstractmethod
    def is_head(self) -> bool:
        pass

    @abstractmethod
    def is_https(self) -> bool:
        pass
JJ-Author commented 3 months ago

@white-gecko can you check please whether this is correct python development pattern (you know I am more the java guy) also you mentioned this http library that abstracts from different implementations so would be good to give some guidance here since doing this right is essential for maintainability and sustainability of the project. i also wonder how it is best achieve some type safety such that it easier to find issues if an implementation-specific wrapper is not in conformance with the abstract class - meaning it does not return the proper object structure.

JJ-Author commented 3 months ago

the huge advantage is also that you can basically define your own API and test that the proxy-wrapper is in accordance with the API "spec". so on updating to a new release of e.g. proxy.py you can detect e.g. with a test for the is_https method whether an https connection is still recognizable via the dirty host==None hack.

white-gecko commented 3 months ago

For me, this pattern looks good.