Reflect the difference between protocol-level and system-level functi…

…onality in the code (#383)

* Split methods on Node subtypes between regular Nodes and Nodes with diagnostic abilities (like the Imp)
* Factor out functionality from Imp that is useful in other WebFingerDiagClients -> AbstractWebFingerDiagClient
* Two exceptions are WebFinger-related not Web-related: move
* Make diag_override_http_response implementation consistent with turning on HTTP request logging
* Be clear that FediTest does not itself fetch ActivityPub documents, it only asks other Nodes to do it
* Better documentation and comments
* Comment-out some currently inconsistent code; get back to it with ActivityPub tests
* Make the sequence of class definitions in files Client, Server, not Server, Client: it's called client-server after all, not server-client.

---------

Co-authored-by: Johannes Ernst <git@j12t.org>

src/feditest/nodedrivers/__init__.py

@@ -30,7 +30,7 @@
 class Account(ABC):
     """
     The notion of an existing account on a Node. As different Nodes have different ideas about
-    what they know about an Account, this is an entirey abstract base class here.
+    what they know about an Account, this is an entirely abstract base class here.
     """
     def __init__(self, role: str | None):
         self._role = role

src/feditest/nodedrivers/fallback/fediverse.py

@@ -53,16 +53,6 @@
         https_uri_validate
 )
 
-"""
-Pre-existing accounts in TestPlans are specified as follows:
-* URI_KEY: URI that either is a WebFinger resource (e.g. acct:joe@example.com or https://example.com/ ) or an Actor URI
-* ROLE_KEY: optional account role
-* ACTOR_URI_KEY: optional https URI that points to the Actor. This is calculated by WebFinger lookup if not provided
-
-Known non-existing accounts are specified as follows:
-* URI_KEY: URI that neither is a WebFinger resource nor an Actor URI
-* ROLE_KEY: optional non-existing account role
-"""
 
 class FallbackFediverseAccount(Account):
     def __init__(self, role: str | None, uri: str, actor_uri: str | None):

src/feditest/nodedrivers/imp/__init__.py

@@ -2,20 +2,17 @@
 An in-process Node implementation for now.
 """
 
-from typing import cast
-
 import httpx
 from multidict import MultiDict
 
 from feditest.nodedrivers import AccountManager, Node, NodeConfiguration, NodeDriver, HOSTNAME_PAR
-from feditest.protocols.web import ParsedUri, WebClient
-from feditest.protocols.web.traffic import (
+from feditest.protocols.web.diag import (
     HttpRequest,
     HttpRequestResponsePair,
     HttpResponse,
+    WebDiagClient
 )
-from feditest.protocols.webfinger import WebFingerClient, WebFingerServer
-from feditest.protocols.webfinger.traffic import ClaimedJrd, WebFingerQueryResponse
+from feditest.protocols.webfinger.abstract import AbstractWebFingerDiagClient
 from feditest.reporting import trace
 from feditest.testplan import TestPlanConstellationNode, TestPlanNodeParameter
 from feditest.utils import FEDITEST_VERSION
@@ -25,95 +22,30 @@
     "Origin": "test.example" # to trigger CORS headers in response
 }
 
-class Imp(WebFingerClient):
+class Imp(AbstractWebFingerDiagClient):
     """
