Links service refactor

h/activity/bucketing.py

@@ -7,7 +7,8 @@
 import newrelic.agent
 from pyramid import i18n
 
-from h import links, presenters
+from h import presenters
+from h.services.links import incontext_link
 
 _ = i18n.TranslationStringFactory(__package__)
 
@@ -48,7 +49,7 @@ def incontext_link(self, request):
         """
         if not self.annotations:
             return None
-        return links.incontext_link(request, self.annotations[0])
+        return incontext_link(request, self.annotations[0])
 
     def append(self, annotation):
         self.annotations.append(annotation)

h/emails/mention_notification.py

@@ -1,12 +1,12 @@
 from pyramid.renderers import render
 from pyramid.request import Request
 
-from h import links
 from h.emails.util import email_subject
 from h.models import Subscriptions
 from h.notification.mention import MentionNotification
 from h.services import SubscriptionService
 from h.services.email import EmailData, EmailTag
+from h.services.links import incontext_link
 
 
 def generate(request: Request, notification: MentionNotification) -> EmailData:
@@ -20,7 +20,7 @@ def generate(request: Request, notification: MentionNotification) -> EmailData:
         "username": username,
         "user_display_name": notification.mentioning_user.display_name
         or f"@{username}",
-        "annotation_url": links.incontext_link(request, notification.annotation)
+        "annotation_url": incontext_link(notification.annotation)
         or request.route_url("annotation", id=notification.annotation.id),
         "document_title": notification.document.title
         or notification.annotation.target_uri,

h/emails/reply_notification.py

@@ -1,12 +1,12 @@
 from pyramid.renderers import render
 from pyramid.request import Request
 
-from h import links
 from h.emails.util import email_subject, get_user_url
 from h.models import Subscriptions
 from h.notification.reply import Notification
 from h.services import SubscriptionService
 from h.services.email import EmailData, EmailTag
+from h.services.links import incontext_link
 
 
 def generate(request: Request, notification: Notification) -> EmailData:
@@ -15,7 +15,6 @@ def generate(request: Request, notification: Notification) -> EmailData:
     :param request: the current request
     :param notification: the reply notification data structure
     """
-
     unsubscribe_token = request.find_service(SubscriptionService).get_unsubscribe_token(
         user_id=notification.parent_user.userid, type_=Subscriptions.Type.REPLY
     )
@@ -34,7 +33,7 @@ def generate(request: Request, notification: Notification) -> EmailData:
         ),
         # Reply related
         "reply": notification.reply,
-        "reply_url": links.incontext_link(request, notification.reply)
+        "reply_url": incontext_link(notification.reply)
         or request.route_url("annotation", id=notification.reply.id),
         "reply_user_display_name": notification.reply_user.display_name
         or notification.reply_user.username,

h/presenters/annotation_jsonld.py

@@ -1,3 +1,4 @@
+from h.services.links import LinksService
 from h.util.datetime import utc_iso8601
 
 
@@ -10,7 +11,7 @@ class AnnotationJSONLDPresenter:
       https://www.w3.org/TR/annotation-model/
     """
 
-    def __init__(self, annotation, links_service):
+    def __init__(self, annotation, links_service: LinksService):
         self.annotation = annotation
         self._links_service = links_service
 
@@ -20,7 +21,7 @@ def asdict(self):
         return {
             "@context": self.CONTEXT_URL,
             "type": "Annotation",
-            "id": self._links_service.get(self.annotation, "jsonld_id"),
+            "id": self._links_service.jsonld_id(self.annotation),
             "created": utc_iso8601(self.annotation.created),
             "modified": utc_iso8601(self.annotation.updated),
             "creator": self.annotation.userid,

h/services/__init__.py

@@ -112,10 +112,6 @@ def includeme(config):  # pragma: no cover
     )
 
     # Other services
-    config.add_directive(
-        "add_annotation_link_generator",
-        "h.services.links.add_annotation_link_generator",
-    )
     config.register_service_factory("h.services.links.links_factory", name="links")
     config.register_service_factory(
         "h.services.list_organizations.list_organizations_factory",

h/services/annotation_json.py

@@ -80,7 +80,11 @@ def present(self, annotation: Annotation, with_metadata: bool = False):  # noqa:
                 },
                 "target": annotation.target,
                 "document": DocumentJSONPresenter(annotation.document).asdict(),
-                "links": self._links_service.get_all(annotation),
+                "links": {
+                    "html": self._links_service.html_link(annotation),
+                    "incontext": self._links_service.incontext_link(annotation),
+                    "json": self._links_service.json_link(annotation),
+                },
             }
         )
 

h/services/links.py

@@ -1,16 +1,56 @@
 """Tools for generating links to domain objects."""
 
+from urllib.parse import urljoin
+
 from pyramid.request import Request
 
 from h.security.request_methods import default_authority
 
-LINK_GENERATORS_KEY = "h.links.link_generators"
+
+def json_link(request, annotation) -> str:
+    return request.route_url("api.annotation", id=annotation.id)
+
+
+def jsonld_id_link(request, annotation) -> str:
+    return request.route_url("annotation", id=annotation.id)
+
+
+def html_link(request, annotation) -> str | None:
+    """Return a link to an HTML representation of the given annotation, or None."""
+    is_third_party_annotation = annotation.authority != request.default_authority
+    if is_third_party_annotation:
+        # We don't currently support HTML representations of third party
+        # annotations.
+        return None
+    return request.route_url("annotation", id=annotation.id)
+
+
+def incontext_link(request, annotation) -> str | None:
+    """Generate a link to an annotation on the page where it was made."""
+    bouncer_url = request.registry.settings.get("h.bouncer_url")
+    if not bouncer_url:
+        return None
+
+    link = urljoin(bouncer_url, annotation.thread_root_id)
+    uri = annotation.target_uri
+    if uri.startswith(("http://", "https://")):
+        # We can't use urljoin here, because if it detects the second argument
+        # is a URL it will discard the base URL, breaking the link entirely.
+        link += "/" + uri[uri.index("://") + 3 :]
+    elif uri.startswith("urn:x-pdf:") and annotation.document:  # pragma: no cover
+        for docuri in annotation.document.document_uris:
+            uri = docuri.uri
+            if uri.startswith(("http://", "https://")):
+                link += "/" + uri[uri.index("://") + 3 :]
+                break
+
+    return link
 
 
 class LinksService:
     """A service for generating links to annotations."""
 
-    def __init__(self, base_url, registry):
+    def __init__(self, base_url):
         """
         Create a new links service.
 
