Add an initial revision of a semantic vanity url servlet #529
Conversation
| // /api/module/vanity.json/{locale}/{productLabel}/{versionLabel}/{vanityUrl} | ||
| public class VanityVariantJsonServlet extends SlingSafeMethodsServlet { | ||
|
|
||
| private final SlingPathSuffix suffix = new SlingPathSuffix("/{locale}/{productLabel}/{versionLabel}/{vanityUrl}"); |
There was a problem hiding this comment.
the customer portal view_uri is constructed with product urlFragment and version urlFragment. not labels. Should we follow the same convention and use productUrlFragment and versionUrlFragment?
There was a problem hiding this comment.
Indeed. If you look at the code it is in fact filtering by url fragment. The strings are just pattern placeholders to be able to extract the right values from the url. I will change them to reflect that they are url fragments and not label.
There was a problem hiding this comment.
Carlos, I like this initial stab at vanity URLs - but there are some big questions that I touched on in my comments and I'd like us to have a conversation about our high-level approach/architecture before we start committing code for any functionality here. Regardless, thanks for this submission, it will definitely help add some context to our conversations.
|
|
||
| // Find the metadata with the right vanity url, | ||
| Optional<ModuleVersion> moduleVersion = queryHelper.query( | ||
| "select * from [pant:moduleVersion] as v where v.[metadata/urlFragment] = '/" + vanityUrl + "'") |
There was a problem hiding this comment.
A couple thoughts...
- Why are you using Module classes instead of Document classes (then it would hopefully work for Modules and Assemblies)?
- I see you're querying for the vanity url at the location we keep it today, which is on the metadata node. That does work, but I think I'd like to see a system where we manage vanity URLs (and redirects, etc) as separate entities in the system (perhaps located at /content/redirects in the JCR, something along those lines?) that contain pointers to the UUIDs of the DocumentVariants (would need to nail down what makes the most sense there) that they redirect to. In that world, this query would obviously need to be refactored. Regardless, the overall concept here is still solid.
| Constants.SERVICE_DESCRIPTION + "=Servlet to facilitate GET operation which accepts several path parameters to fetch module variant data", | ||
| Constants.SERVICE_VENDOR + "=Red Hat Content Tooling team" | ||
| }) | ||
| @SlingServletPaths(value = "/api/module/vanity") |
There was a problem hiding this comment.
Interesting that you register this servlet by path. I'm wondering if that doesn't overburden Drupal... Drupal would have to be smart enough to know whether the URL it was handling was a vanity URL or not and then call the correct Pantheon endpoint accordingly. That might be tough. I wonder if it doesn't make more sense for us to try and implement something more like a DefaultGetServlet for Sling that could handle the whole spectrum of requests and internally forward them to servlets appropriately.
| } | ||
| }) | ||
| // There should be 1 at most, but get the first if there are more | ||
| .findFirst(); |
There was a problem hiding this comment.
In today's world, there could definitely be more than one. There's nothing stopping me from adding a vanity URL of "/cats" to every document in the system. My idea above of managing redirects as separate entities neatly addresses this problem.
|
|
||
| if(!moduleVersion.isPresent()) { | ||
| // try to find by variant uuid, instead of vanity url | ||
| moduleVersion = findModuleByUuid(queryHelper, productLabel, versionLabel, locale, vanityUrl); |
There was a problem hiding this comment.
I'm wondering why this "vanity" servlet is also accepting the UUID - perhaps you envision this new servlet as "the" one-and-only servlet that Drupal would call, and it handles anything you throw at it? Can you clarify if that's what you intended?
This is a very early release for an API that allows the fetching of module data by using a more semantic approach to urls.
Once a module has been fully configured and released, this servlet allows to get its information via the following url patterns:
http://localhost:8181/api/module/vanity.json/<locale>/<product_url>/<version_url>/<module_variant_uuid>or the similar
http://localhost:8181/api/module/vanity.json/<locale>/<product_url>/<version_url>/<module_vanity_url>this may result in API urls like this:
http://localhost:8181/api/module/vanity.json/en-us/rhel/8.0/configuring_wired_connection_using_control_centerThe url pattern can be changed but this would allow Pantheon to offer the content based on the configured metadata values, and as a result would offer repeatable urls for content.