-    Our placeholder test client. Its future is to ~~tbd~~ be factored out of here.
+    In-process diagnostic WebFinger client.
     """
-    # use superclass constructor
-
-
-    # @override # from WebClient
+    # Python 3.12 @override
     def http(self, request: HttpRequest, follow_redirects: bool = True, verify=False) -> HttpRequestResponsePair:
         trace( f'Performing HTTP { request.method } on { request.uri.get_uri() }')
 
         httpx_response = None
         # Do not follow redirects automatically, we need to know whether there are any
-        with httpx.Client(verify=verify, follow_redirects=follow_redirects) as httpx_client:  # FIXME disable TLS cert verification for now
+        with httpx.Client(verify=verify, follow_redirects=follow_redirects) as httpx_client:
             httpx_request = httpx.Request(request.method, request.uri.get_uri(), headers=_HEADERS) # FIXME more arguments
             httpx_response = httpx_client.send(httpx_request)
 
+# FIXME: catch Tls exception and raise WebDiagClient.TlsError
+
         if httpx_response:
             response_headers : MultiDict = MultiDict()
             for key, value in httpx_response.headers.items():
                 response_headers.add(key.lower(), value)
             ret = HttpRequestResponsePair(request, request, HttpResponse(httpx_response.status_code, response_headers, httpx_response.read()))
             trace( f'HTTP query returns { ret }')
             return ret
-        raise WebClient.HttpUnsuccessfulError(request)
-
-
-    # @override # from WebFingerClient
-    def perform_webfinger_query(
-        self,
-        resource_uri: str,
-        rels: list[str] | None = None,
-        server: WebFingerServer | None = None
-    ) -> WebFingerQueryResponse:
-        query_url = self.construct_webfinger_uri_for(resource_uri, rels, server.hostname() if server else None )
-        parsed_uri = ParsedUri.parse(query_url)
-        if not parsed_uri:
-            raise ValueError('Not a valid URI:', query_url) # can't avoid this
-        first_request = HttpRequest(parsed_uri)
-        current_request = first_request
-        pair : HttpRequestResponsePair | None = None
-        for redirect_count in range(10, 0, -1):
-            pair = self.http(current_request)
-            if pair.response and pair.response.is_redirect():
-                if redirect_count <= 0:
-                    return WebFingerQueryResponse(pair, None, WebClient.TooManyRedirectsError(current_request))
-                parsed_location_uri = ParsedUri.parse(pair.response.location())
-                if not parsed_location_uri:
-                    return WebFingerQueryResponse(pair, None, ValueError('Location header is not a valid URI:', query_url, '(from', resource_uri, ')'))
-                current_request = HttpRequest(parsed_location_uri)
-            break
-
-        # I guess we always have a non-null responses here, but mypy complains without the cast
-        pair = cast(HttpRequestResponsePair, pair)
-        ret_pair = HttpRequestResponsePair(first_request, current_request, pair.response)
-        if ret_pair.response is None:
-            raise RuntimeError('Unexpected None HTTP response')
-
-        excs : list[Exception] = []
-        if ret_pair.response.http_status != 200:
-            excs.append(WebClient.WrongHttpStatusError(ret_pair))
-
-        content_type = ret_pair.response.content_type()
-        if (content_type is None or (content_type != "application/jrd+json"
-            and not content_type.startswith( "application/jrd+json;" ))
-        ):
-            excs.append(WebClient.WrongContentTypeError(ret_pair))
-
-        jrd : ClaimedJrd | None = None
-
-        if ret_pair.response.payload is None:
-            raise RuntimeError('Unexpected None payload in HTTP response')
-
-        try:
-            json_string = ret_pair.response.payload.decode(encoding=ret_pair.response.payload_charset() or "utf8")
-
-            jrd = ClaimedJrd(json_string) # May throw JSONDecodeError
-            jrd.validate() # May throw JrdError
-        except ExceptionGroup as exc:
-            excs += exc.exceptions
-        except Exception as exc:
-            excs.append(exc)
-
-        if len(excs) > 1:
-            return WebFingerQueryResponse(ret_pair, jrd, ExceptionGroup('WebFinger errors', excs))
-        elif len(excs) == 1:
-            return WebFingerQueryResponse(ret_pair, jrd, excs[0])
-        else:
-            return WebFingerQueryResponse(ret_pair, jrd, None)
+        raise WebDiagClient.HttpUnsuccessfulError(request)
 
 
     # Python 3.12 @override

src/feditest/nodedrivers/mastodon/__init__.py

@@ -11,7 +11,6 @@
 import time
 from typing import Any, Callable, cast
 
-from feditest import AssertionFailure, InteropLevel, SpecLevel
 from feditest.nodedrivers import (
     Account,
     AccountManager,
@@ -25,7 +24,6 @@
     APP_VERSION_PAR,
     HOSTNAME_PAR
 )
-from feditest.protocols.activitypub import AnyObject
 from feditest.protocols.fediverse import FediverseNode
 from feditest.reporting import is_trace_active, trace
 from feditest.testplan import InvalidAccountSpecificationException, TestPlanConstellationNode, TestPlanNodeAccountField, TestPlanNodeNonExistingAccountField, TestPlanNodeParameter
@@ -518,35 +516,38 @@ def obtain_actor_document_uri(self, rolename: str | None = None) -> str:
     # def obtain_following_collection_uri(self, actor_uri: str) -> str:
 
     # Python 3.12 @override
-    def assert_member_of_collection_at(
-        self,
-        candidate_member_uri: str,
-        collection_uri: str,
-        spec_level: SpecLevel | None = None,
-        interop_level: InteropLevel | None= None
-    ):
-        collection = AnyObject(collection_uri).as_collection()
-        if not collection.contains_item_with_id(candidate_member_uri):
-            raise AssertionFailure(
-                spec_level or SpecLevel.UNSPECIFIED,
-                interop_level or InteropLevel.UNKNOWN,
-                f"Node { self }: {candidate_member_uri} not in {collection_uri}")
-
 
-    # Python 3.12 @override
-    def assert_not_member_of_collection_at(
-        self,
-        candidate_member_uri: str,
-        collection_uri: str,
-        spec_level: SpecLevel | None = None,
-        interop_level: InteropLevel | None= None
-    ):
-        collection = AnyObject(collection_uri).as_collection()
-        if collection.contains_item_with_id(candidate_member_uri):
-            raise AssertionFailure(
-                spec_level or SpecLevel.UNSPECIFIED,
-                interop_level or InteropLevel.UNKNOWN,
-                f"Node { self }: {candidate_member_uri} must not be in {collection_uri}")
+    # Work in progress
+
+    # def assert_member_of_collection_at(
+    #     self,
+    #     candidate_member_uri: str,
+    #     collection_uri: str,
+    #     spec_level: SpecLevel | None = None,
+    #     interop_level: InteropLevel | None= None
+    # ):
+    #     collection = AnyObject(collection_uri).as_collection()
+    #     if not collection.contains_item_with_id(candidate_member_uri):
+    #         raise AssertionFailure(
+    #             spec_level or SpecLevel.UNSPECIFIED,
+    #             interop_level or InteropLevel.UNKNOWN,
+    #             f"Node { self }: {candidate_member_uri} not in {collection_uri}")
+
+
+    # # Python 3.12 @override
+    # def assert_not_member_of_collection_at(
+    #     self,
+    #     candidate_member_uri: str,
+    #     collection_uri: str,
+    #     spec_level: SpecLevel | None = None,
+    #     interop_level: InteropLevel | None= None
+    # ):
+    #     collection = AnyObject(collection_uri).as_collection()
+    #     if collection.contains_item_with_id(candidate_member_uri):
+    #         raise AssertionFailure(
+    #             spec_level or SpecLevel.UNSPECIFIED,
+    #             interop_level or InteropLevel.UNKNOWN,
+    #             f"Node { self }: {candidate_member_uri} must not be in {collection_uri}")
 
 # From WebFingerServer
 
@@ -564,7 +565,7 @@ def obtain_non_existing_account_identifier(self, rolename: str | None = None ) -
         return non_account.webfinger_uri
 
     # Not implemented:
-    # def obtain_account_identifier_requiring_percent_encoding(self, nickname: str | None = None) -> str:
+    # def obtain_account_identifier_requiring_percent_encoding(self, rolename: str | None = None) -> str:
     # def override_webfinger_response(self, client_operation: Callable[[],Any], overridden_json_response: Any):
 
 # From WebServer