Links service refactor #9460
Links service refactor #9460
Conversation
Remove function registry pattern and return all links explicitly
| 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.
| "@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.
| "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.
|
|
||
| LINK_GENERATORS_KEY = "h.links.link_generators" | ||
|
|
||
| def json_link(request, annotation) -> str: |
There was a problem hiding this comment.
Functions from h.links
| return LinksService(base_url=base_url, registry=request.registry) | ||
|
|
||
|
|
||
| def add_annotation_link_generator(config, name, generator, hidden=False): # noqa: FBT002 |
There was a problem hiding this comment.
Remove the registry
| 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.
| 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.
seanh
left a comment
seanh
left a comment
There was a problem hiding this comment.
Yeah this seems like an improvement -- more similar to our usual services pattern, and the "link registry" thing seems unnecessary
| 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