Links service refactor #9460
Links service refactor #9460
Conversation
Remove function registry pattern and return all links explicitly
| @@ -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]) | |||
There was a problem hiding this comment.
We often use these functions directly, keep it as functions but moved them to the service namespace.
There was a problem hiding this comment.
We could maybe just change code paths like this to use the service methods: request.find_service(name="links").incontext_link(annotation).
| from urllib.parse import unquote, urljoin, urlparse | ||
|
|
||
|
|
||
| def pretty_link(url): |
There was a problem hiding this comment.
Moved these to the 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), | |||
There was a problem hiding this comment.
Call the right service method directly here.
| @@ -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": { | |||
There was a problem hiding this comment.
List every type of link here.
| 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 LinksService(base_url=base_url, registry=request.registry) | ||
|
|
||
|
|
||
| def add_annotation_link_generator(config, name, generator, hidden=False): # noqa: FBT002 |
| @@ -21,6 +20,21 @@ | |||
| PAGE_SIZE = 200 | |||
|
|
|||
|
|
|||
| def pretty_link(url): | |||
There was a problem hiding this comment.
This was in h.links but only used here, 🤷 I could a function in the service like the others but it's a bit different in the intent.
| @@ -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 | |||
There was a problem hiding this comment.
This request object and the comment below is the only reason to keep this a service, not just functions.
I don't want to go on a huge refactoring path, IMO keeping the service and the functions in the same module is a bit clearer.
| @@ -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]) | |||
There was a problem hiding this comment.
We could maybe just change code paths like this to use the service methods: request.find_service(name="links").incontext_link(annotation).
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
github-actions
bot
added
the
stale
Remove function registry pattern and return all links explicitly