From acd1cd91f5c9be0f6d631951821145908ca4331e Mon Sep 17 00:00:00 2001 From: zhang hongyuan <100520587+Metastarx@users.noreply.github.com> Date: Fri, 9 Oct 2026 02:12:23 +0800 Subject: [PATCH] apps/web/src/common/utils/format.tsx:19 - checkHasSameRepoPath matches repo paths by substring and mislabels providers Signed-off-by: zhang hongyuan <100520587+Metastarx@users.noreply.github.com> --- apps/web/src/common/utils/format.test.tsx | 144 +++++++++++++++++++++- apps/web/src/common/utils/format.tsx | 44 +++++-- apps/web/src/common/utils/url.test.ts | 62 ++++++++++ apps/web/src/common/utils/url.ts | 33 +++++ 4 files changed, 273 insertions(+), 10 deletions(-) diff --git a/apps/web/src/common/utils/format.test.tsx b/apps/web/src/common/utils/format.test.tsx index 7b6f1ea559..ff0b8c2f71 100644 --- a/apps/web/src/common/utils/format.test.tsx +++ b/apps/web/src/common/utils/format.test.tsx @@ -1,4 +1,9 @@ -import { formatRepoName, shortenAxisLabel, roundedNum } from './format'; +import { + formatRepoName, + checkHasSameRepoPath, + shortenAxisLabel, + roundedNum, +} from './format'; describe('format', () => { it('formatRepoName', function () { @@ -20,6 +25,143 @@ describe('format', () => { }); }); + it('formatRepoName does not mark unrelated repos as the same repository', function () { + // Regression: apache/dubbo is a substring of apache/dubbo-go, so the old + // substring check counted two distinct repos as the same one and the legend + // rendered the wrong provider for both of them. + const result = formatRepoName({ + label: 'https://github.com/apache/dubbo', + compareLabels: [ + 'https://github.com/apache/dubbo', + 'https://github.com/apache/dubbo-go', + ], + }); + expect(result).toEqual({ + name: 'dubbo', + meta: { + namespace: 'apache', + provider: 'Github', + showProvider: false, + }, + }); + }); + + it('formatRepoName marks the same repo across providers despite trailing slash and case', function () { + // Regression: getPathname kept the trailing slash, so + // github.com/oss-compass/compass-web/ never matched + // gitee.com/oss-compass/compass-web and the same repo was left without a + // provider, producing two identical legend entries. + const result = formatRepoName({ + label: 'https://github.com/oss-compass/compass-web/', + compareLabels: [ + 'https://github.com/oss-compass/compass-web/', + 'https://gitee.com/oss-compass/compass-web', + ], + }); + expect(result).toEqual({ + name: 'compass-web', + meta: { + namespace: 'oss-compass', + provider: 'Github', + showProvider: true, + }, + }); + }); + + it('formatRepoName ignores non-URL labels', function () { + expect( + formatRepoName({ + label: 'compass-web', + compareLabels: ['compass-web', 'gitee.com/oss-compass/compass-web'], + }) + ).toEqual({ name: 'compass-web' }); + }); + + it('checkHasSameRepoPath only matches the exact repository path', function () { + const testCases = [ + { + // apache/dubbo is a substring of apache/dubbo-go, so the previous + // indexOf-based check wrongly reported these as the same repo. + label: 'https://github.com/apache/dubbo', + labels: [ + 'https://github.com/apache/dubbo', + 'https://github.com/apache/dubbo-go', + ], + result: false, + }, + { + label: 'https://github.com/cli/cli', + labels: [ + 'https://github.com/cli/cli', + 'https://github.com/cli/cli-extra', + ], + result: false, + }, + { + // Two completely unrelated repos must never be flagged. + label: 'https://github.com/apache/dubbo', + labels: [ + 'https://github.com/apache/dubbo', + 'https://github.com/spring-projects/spring-boot', + ], + result: false, + }, + { + // Same repo on two hosts is the intended positive case. + label: 'https://github.com/cli/cli', + labels: ['https://github.com/cli/cli', 'https://gitee.com/cli/cli'], + result: true, + }, + { + // Trailing slashes used to break the equality check. + label: 'https://github.com/oss-compass/compass-web/', + labels: [ + 'https://github.com/oss-compass/compass-web/', + 'https://gitee.com/oss-compass/compass-web', + ], + result: true, + }, + { + // Case differences must not hide the same repo either. + label: 'https://github.com/OSS-Compass/Compass-Web', + labels: [ + 'https://github.com/OSS-Compass/Compass-Web', + 'https://gitee.com/oss-compass/compass-web', + ], + result: true, + }, + { + // The label does not have to be present in compareLabels. + label: 'https://github.com/cli/cli', + labels: ['https://gitee.com/cli/cli', 'https://gitcode.com/cli/cli'], + result: true, + }, + { + // Duplicates on a single provider have nothing to disambiguate. + label: 'https://github.com/apache/dubbo', + labels: [ + 'https://github.com/apache/dubbo', + 'https://github.com/apache/dubbo', + ], + result: false, + }, + { + // Non-URL labels cannot be compared reliably. + label: 'apache/dubbo', + labels: ['apache/dubbo', 'gitee.com/apache/dubbo'], + result: false, + }, + { + label: 'https://github.com/apache/dubbo', + labels: [], + result: false, + }, + ]; + testCases.forEach((item) => { + expect(checkHasSameRepoPath(item.label, item.labels)).toBe(item.result); + }); + }); + it('shortenAxisLabel ', function () { expect(shortenAxisLabel('10000')).toEqual('10k'); expect(shortenAxisLabel('10001')).toEqual('10k'); diff --git a/apps/web/src/common/utils/format.tsx b/apps/web/src/common/utils/format.tsx index b684d448ba..0287f28d70 100644 --- a/apps/web/src/common/utils/format.tsx +++ b/apps/web/src/common/utils/format.tsx @@ -1,6 +1,6 @@ import { + getCanonicalRepoPath, getNameSpace, - getPathname, getProvider, getRepoName, toFixed, @@ -9,21 +9,47 @@ import capitalize from 'lodash/capitalize'; import { OptionDataValue } from 'echarts/types/src/util/types'; /** - * check is need show provider in legend + * Whether the provider should be appended to the legend/tooltip of `label`. + * * eg: * https://github.com/cli/cli * https://gitee.com/cli/cli * - * need show gitee or github + * need show gitee or github, because the two entries would otherwise render the + * exact same `name` / `namespace` and be indistinguishable. + * + * The comparison is done on the canonical `namespace/repo` path (see + * `getCanonicalRepoPath`) instead of on a substring of the URL. A substring + * test is wrong in both directions: + * - false positive: `apache/dubbo` is a substring of `apache/dubbo-go`, so two + * unrelated repos were treated as the same one; + * - false negative: a trailing slash or a different letter case made the very + * same repo (`github.com/oss-compass/compass-web/` vs + * `gitee.com/oss-compass/compass-web`) look like two different ones. + * + * The provider is only useful when the same canonical path really is present + * under two different hosts, so the result requires at least two distinct + * providers; a path repeated on a single provider has nothing to disambiguate. */ export const checkHasSameRepoPath = (label: string, labels: string[]) => { - const pathname = getPathname(label); - - let count = 0; - return labels.some((item) => { - if (item.indexOf(pathname) > -1) count++; - return count >= 2; + const canonicalPath = getCanonicalRepoPath(label); + // Non-URL labels (or URLs without a full namespace/repo pair) cannot be + // compared reliably, so never append a provider for them. + if (!canonicalPath) return false; + + const providers = new Set(); + let matches = 0; + + // Fold the label in as well so that a single duplicate inside `labels` cannot + // satisfy the `matches >= 2` condition on its own. + [label, ...(labels || [])].forEach((item) => { + if (getCanonicalRepoPath(item) !== canonicalPath) return; + matches += 1; + // Hosts are case-insensitive, so compare them lowercased as well. + providers.add(getProvider(item).toLowerCase()); }); + + return matches >= 2 && providers.size >= 2; }; export const formatLabel = ( diff --git a/apps/web/src/common/utils/url.test.ts b/apps/web/src/common/utils/url.test.ts index 15d931dec7..c9e7604848 100644 --- a/apps/web/src/common/utils/url.test.ts +++ b/apps/web/src/common/utils/url.test.ts @@ -1,5 +1,6 @@ import { getPathname, + getCanonicalRepoPath, getProvider, getNameSpace, getRepoName, @@ -33,6 +34,67 @@ describe('utils url ', () => { }); }); + it('getCanonicalRepoPath', () => { + const testCases = [ + // Every provider for the same namespace/repo must collapse to one value, + // otherwise the same repo compared across hosts looks like two entries. + { + input: 'https://github.com/oss-compass/compass-web', + result: 'oss-compass/compass-web', + }, + { + input: 'https://gitee.com/oss-compass/compass-web', + result: 'oss-compass/compass-web', + }, + { + input: 'http://gitcode.com/oss-compass/compass-web', + result: 'oss-compass/compass-web', + }, + // Trailing slashes used to survive getPathname and break equality. + { + input: 'https://github.com/oss-compass/compass-web/', + result: 'oss-compass/compass-web', + }, + { + input: 'https://gitee.com/oss-compass/compass-web/', + result: 'oss-compass/compass-web', + }, + // Hosts and owner/repo names are compared case-insensitively. + { + input: 'https://github.com/OSS-Compass/Compass-Web', + result: 'oss-compass/compass-web', + }, + // A `.git` suffix is a clone URL artifact, not part of the identity. + { + input: 'https://github.com/apache/dubbo.git', + result: 'apache/dubbo', + }, + // Deeper paths (issues, trees, ...) must not change the repo identity. + { + input: 'https://github.com/apache/dubbo/issues/1', + result: 'apache/dubbo', + }, + // A longer basename is a different repository, not the same one. + { + input: 'https://github.com/apache/dubbo-go', + result: 'apache/dubbo-go', + }, + { + input: 'https://github.com/cli/cli-extra', + result: 'cli/cli-extra', + }, + // Non-URLs and incomplete paths cannot be compared and must be empty. + { input: 'apache/dubbo', result: '' }, + { input: 'https://github.com/apache', result: '' }, + { input: 'https://github.com/', result: '' }, + { input: '', result: '' }, + { input: undefined, result: '' }, + ]; + testCases.map((item) => { + expect(getCanonicalRepoPath(item.input)).toEqual(item.result); + }); + }); + it('getProvider', () => { const testCases = [ { input: 'https://github.com/EdmondFrank/.emacs.d', result: 'github' }, diff --git a/apps/web/src/common/utils/url.ts b/apps/web/src/common/utils/url.ts index 8a48f0aaf7..d3b0e8d6f2 100644 --- a/apps/web/src/common/utils/url.ts +++ b/apps/web/src/common/utils/url.ts @@ -20,6 +20,39 @@ export const getAllPathname = (urls: string[]) => { return uniq(pathName); }; +/** + * Canonical identity of a repository URL, used to tell whether two labels + * point at the *same* repository (possibly hosted on different providers). + * + * Only the `namespace/repo` pair is kept, lowercased, with any trailing slash + * or `.git` suffix removed. The host is deliberately dropped here because it is + * handled separately by `getProvider`: two URLs that share this canonical path + * but have different hosts are the exact case where the provider has to be + * rendered to keep the legend/tooltip unambiguous. + * + * Returning '' for non-URLs (or URLs without a full `namespace/repo` pair) lets + * callers skip entries that cannot be compared instead of matching them by + * accident. + */ +export function getCanonicalRepoPath(url?: string): string { + if (!url) return ''; + + const parsed = parseUrl(url); + if (!parsed) return ''; + + const segments = parsed.pathname + .split('/') + .map((segment) => segment.trim()) + .filter(Boolean); + if (segments.length < 2) return ''; + + const namespace = segments[0].toLowerCase(); + const repo = segments[1].toLowerCase().replace(/\.git$/, ''); + if (!namespace || !repo) return ''; + + return `${namespace}/${repo}`; +} + // gg/cli => gg export function getFirstPathSegment(path: string) { if (!path) return '';