From 969df0e5bf094a4ddfca979bcbd0db395dae5686 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Mon, 16 Oct 2023 13:48:02 +0530 Subject: [PATCH] JS: Refactor link & badge generation, use URLs (not string) for base URLs This is two changes that were easier to make together - BASE_URL and BADGE_BASE_URL are now URL objects rather than strings that are manipulated. With this done, we no longer use string manipulation for URLs anywhere! - Both BASE_URL and BADGE_BASE_URL are now always set, as we had a bunch of code that was using BADGE_BASE_URL if available but falls back to BASE_URL + origin if it was not set. This fallback is now implemented globally, and correctly. - BASE_URL is also now always fully qualified, and we document that the python code ensures it has a trailing slash always. - The function to make links and generate badge markup is moved into `@jupyterhub/binderhub-client` as it is reasonably generic and not super specific to our frontend alone. This also involves them not reading BASE_URL and BADGE_BASE_URL globally, but having that information be passed in. Tests are also added here to catch any future issues that may arise. - Note for future fix - BADGE_BASE_URL is really PUBLIC_BASE_URL or similar, as it is used both for the location of the badge image (original intent) but also for the links we generate to share. This is relevant only for federation, where we want shared links to point to mybinder.org even though the API call itself may go to a specific member of the federation. I will do this deprecation + rename in a future PR so as to not make this PR bigger. Ref https://github.com/jupyterhub/binderhub/issues/774 --- binderhub/static/js/index.js | 30 +++--- binderhub/static/js/src/badge.js | 24 ----- binderhub/static/js/src/constants.js | 23 +++-- binderhub/static/js/src/favicon.js | 2 +- binderhub/static/js/src/repo.js | 2 +- binderhub/static/js/src/urls.js | 70 +++---------- js/packages/binderhub-client/lib/index.js | 62 ++++++++++++ .../binderhub-client/tests/index.test.js | 97 ++++++++++++++++++- 8 files changed, 204 insertions(+), 106 deletions(-) delete mode 100644 binderhub/static/js/src/badge.js diff --git a/binderhub/static/js/index.js b/binderhub/static/js/index.js index 6e192dadb..7630b0d33 100644 --- a/binderhub/static/js/index.js +++ b/binderhub/static/js/index.js @@ -18,12 +18,12 @@ import "bootstrap/dist/css/bootstrap-theme.min.css"; import "../index.css"; import { setUpLog } from "./src/log"; import { updateUrls } from "./src/urls"; -import { BASE_URL } from "./src/constants"; +import { BASE_URL, BADGE_BASE_URL } from "./src/constants"; import { getBuildFormValues } from "./src/form"; import { updateRepoText } from "./src/repo"; async function build(providerSpec, log, fitAddon, path, pathType) { - updateFavicon(BASE_URL + "favicon_building.ico"); + updateFavicon(new URL("favicon_building.ico", BASE_URL)); // split provider prefix off of providerSpec const spec = providerSpec.slice(providerSpec.indexOf("/") + 1); // Update the text of the loading page if it exists @@ -39,13 +39,7 @@ async function build(providerSpec, log, fitAddon, path, pathType) { $(".on-build").removeClass("hidden"); const buildToken = $("#build-token").data("token"); - // If BASE_URL is absolute, use that as the base for build endpoint URL. - // Else, first resolve BASE_URL relative to current URL, then use *that* as the - // base for the build endpoint url. - const buildEndpointUrl = new URL( - "build", - new URL(BASE_URL, window.location.href), - ); + const buildEndpointUrl = new URL("build", BASE_URL); const image = new BinderRepository( providerSpec, buildEndpointUrl, @@ -82,7 +76,7 @@ async function build(providerSpec, log, fitAddon, path, pathType) { $("#loader").addClass("paused"); // If we fail for any reason, show an error message and logs - updateFavicon(BASE_URL + "favicon_fail.ico"); + updateFavicon(new URL("favicon_fail.ico", BASE_URL)); log.show(); if ($("div#loader-text").length > 0) { $("#loader").addClass("error"); @@ -96,7 +90,7 @@ async function build(providerSpec, log, fitAddon, path, pathType) { case "built": { $("#phase-already-built").removeClass("hidden"); $("#phase-launching").removeClass("hidden"); - updateFavicon(BASE_URL + "favicon_success.ico"); + updateFavicon(new URL("favicon_success.ico", BASE_URL)); break; } case "ready": { @@ -127,7 +121,7 @@ function indexMain() { const [log, fitAddon] = setUpLog(); // setup badge dropdown and default values. - updateUrls(); + updateUrls(BADGE_BASE_URL); $("#provider_prefix_sel li").click(function (event) { event.preventDefault(); @@ -135,7 +129,7 @@ function indexMain() { $("#provider_prefix-selected").text($(this).text()); $("#provider_prefix").val($(this).attr("value")); updateRepoText(); - updateUrls(); + updateUrls(BADGE_BASE_URL); }); $("#url-or-file-btn") @@ -145,21 +139,21 @@ function indexMain() { $("#url-or-file-selected").text($(this).text()); updatePathText(); - updateUrls(); + updateUrls(BADGE_BASE_URL); }); updatePathText(); updateRepoText(); $("#repository").on("keyup paste change", function () { - updateUrls(); + updateUrls(BADGE_BASE_URL); }); $("#ref").on("keyup paste change", function () { - updateUrls(); + updateUrls(BADGE_BASE_URL); }); $("#filepath").on("keyup paste change", function () { - updateUrls(); + updateUrls(BADGE_BASE_URL); }); $("#toggle-badge-snippet").on("click", function () { @@ -180,7 +174,7 @@ function indexMain() { $("#build-form").submit(async function (e) { e.preventDefault(); const formValues = getBuildFormValues(); - updateUrls(formValues); + updateUrls(BADGE_BASE_URL, formValues); await build( formValues.providerPrefix + "/" + formValues.repo + "/" + formValues.ref, log, diff --git a/binderhub/static/js/src/badge.js b/binderhub/static/js/src/badge.js deleted file mode 100644 index d191e8d82..000000000 --- a/binderhub/static/js/src/badge.js +++ /dev/null @@ -1,24 +0,0 @@ -/** - * Generate markdown that people can put on their README or documentation to link to this binder - * - * @param {string} badgeBaseUrl Optional base URL to use for badge images. If not passed, current origin + baseUrl is used - * @param {string} baseUrl Base URL of this binderhub installation. Used only if badgeBaseUrl is not passed - * @param {string} url Link target URL that represents this binder installation - * @param {string} syntax Kind of markup to generate. Supports 'markdown' and 'rst' - * @returns {string} - */ -export function makeBadgeMarkup(badgeBaseUrl, baseUrl, url, syntax) { - let badgeImageUrl; - - if (badgeBaseUrl) { - badgeImageUrl = badgeBaseUrl + "badge_logo.svg"; - } else { - badgeImageUrl = window.location.origin + baseUrl + "badge_logo.svg"; - } - - if (syntax === "markdown") { - return "[![Binder](" + badgeImageUrl + ")](" + url + ")"; - } else if (syntax === "rst") { - return ".. image:: " + badgeImageUrl + "\n :target: " + url; - } -} diff --git a/binderhub/static/js/src/constants.js b/binderhub/static/js/src/constants.js index 96b5f880c..4ffacd439 100644 --- a/binderhub/static/js/src/constants.js +++ b/binderhub/static/js/src/constants.js @@ -1,13 +1,22 @@ /** - * @type {string} - * Base URL of this binderhub installation + * @type {URL} + * Base URL of this binderhub installation. + * + * Guaranteed to havea trailing slash by the binderhub python configuration. */ -export const BASE_URL = $("#base-url").data().url; +export const BASE_URL = new URL( + document.getElementById("base-url").dataset.url, + document.location.origin, +); +const badge_base_url = document.getElementById("badge-base-url").dataset.url; /** - * @type {string} - * Optional base URL to use for both badge images as well as launch links. + * @type {URL} + * Base URL to use for both badge images as well as launch links. * - * Is different from BASE_URL primarily when used as part of a federation. + * If not explicitly set, will default to BASE_URL. Primarily set up different than BASE_URL + * when used as part of a federation */ -export const BADGE_BASE_URL = $("#badge-base-url").data().url; +export const BADGE_BASE_URL = badge_base_url + ? new URL(badge_base_url, document.location.origin) + : BASE_URL; diff --git a/binderhub/static/js/src/favicon.js b/binderhub/static/js/src/favicon.js index b4a7dead1..ae6592cfa 100644 --- a/binderhub/static/js/src/favicon.js +++ b/binderhub/static/js/src/favicon.js @@ -1,7 +1,7 @@ /** * Dynamically set current page's favicon. * - * @param {String} href Path to Favicon to use + * @param {URL} href Path to Favicon to use */ function updateFavicon(href) { let link = document.querySelector("link[rel*='icon']"); diff --git a/binderhub/static/js/src/repo.js b/binderhub/static/js/src/repo.js index 06c811ef7..56dfc8843 100644 --- a/binderhub/static/js/src/repo.js +++ b/binderhub/static/js/src/repo.js @@ -26,7 +26,7 @@ function setLabels() { */ export function updateRepoText() { if (Object.keys(configDict).length === 0) { - const configUrl = BASE_URL + "_config"; + const configUrl = new URL("_config", BASE_URL); fetch(configUrl).then((resp) => { resp.json().then((data) => { configDict = data; diff --git a/binderhub/static/js/src/urls.js b/binderhub/static/js/src/urls.js index 395917eee..698ccfdb6 100644 --- a/binderhub/static/js/src/urls.js +++ b/binderhub/static/js/src/urls.js @@ -1,71 +1,33 @@ -import { makeBadgeMarkup } from "./badge"; import { getBuildFormValues } from "./form"; -import { BADGE_BASE_URL, BASE_URL } from "./constants"; - -/** - * Generate a shareable binder URL for given repository - - * @param {string} providerPrefix prefix denoting what provider was selected - * @param {string} repo repo to build - * @param {[string]} ref optional ref in this repo to build - * @param {string} path Path to launch after this repo has been built - * @param {string} pathType Type of thing to open path with (raw url, notebook file, lab, etc) - * - * @returns {string|null} A URL that can be shared with others, and clicking which will launch the repo - */ -function v2url(providerPrefix, repository, ref, path, pathType) { - // return a v2 url from a providerPrefix, repository, ref, and (file|url)path - if (repository.length === 0) { - // no repo, no url - return null; - } - let url; - if (BADGE_BASE_URL) { - url = - BADGE_BASE_URL + "v2/" + providerPrefix + "/" + repository + "/" + ref; - } else { - url = - window.location.origin + - BASE_URL + - "v2/" + - providerPrefix + - "/" + - repository + - "/" + - ref; - } - if (path && path.length > 0) { - // encode the path, it will be decoded in loadingMain - url = url + "?" + pathType + "path=" + encodeURIComponent(path); - } - return url; -} +import { + makeShareableBinderURL, + makeBadgeMarkup, +} from "@jupyterhub/binderhub-client"; /** * Update the shareable URL and badge snippets in the UI based on values user has entered in the form */ -export function updateUrls(formValues) { +export function updateUrls(publicBaseUrl, formValues) { if (typeof formValues === "undefined") { formValues = getBuildFormValues(); } - const url = v2url( - formValues.providerPrefix, - formValues.repo, - formValues.ref, - formValues.path, - formValues.pathType, - ); + if (formValues.repo) { + const url = makeShareableBinderURL( + publicBaseUrl, + formValues.providerPrefix, + formValues.repo, + formValues.ref, + formValues.path, + formValues.pathType, + ); - if ((url || "").trim().length > 0) { // update URLs and links (badges, etc.) $("#badge-link").attr("href", url); $("#basic-url-snippet").text(url); $("#markdown-badge-snippet").text( - makeBadgeMarkup(BADGE_BASE_URL, BASE_URL, url, "markdown"), - ); - $("#rst-badge-snippet").text( - makeBadgeMarkup(BADGE_BASE_URL, BASE_URL, url, "rst"), + makeBadgeMarkup(publicBaseUrl, url, "markdown"), ); + $("#rst-badge-snippet").text(makeBadgeMarkup(publicBaseUrl, url, "rst")); } else { ["#basic-url-snippet", "#markdown-badge-snippet", "#rst-badge-snippet"].map( function (item) { diff --git a/js/packages/binderhub-client/lib/index.js b/js/packages/binderhub-client/lib/index.js index 90ca683a4..a5653e597 100644 --- a/js/packages/binderhub-client/lib/index.js +++ b/js/packages/binderhub-client/lib/index.js @@ -149,3 +149,65 @@ export class BinderRepository { return url; } } + +/** + * Generate a shareable binder URL for given repository + * + * @param {URL} publicBaseUrl Base URL to use for making public URLs. Must end with a trailing slash. + * @param {string} providerPrefix prefix denoting what provider was selected + * @param {string} repo repo to build + * @param {string} ref optional ref in this repo to build + * @param {[string]} path Path to launch after this repo has been built + * @param {[string]} pathType Type of thing to open path with (raw url, notebook file, lab, etc) + * + * @returns {URL} A URL that can be shared with others, and clicking which will launch the repo + */ +export function makeShareableBinderURL( + publicBaseUrl, + providerPrefix, + repository, + ref, + path, + pathType, +) { + if (!publicBaseUrl.pathname.endsWith("/")) { + throw new Error( + `publicBaseUrl must end with a trailing slash, got ${publicBaseUrl}`, + ); + } + const url = new URL( + `v2/${providerPrefix}/${repository}/${ref}`, + publicBaseUrl, + ); + if (path && path.length > 0) { + url.searchParams.append(`${pathType}path`, path); + } + return url; +} + +/** + * Generate markup that people can put on their README or documentation to link to a specific binder + * + * @param {URL} publicBaseUrl Base URL to use for making public URLs + * @param {URL} url Link target URL that represents this binder installation + * @param {string} syntax Kind of markup to generate. Supports 'markdown' and 'rst' + * @returns {string} + */ +export function makeBadgeMarkup(publicBaseUrl, url, syntax) { + if (!publicBaseUrl.pathname.endsWith("/")) { + throw new Error( + `publicBaseUrl must end with a trailing slash, got ${publicBaseUrl}`, + ); + } + const badgeImageUrl = new URL("badge_logo.svg", publicBaseUrl); + + if (syntax === "markdown") { + return `[![Binder](${badgeImageUrl})](${url})`; + } else if (syntax === "rst") { + return `.. image:: ${badgeImageUrl}\n :target: ${url}`; + } else { + throw new Error( + `Only markdown or rst badges are supported, got ${syntax} instead`, + ); + } +} diff --git a/js/packages/binderhub-client/tests/index.test.js b/js/packages/binderhub-client/tests/index.test.js index 37e11ac76..553a1e9aa 100644 --- a/js/packages/binderhub-client/tests/index.test.js +++ b/js/packages/binderhub-client/tests/index.test.js @@ -1,4 +1,8 @@ -import { BinderRepository } from "@jupyterhub/binderhub-client"; +import { + BinderRepository, + makeShareableBinderURL, + makeBadgeMarkup, +} from "@jupyterhub/binderhub-client"; import { parseEventSource, simpleEventSourceServer } from "./utils"; import fs from "node:fs"; @@ -295,3 +299,94 @@ test("Get full redirect URL with nbgitpuller URL", () => { "https://hub.test-binder.org/user/something/git-pull?repo=https%3A%2F%2Fgithub.com%2Falperyilmaz%2Fjupyterlab-python-intro&urlpath=lab%2Ftree%2Fjupyterlab-python-intro%2F&branch=master&token=token", ); }); + +test("Make a shareable URL", () => { + const url = makeShareableBinderURL( + new URL("https://test.binder.org"), + "gh", + "yuvipanda", + "requirements", + ); + expect(url.toString()).toBe( + "https://test.binder.org/v2/gh/yuvipanda/requirements", + ); +}); + +test("Make a shareable path with URL", () => { + const url = makeShareableBinderURL( + new URL("https://test.binder.org"), + "gh", + "yuvipanda", + "requirements", + "url", + "git-pull?repo=https://github.com/alperyilmaz/jupyterlab-python-intro&urlpath=lab/tree/jupyterlab-python-intro/&branch=master", + ); + expect(url.toString()).toBe( + "https://test.binder.org/v2/gh/yuvipanda/requirements?git-pull%3Frepo%3Dhttps%3A%2F%2Fgithub.com%2Falperyilmaz%2Fjupyterlab-python-intro%26urlpath%3Dlab%2Ftree%2Fjupyterlab-python-intro%2F%26branch%3Dmasterpath=url", + ); +}); + +test("Making a shareable URL with base URL without trailing / throws error", () => { + expect(() => { + makeShareableBinderURL( + new URL("https://test.binder.org/suffix"), + "gh", + "yuvipanda", + "requirements", + ); + }).toThrow(Error); +}); + +test("Make a markdown badge", () => { + const url = makeShareableBinderURL( + new URL("https://test.binder.org"), + "gh", + "yuvipanda", + "requirements", + ); + const badge = makeBadgeMarkup( + new URL("https://test.binder.org"), + url, + "markdown", + ); + expect(badge).toBe( + "[![Binder](https://test.binder.org/badge_logo.svg)](https://test.binder.org/v2/gh/yuvipanda/requirements)", + ); +}); + +test("Make a rst badge", () => { + const url = makeShareableBinderURL( + new URL("https://test.binder.org"), + "gh", + "yuvipanda", + "requirements", + ); + const badge = makeBadgeMarkup(new URL("https://test.binder.org"), url, "rst"); + expect(badge).toBe( + ".. image:: https://test.binder.org/badge_logo.svg\n :target: https://test.binder.org/v2/gh/yuvipanda/requirements", + ); +}); + +test("Making a badge with an unsupported syntax throws error", () => { + const url = makeShareableBinderURL( + new URL("https://test.binder.org"), + "gh", + "yuvipanda", + "requirements", + ); + expect(() => { + makeBadgeMarkup(new URL("https://test.binder.org"), url, "docx"); + }).toThrow(Error); +}); + +test("Making a badge with base URL without trailing / throws error", () => { + const url = makeShareableBinderURL( + new URL("https://test.binder.org"), + "gh", + "yuvipanda", + "requirements", + ); + expect(() => { + makeBadgeMarkup(new URL("https://test.binder.org/suffix"), url, "markdown"); + }).toThrow(Error); +});