diff --git a/README.md b/README.md index 07651bd9..3cf6ebdf 100644 --- a/README.md +++ b/README.md @@ -44,12 +44,13 @@ You must also configure the plugin settings before syncing. These settings are mandatory: - Your GitHub Token (see below) -- Repository owner -- Repository name +- Repository URL - Repository branch If any of this is not set sync won't start. +The repository URL is the full URL of your repository, like `https://github.com/owner/repository`. GitHub enterprise users should enter their self-hosted url e.g. `https://github.example.com/owner/repository`. + ### Token A GitHub Fine-grained token is required to sync with your repository. You can create one by clicking [here](https://github.com/settings/personal-access-tokens/new). diff --git a/benchmark.ts b/benchmark.ts index 27fbc6ea..9eeb85d3 100644 --- a/benchmark.ts +++ b/benchmark.ts @@ -47,8 +47,8 @@ async function runBenchmark(vaultRootDir: string) { // Settings for the sync manager const settings = { githubToken: process.env.GITHUB_TOKEN, - githubOwner: process.env.REPO_OWNER, - githubRepo: process.env.REPO_NAME, + githubRepoUrl: `https://github.com/${process.env.REPO_OWNER}/${process.env.REPO_NAME}`, + githubApiBaseUrl: "", githubBranch: process.env.REPO_BRANCH, syncConfigDir: false, }; diff --git a/package.json b/package.json index cb87657d..9c5004c2 100644 --- a/package.json +++ b/package.json @@ -7,6 +7,7 @@ "dev": "node esbuild.config.mjs", "build": "tsc -noEmit -skipLibCheck && node esbuild.config.mjs production", "version": "node version-bump.mjs", + "test": "tsx --test \"src/**/*.test.ts\"", "benchmark": "tsx benchmark.ts" }, "keywords": [], diff --git a/src/github/client.ts b/src/github/client.ts index b09fa4bb..7a367388 100644 --- a/src/github/client.ts +++ b/src/github/client.ts @@ -2,6 +2,7 @@ import { requestUrl } from "obsidian"; import Logger from "src/logger"; import { GitHubSyncSettings } from "src/settings/settings"; import { retryUntil } from "src/utils"; +import { resolveRepoTarget } from "./repo-url"; export type RepoContent = { files: { [key: string]: GetTreeResponseItem }; @@ -59,6 +60,11 @@ class GithubAPIError extends Error { } } +/** + * Raised when the repository to sync can't be determined from the settings + */ +class RepoConfigurationError extends Error {} + export default class GithubClient { constructor( private settings: GitHubSyncSettings, @@ -73,6 +79,27 @@ export default class GithubClient { }; } + /** + * Builds the URL of a REST API endpoint of the configured repository. + * + * @param path Endpoint path relative to the repository, must start with a slash + * @returns The full URL to request + */ + private async repoApiUrl(path: string): Promise { + const resolved = resolveRepoTarget( + this.settings.githubRepoUrl, + this.settings.githubApiBaseUrl, + ); + if (!resolved.valid) { + await this.logger.error("Failed to resolve repository URL", { + error: resolved.error, + }); + throw new RepoConfigurationError(resolved.error); + } + const { apiBaseUrl, owner, repo } = resolved.target; + return `${apiBaseUrl}/repos/${owner}/${repo}${path}`; + } + /** * Gets the content of the repo. * @@ -84,10 +111,13 @@ export default class GithubClient { retry = false, maxRetries = 5, } = {}): Promise { + const url = await this.repoApiUrl( + `/git/trees/${this.settings.githubBranch}?recursive=1`, + ); const response = await retryUntil( async () => { return requestUrl({ - url: `https://api.github.com/repos/${this.settings.githubOwner}/${this.settings.githubRepo}/git/trees/${this.settings.githubBranch}?recursive=1`, + url, headers: this.headers(), throw: false, }); @@ -133,10 +163,11 @@ export default class GithubClient { retry?: boolean; maxRetries?: number; }) { + const url = await this.repoApiUrl("/git/trees"); const response = await retryUntil( async () => { return requestUrl({ - url: `https://api.github.com/repos/${this.settings.githubOwner}/${this.settings.githubRepo}/git/trees`, + url, headers: this.headers(), method: "POST", body: JSON.stringify(tree), @@ -180,10 +211,11 @@ export default class GithubClient { retry?: boolean; maxRetries?: number; }): Promise { + const url = await this.repoApiUrl("/git/commits"); const response = await retryUntil( async () => { return requestUrl({ - url: `https://api.github.com/repos/${this.settings.githubOwner}/${this.settings.githubRepo}/git/commits`, + url, headers: this.headers(), method: "POST", body: JSON.stringify({ @@ -216,10 +248,13 @@ export default class GithubClient { * @returns The SHA of the branch head */ async getBranchHeadSha({ retry = false, maxRetries = 5 } = {}) { + const url = await this.repoApiUrl( + `/git/refs/heads/${this.settings.githubBranch}`, + ); const response = await retryUntil( async () => { return requestUrl({ - url: `https://api.github.com/repos/${this.settings.githubOwner}/${this.settings.githubRepo}/git/refs/heads/${this.settings.githubBranch}`, + url, headers: this.headers(), throw: false, }); @@ -254,10 +289,13 @@ export default class GithubClient { retry?: boolean; maxRetries?: number; }) { + const url = await this.repoApiUrl( + `/git/refs/heads/${this.settings.githubBranch}`, + ); const response = await retryUntil( async () => { return requestUrl({ - url: `https://api.github.com/repos/${this.settings.githubOwner}/${this.settings.githubRepo}/git/refs/heads/${this.settings.githubBranch}`, + url, headers: this.headers(), method: "PATCH", body: JSON.stringify({ @@ -299,10 +337,11 @@ export default class GithubClient { retry?: boolean; maxRetries?: number; }): Promise { + const url = await this.repoApiUrl("/git/blobs"); const response = await retryUntil( async () => { return requestUrl({ - url: `https://api.github.com/repos/${this.settings.githubOwner}/${this.settings.githubRepo}/git/blobs`, + url, headers: this.headers(), method: "POST", body: JSON.stringify({ content, encoding }), @@ -342,10 +381,11 @@ export default class GithubClient { retry?: boolean; maxRetries?: number; }): Promise { + const url = await this.repoApiUrl(`/git/blobs/${sha}`); const response = await retryUntil( async () => { return requestUrl({ - url: `https://api.github.com/repos/${this.settings.githubOwner}/${this.settings.githubRepo}/git/blobs/${sha}`, + url, headers: this.headers(), throw: false, }); @@ -386,10 +426,11 @@ export default class GithubClient { retry?: boolean; maxRetries?: number; }) { + const url = await this.repoApiUrl(`/contents/${path}`); const response = await retryUntil( async () => { return requestUrl({ - url: `https://api.github.com/repos/${this.settings.githubOwner}/${this.settings.githubRepo}/contents/${path}`, + url, headers: this.headers(), method: "PUT", body: JSON.stringify({ @@ -424,10 +465,11 @@ export default class GithubClient { retry = false, maxRetries = 5, } = {}): Promise { + const url = await this.repoApiUrl(`/zipball/${this.settings.githubBranch}`); const response = await retryUntil( async () => { return requestUrl({ - url: `https://api.github.com/repos/${this.settings.githubOwner}/${this.settings.githubRepo}/zipball/${this.settings.githubBranch}`, + url, headers: this.headers(), method: "GET", throw: false, diff --git a/src/github/repo-url.test.ts b/src/github/repo-url.test.ts new file mode 100644 index 00000000..60b4ba03 --- /dev/null +++ b/src/github/repo-url.test.ts @@ -0,0 +1,94 @@ +import test from "node:test"; +import * as assert from "node:assert/strict"; +import { resolveRepoTarget } from "./repo-url"; + +test("resolves repository URLs", () => { + const cases: [string, string][] = [ + ["https://github.com/owner/repo", "https://api.github.com"], + ["https://www.github.com/owner/repo", "https://api.github.com"], + // Enterprise Server serves the API on the same host + [ + "https://github.example.com/owner/repo", + "https://github.example.com/api/v3", + ], + [ + "http://github.internal:8443/owner/repo", + "http://github.internal:8443/api/v3", + ], + // Enterprise Cloud tenants have a dedicated API subdomain + ["https://acme.ghe.com/owner/repo", "https://api.acme.ghe.com"], + // Forms users are likely to paste: clone URL, trailing slash, no scheme, + // surrounding whitespace, URL of a page inside the repository, query and fragment + ["https://github.com/owner/repo.git", "https://api.github.com"], + ["https://github.com/owner/repo/", "https://api.github.com"], + ["github.com/owner/repo", "https://api.github.com"], + [" https://github.com/owner/repo ", "https://api.github.com"], + ["https://github.com/owner/repo/tree/main/notes", "https://api.github.com"], + ["https://github.com/owner/repo?tab=readme#top", "https://api.github.com"], + ]; + for (const [repoUrl, apiBaseUrl] of cases) { + assert.deepEqual( + resolveRepoTarget(repoUrl, ""), + { valid: true, target: { owner: "owner", repo: "repo", apiBaseUrl } }, + repoUrl, + ); + } +}); + +test("keeps the case of owner and repository", () => { + assert.deepEqual( + resolveRepoTarget("https://GitHub.com/Owner/My.Repo-1_2", ""), + { + valid: true, + target: { + owner: "Owner", + repo: "My.Repo-1_2", + apiBaseUrl: "https://api.github.com", + }, + }, + ); +}); + +test("refuses URLs that don't point to a repository", () => { + const cases = [ + "", + " ", + "not a url", + "ftp://github.com/owner/repo", + // SSH remotes can't be used with the REST API + "git@github.com:owner/repo.git", + "https://github.com", + "https://github.com/owner", + "https://github.com//repo", + // Characters GitHub doesn't allow in owner and repository names + "https://github.com/ow ner/repo", + "https://github.com/owner/re%20po", + ]; + for (const repoUrl of cases) { + assert.equal(resolveRepoTarget(repoUrl, "").valid, false, repoUrl); + } +}); + +test("prefers the API base URL override over the derived one", () => { + const overridden = (override: string) => + resolveRepoTarget("https://github.example.com/owner/repo", override); + + assert.deepEqual(overridden("https://api.example.com/v3/"), { + valid: true, + target: { + owner: "owner", + repo: "repo", + apiBaseUrl: "https://api.example.com/v3", + }, + }); + // A blank override is ignored, an unparsable one is refused instead + assert.deepEqual(overridden(" "), { + valid: true, + target: { + owner: "owner", + repo: "repo", + apiBaseUrl: "https://github.example.com/api/v3", + }, + }); + assert.equal(overridden("ftp://api.example.com").valid, false); +}); diff --git a/src/github/repo-url.ts b/src/github/repo-url.ts new file mode 100644 index 00000000..ef24109a --- /dev/null +++ b/src/github/repo-url.ts @@ -0,0 +1,120 @@ +/** + * Everything needed to build the REST API URLs of a repository. + * The API base URL never ends with a slash. + */ +export type RepoTarget = { + owner: string; + repo: string; + apiBaseUrl: string; +}; + +/** + * Result of resolving the repository settings, the error is meant to be shown to the user. + */ +export type ResolvedRepoTarget = + { valid: true; target: RepoTarget } | { valid: false; error: string }; + +const GITHUB_COM_HOSTNAMES = ["github.com", "www.github.com"]; + +// GitHub only allows these characters in owner and repository names +const OWNER_OR_REPO_PATTERN = /^[A-Za-z0-9._-]+$/; + +const HAS_SCHEME_PATTERN = /^[A-Za-z][A-Za-z0-9+.-]*:\/\//; + +/** + * Parses an HTTP URL, assuming HTTPS if the scheme is missing. + * + * @param rawUrl URL to parse, can be surrounded by whitespace + * @returns The parsed URL, or null if it's not a valid HTTP URL + */ +function parseHttpUrl(rawUrl: string): URL | null { + const trimmed = rawUrl.trim(); + if (trimmed === "") { + return null; + } + try { + const url = new URL( + HAS_SCHEME_PATTERN.test(trimmed) ? trimmed : `https://${trimmed}`, + ); + if (url.protocol !== "https:" && url.protocol !== "http:") { + return null; + } + return url; + } catch { + return null; + } +} + +/** + * Derives the REST API base URL of the instance hosting a repository. + * This is different for self-hosted ghe vs github hosted by github itself + */ +function deriveApiBaseUrl(url: URL): string { + const hostname = url.hostname.toLowerCase(); + if (GITHUB_COM_HOSTNAMES.includes(hostname)) { + return "https://api.github.com"; // main github + } + if (hostname.endsWith(".ghe.com")) { // github-hosted enterprise cloud + return `https://api.${hostname}`; + } + return `${url.protocol}//${url.host}/api/v3`; // someone's self-hosted ghe server +} + +/** + * Resolves the repository and API base URL to use for requests. + * + * Extra path segments are ignored so that the URL of any repository page works, + * same goes for the `.git` suffix of clone URLs. + * + * @param repoUrl Full URL of the repository to sync + * @param apiBaseUrlOverride API base URL to use instead of the derived one, can be empty + * @returns The resolved target, or the reason why the settings are not valid + */ +export function resolveRepoTarget( + repoUrl: string, + apiBaseUrlOverride: string, +): ResolvedRepoTarget { + const url = parseHttpUrl(repoUrl); + if (url === null) { + return { + valid: false, + error: + "Invalid repository URL, it must look like https://github.com/owner/repository", + }; + } + + const segments = url.pathname.split("/").filter((s) => s !== ""); + if (segments.length < 2) { + return { + valid: false, + error: + "The repository URL must contain both owner and repository name, " + + "like https://github.com/owner/repository", + }; + } + + const owner = segments[0]; + const repo = segments[1].replace(/\.git$/, ""); + if (!OWNER_OR_REPO_PATTERN.test(owner) || !OWNER_OR_REPO_PATTERN.test(repo)) { + return { + valid: false, + error: `Invalid owner or repository name in the repository URL: "${owner}/${repo}"`, + }; + } + + let apiBaseUrl = deriveApiBaseUrl(url); + if (apiBaseUrlOverride.trim() !== "") { + const override = parseHttpUrl(apiBaseUrlOverride); + if (override === null) { + return { + valid: false, + error: + "Invalid API base URL, it must look like https://github.example.com/api/v3", + }; + } + // A trailing slash would end up in the middle of every request URL + apiBaseUrl = `${override.protocol}//${override.host}${override.pathname.replace(/\/+$/, "")}`; + } + + return { valid: true, target: { owner, repo, apiBaseUrl } }; +} diff --git a/src/main.ts b/src/main.ts index bb43fb31..1165e82c 100644 --- a/src/main.ts +++ b/src/main.ts @@ -5,7 +5,12 @@ import { normalizePath, Notice, } from "obsidian"; -import { GitHubSyncSettings, DEFAULT_SETTINGS } from "./settings/settings"; +import { + GitHubSyncSettings, + DEFAULT_SETTINGS, + isSyncConfigured, + migrateSettings, +} from "./settings/settings"; import GitHubSyncSettingsTab from "./settings/tab"; import SyncManager, { ConflictFile, ConflictResolution } from "./sync-manager"; import Logger from "./logger"; @@ -41,12 +46,7 @@ export default class GitHubSyncPlugin extends Plugin { private conflicts: ConflictFile[] = []; async onUserEnable() { - if ( - this.settings.githubToken === "" || - this.settings.githubOwner === "" || - this.settings.githubRepo === "" || - this.settings.githubBranch === "" - ) { + if (!isSyncConfigured(this.settings)) { new Notice("Go to settings to configure syncing"); } } @@ -142,12 +142,7 @@ export default class GitHubSyncPlugin extends Plugin { } async sync() { - if ( - this.settings.githubToken === "" || - this.settings.githubOwner === "" || - this.settings.githubRepo === "" || - this.settings.githubBranch === "" - ) { + if (!isSyncConfigured(this.settings)) { new Notice("Sync plugin not configured"); return; } @@ -274,6 +269,9 @@ export default class GitHubSyncPlugin extends Plugin { async loadSettings() { this.settings = Object.assign({}, DEFAULT_SETTINGS, await this.loadData()); + if (migrateSettings(this.settings)) { + await this.saveSettings(); + } } async saveSettings() { diff --git a/src/settings/settings.test.ts b/src/settings/settings.test.ts new file mode 100644 index 00000000..0a7a92e8 --- /dev/null +++ b/src/settings/settings.test.ts @@ -0,0 +1,53 @@ +import test from "node:test"; +import * as assert from "node:assert/strict"; +import { + DEFAULT_SETTINGS, + GitHubSyncSettings, + isSyncConfigured, + migrateSettings, +} from "./settings"; + +// Mimics how the plugin loads the data saved in the vault +const load = (saved: object): GitHubSyncSettings => + Object.assign({}, DEFAULT_SETTINGS, saved); + +test("migrates the repository of vaults synced with older versions", () => { + const settings = load({ + firstSync: false, + githubToken: "token", + githubOwner: "owner", + githubRepo: "repo", + githubBranch: "main", + }); + + assert.equal(migrateSettings(settings), true); + assert.equal(settings.githubRepoUrl, "https://github.com/owner/repo"); + assert.equal("githubOwner" in settings, false); + assert.equal("githubRepo" in settings, false); + // The vault must keep syncing without going through the first sync again + assert.equal(isSyncConfigured(settings), true); + assert.equal(migrateSettings(settings), false); +}); + +test("drops the legacy settings of an incomplete setup", () => { + const settings = load({ githubOwner: "owner", githubBranch: "main" }); + + assert.equal(migrateSettings(settings), true); + assert.equal(settings.githubRepoUrl, ""); + assert.equal(isSyncConfigured(settings), false); +}); + +test("keeps an already migrated repository URL", () => { + const settings = load({ + githubRepoUrl: "https://github.example.com/owner/repo", + githubOwner: "stale", + githubRepo: "stale", + }); + + assert.equal(migrateSettings(settings), true); + assert.equal(settings.githubRepoUrl, "https://github.example.com/owner/repo"); +}); + +test("has nothing to migrate on a fresh install", () => { + assert.equal(migrateSettings(load({})), false); +}); diff --git a/src/settings/settings.ts b/src/settings/settings.ts index 97baf096..372ff651 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -1,8 +1,10 @@ +import { resolveRepoTarget } from "src/github/repo-url"; + export interface GitHubSyncSettings { firstSync: boolean; githubToken: string; - githubOwner: string; - githubRepo: string; + githubRepoUrl: string; + githubApiBaseUrl: string; githubBranch: string; syncStrategy: "manual" | "interval"; syncInterval: number; @@ -19,8 +21,8 @@ export interface GitHubSyncSettings { export const DEFAULT_SETTINGS: GitHubSyncSettings = { firstSync: true, githubToken: "", - githubOwner: "", - githubRepo: "", + githubRepoUrl: "", + githubApiBaseUrl: "", githubBranch: "main", syncStrategy: "manual", syncInterval: 1, @@ -33,3 +35,49 @@ export const DEFAULT_SETTINGS: GitHubSyncSettings = { showConflictsRibbonButton: true, enableLogging: false, }; + +/** + * Settings replaced by `githubRepoUrl`, they're still found in data saved by older versions. + */ +interface LegacyRepoSettings { + githubOwner?: string; + githubRepo?: string; +} + +/** + * Converts settings saved by older versions, that only supported github.com, + * to the current format. + * + * @param settings Settings to migrate in place + * @returns True if anything changed and the settings must be saved + */ +export function migrateSettings( + settings: GitHubSyncSettings & LegacyRepoSettings, +): boolean { + if (settings.githubOwner === undefined && settings.githubRepo === undefined) { + return false; + } + + // A missing owner or repository means the setup was never completed + if ( + settings.githubRepoUrl === "" && + settings.githubOwner && + settings.githubRepo + ) { + settings.githubRepoUrl = `https://github.com/${settings.githubOwner}/${settings.githubRepo}`; + } + delete settings.githubOwner; + delete settings.githubRepo; + return true; +} + +/** + * Returns true if all the settings necessary to sync are set and valid. + */ +export function isSyncConfigured(settings: GitHubSyncSettings): boolean { + return ( + settings.githubToken !== "" && + settings.githubBranch !== "" && + resolveRepoTarget(settings.githubRepoUrl, settings.githubApiBaseUrl).valid + ); +} diff --git a/src/settings/tab.ts b/src/settings/tab.ts index 8921460d..77b9ec39 100644 --- a/src/settings/tab.ts +++ b/src/settings/tab.ts @@ -8,6 +8,7 @@ import { } from "obsidian"; import GitHubSyncPlugin from "src/main"; import { copyToClipboard } from "src/utils"; +import { resolveRepoTarget } from "src/github/repo-url"; export default class GitHubSyncSettingsTab extends PluginSettingTab { plugin: GitHubSyncPlugin; @@ -52,42 +53,62 @@ export default class GitHubSyncSettingsTab extends PluginSettingTab { tokenInput = text; }); - new Setting(containerEl) - .setName("Owner") - .setDesc("Owner of the repository to sync") - .addText((text) => - text - .setPlaceholder("Owner") - .setValue(this.plugin.settings.githubOwner) - .onChange(async (value) => { - this.plugin.settings.githubOwner = value; - await this.plugin.saveSettings(); - }), + const repoUrlSetting = new Setting(containerEl) + .setName("Repository URL") + .setDesc( + "Full URL of the repository to sync, usually https://github.com/owner/repository", + ); + const errorEl = repoUrlSetting.descEl.createDiv({ + cls: "invalid-setting-message", + }); + // Errors of both this and the API base URL setting are shown in here, + // an empty URL only means the user hasn't finished the setup yet + const showError = () => { + const { githubRepoUrl, githubApiBaseUrl } = this.plugin.settings; + const resolved = resolveRepoTarget(githubRepoUrl, githubApiBaseUrl); + errorEl.setText( + githubRepoUrl.trim() === "" || resolved.valid ? "" : resolved.error, ); + }; + repoUrlSetting.addText((text) => + text + .setPlaceholder("https://github.com/owner/repository") + .setValue(this.plugin.settings.githubRepoUrl) + .onChange(async (value) => { + this.plugin.settings.githubRepoUrl = value; + await this.plugin.saveSettings(); + showError(); + }), + ); + showError(); new Setting(containerEl) - .setName("Repository") - .setDesc("Name of the repository to sync") + .setName("Repository branch") + .setDesc("Branch to sync") .addText((text) => text - .setPlaceholder("Repository") - .setValue(this.plugin.settings.githubRepo) + .setPlaceholder("Branch name") + .setValue(this.plugin.settings.githubBranch) .onChange(async (value) => { - this.plugin.settings.githubRepo = value; + this.plugin.settings.githubBranch = value; await this.plugin.saveSettings(); }), ); new Setting(containerEl) - .setName("Repository branch") - .setDesc("Branch to sync") + .setName("API base URL") + .setDesc( + "Leave empty to derive it from the repository URL. Set it only if your " + + "GitHub Enterprise instance serves its REST API from another address.", + ) .addText((text) => text - .setPlaceholder("Branch name") - .setValue(this.plugin.settings.githubBranch) + .setPlaceholder("https://github.example.com/api/v3") + .setValue(this.plugin.settings.githubApiBaseUrl) .onChange(async (value) => { - this.plugin.settings.githubBranch = value; + this.plugin.settings.githubApiBaseUrl = value; await this.plugin.saveSettings(); + showError(); }), ); diff --git a/styles.css b/styles.css index 27862183..4e88845f 100644 --- a/styles.css +++ b/styles.css @@ -7,3 +7,8 @@ height: 100%; width: 100%; } + +.invalid-setting-message:not(:empty) { + color: var(--text-error); + margin-top: var(--size-4-1); +}