@@ -19,7 +59,6 @@ def __init__(self, base_url, registry):
         :type registry: pyramid.registry.Registry
         """
         self.base_url = base_url
-        self.registry = registry
 
         # It would be absolutely fair if at this point you asked yourself any
         # of the following questions:
@@ -41,53 +80,28 @@ def __init__(self, base_url, registry):
         # error-prone way to get access to the route_url function, which can
         # be used by link generators.
         self._request = Request.blank("/", base_url=base_url)
-        self._request.registry = registry
 
         # Allow retrieval of the authority from the fake request object, the
         # same as we do for real requests.
         self._request.set_property(
             default_authority, name="default_authority", reify=True
         )
 
-    def get(self, annotation, name):
-        """Get the link named `name` for the passed `annotation`."""
-        link_generator, _ = self.registry[LINK_GENERATORS_KEY][name]
-        return link_generator(self._request, annotation)
-
-    def get_all(self, annotation):
-        """Get all (non-hidden) links for the passed `annotation`."""
-        links = {}
-        for name, (link_generator, hidden) in self.registry[
-            LINK_GENERATORS_KEY
-        ].items():
-            if hidden:
-                continue
-            link = link_generator(self._request, annotation)
-            if link is not None:
-                links[name] = link
-        return links
+    def json_link(self, annotation):
+        return json_link(self._request, annotation)
+
+    def jsonld_id_link(self, annotation) -> str:
+        return jsonld_id_link(self._request, annotation)
+
+    def html_link(self, annotation):
+        return html_link(self._request, annotation)
+
+    def incontext_link(self, annotation):
+        return incontext_link(self._request, annotation)
 
 
 def links_factory(_context, request):
     """Return a LinksService instance for the passed context and request."""
-    base_url = request.registry.settings.get("h.app_url", "http://localhost:5000")
-    return LinksService(base_url=base_url, registry=request.registry)
-
-
-def add_annotation_link_generator(config, name, generator, hidden=False):  # noqa: FBT002
-    """
-    Register a function which generates a named link for an annotation.
-
-    Annotation hypermedia links are added to the rendered annotations in a
-    `links` property or similar. `name` is the unique identifier for the link
-    type, and `generator` is a callable which accepts two arguments -- the
-    current request, and the annotation for which to generate a link -- and
-    returns a string.
-
-    If `hidden` is True, then the link generator will not be included in the
-    default links output when rendering annotations.
-    """
-    registry = config.registry
-    if LINK_GENERATORS_KEY not in registry:
-        registry[LINK_GENERATORS_KEY] = {}
-    registry[LINK_GENERATORS_KEY][name] = (generator, hidden)
+    return LinksService(
+        base_url=request.registry.settings.get("h.app_url", "http://localhost:5000")
+    )

h/views/activity.py

@@ -1,6 +1,6 @@
 """Activity pages views."""
 
-from urllib.parse import urlparse
+from urllib.parse import unquote, urlparse
 
 from markupsafe import Markup
 from pyramid import httpexceptions
@@ -9,7 +9,6 @@
 from h import util
 from h.activity import query
 from h.i18n import TranslationString as _
-from h.links import pretty_link
 from h.paginator import paginate
 from h.presenters.organization_json import OrganizationJSONPresenter
 from h.search import parser
@@ -21,6 +20,21 @@
 PAGE_SIZE = 200
 
 
+def pretty_link(url):
+    """
+    Return a nicely formatted version of a URL.
+
+    This strips off 'visual noise' from the URL including common schemes
+    (HTTP, HTTPS), domain prefixes ('www.') and query strings.
+    """
+    parsed = urlparse(url)
+    if parsed.scheme not in ["http", "https"]:
+        return url
+    netloc = parsed.netloc
+    netloc = netloc.removeprefix("www.")
+    return unquote(netloc + parsed.path)
+
+
 @view_defaults(
     route_name="activity.search", renderer="h:templates/activity/search.html.jinja2"
 )