From c2006412de5f5d25920ee1d01a38f4cc6f9c768a Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Tue, 11 Mar 2025 06:49:09 -0700 Subject: [PATCH] Fix custom nodes license parsing logic (#2975) --- .../infoPanel/tabs/DescriptionTabPanel.vue | 76 ++++---- .../__tests__/DescriptionTabPanel.test.ts | 179 ++++++++++++++++++ 2 files changed, 215 insertions(+), 40 deletions(-) create mode 100644 src/components/dialog/content/manager/infoPanel/tabs/__tests__/DescriptionTabPanel.test.ts diff --git a/src/components/dialog/content/manager/infoPanel/tabs/DescriptionTabPanel.vue b/src/components/dialog/content/manager/infoPanel/tabs/DescriptionTabPanel.vue index 430d0cc4d..91ba8d7ac 100644 --- a/src/components/dialog/content/manager/infoPanel/tabs/DescriptionTabPanel.vue +++ b/src/components/dialog/content/manager/infoPanel/tabs/DescriptionTabPanel.vue @@ -18,22 +18,14 @@ import InfoTextSection, { type TextSection } from '@/components/dialog/content/manager/infoPanel/InfoTextSection.vue' import { components } from '@/types/comfyRegistryTypes' +import { isValidUrl } from '@/utils/formatUtil' const { t } = useI18n() -const props = defineProps<{ +const { nodePack } = defineProps<{ nodePack: components['schemas']['Node'] }>() -const isValidUrl = (url: string): boolean => { - try { - new URL(url) - return true - } catch { - return false - } -} - const isLicenseFile = (filename: string): boolean => { // Match LICENSE, LICENSE.md, LICENSE.txt (case insensitive) const licensePattern = /^license(\.md|\.txt)?$/i @@ -41,7 +33,6 @@ const isLicenseFile = (filename: string): boolean => { } const extractBaseRepoUrl = (repoUrl: string): string => { - // Match GitHub repository URL and extract the base URL const githubRepoPattern = /^(https?:\/\/github\.com\/[^/]+\/[^/]+)/i const match = repoUrl.match(githubRepoPattern) return match ? match[1] : repoUrl @@ -50,60 +41,65 @@ const extractBaseRepoUrl = (repoUrl: string): string => { const createLicenseUrl = (filename: string, repoUrl: string): string => { if (!repoUrl || !filename) return '' - // Use the filename if it's a license file, otherwise use LICENSE const licenseFile = isLicenseFile(filename) ? filename : 'LICENSE' - - // Get the base repository URL const baseRepoUrl = extractBaseRepoUrl(repoUrl) - return `${baseRepoUrl}/blob/main/${licenseFile}` } const parseLicenseObject = ( licenseObj: any ): { text: string; isUrl: boolean } => { - // Get the license file or text const licenseFile = licenseObj.file || licenseObj.text - // If it's a string and a license file, create a URL - if (typeof licenseFile === 'string' && isLicenseFile(licenseFile)) { - const url = createLicenseUrl(licenseFile, props.nodePack.repository) + if ( + typeof licenseFile === 'string' && + isLicenseFile(licenseFile) && + nodePack.repository + ) { + const url = createLicenseUrl(licenseFile, nodePack.repository) return { text: url, isUrl: !!url && isValidUrl(url) } - } - // Otherwise use the text directly - else if (licenseObj.text) { + } else if (licenseObj.text) { return { text: licenseObj.text, isUrl: false } + } else if (typeof licenseFile === 'string') { + // Return the license file name if repository is missing + return { + text: licenseFile, + isUrl: false + } } - - // Default fallback return { text: JSON.stringify(licenseObj), isUrl: false } } -const formatLicense = (license: string): { text: string; isUrl: boolean } => { +const formatLicense = ( + license: string +): { text: string; isUrl: boolean } | null => { + // Treat "{}" JSON string as undefined + if (license === '{}') return null + try { const licenseObj = JSON.parse(license) + // Handle empty object case + if (Object.keys(licenseObj).length === 0) { + return null + } return parseLicenseObject(licenseObj) } catch (e) { - // Not JSON, handle as plain string - // If it's a license file, create a URL - if (isLicenseFile(license)) { - const url = createLicenseUrl(license, props.nodePack.repository) + if (isLicenseFile(license) && nodePack.repository) { + const url = createLicenseUrl(license, nodePack.repository) return { text: url, isUrl: !!url && isValidUrl(url) } } - - // Otherwise return as is return { text: license, isUrl: false @@ -115,25 +111,25 @@ const descriptionSections = computed(() => { const sections: TextSection[] = [ { title: t('g.description'), - text: props.nodePack.description || t('manager.noDescription') + text: nodePack.description || t('manager.noDescription') } ] - if (props.nodePack.repository) { + if (nodePack.repository) { sections.push({ title: t('manager.repository'), - text: props.nodePack.repository, - isUrl: isValidUrl(props.nodePack.repository) + text: nodePack.repository, + isUrl: isValidUrl(nodePack.repository) }) } - if (props.nodePack.license) { - const { text, isUrl } = formatLicense(props.nodePack.license) - if (text) { + if (nodePack.license) { + const licenseInfo = formatLicense(nodePack.license) + if (licenseInfo && licenseInfo.text) { sections.push({ title: t('manager.license'), - text, - isUrl + text: licenseInfo.text, + isUrl: licenseInfo.isUrl }) } } diff --git a/src/components/dialog/content/manager/infoPanel/tabs/__tests__/DescriptionTabPanel.test.ts b/src/components/dialog/content/manager/infoPanel/tabs/__tests__/DescriptionTabPanel.test.ts new file mode 100644 index 000000000..9a92ce46e --- /dev/null +++ b/src/components/dialog/content/manager/infoPanel/tabs/__tests__/DescriptionTabPanel.test.ts @@ -0,0 +1,179 @@ +import { mount } from '@vue/test-utils' +import { describe, expect, it } from 'vitest' +import { createI18n } from 'vue-i18n' + +import enMessages from '@/locales/en/main.json' +import { components } from '@/types/comfyRegistryTypes' + +import DescriptionTabPanel from '../DescriptionTabPanel.vue' + +const i18n = createI18n({ + legacy: false, + locale: 'en', + messages: { + en: enMessages + } +}) + +const TRANSLATIONS = { + description: 'Description', + repository: 'Repository', + license: 'License', + noDescription: 'No description available' +} + +describe('DescriptionTabPanel', () => { + const mountComponent = (props: { + nodePack: Partial + }) => { + return mount(DescriptionTabPanel, { + props, + global: { + plugins: [i18n] + } + }) + } + + const getSectionByTitle = ( + wrapper: ReturnType, + title: string + ) => { + const sections = wrapper + .findComponent({ name: 'InfoTextSection' }) + .props('sections') + return sections.find((s: any) => s.title === title) + } + + const createNodePack = ( + overrides: Partial = {} + ) => ({ + description: 'Test description', + ...overrides + }) + + const licenseTests = [ + { + name: 'handles plain text license', + nodePack: createNodePack({ + license: 'MIT License', + repository: 'https://github.com/user/repo' + }), + expected: { + text: 'MIT License', + isUrl: false + } + }, + { + name: 'handles license file names', + nodePack: createNodePack({ + license: 'LICENSE', + repository: 'https://github.com/user/repo' + }), + expected: { + text: 'https://github.com/user/repo/blob/main/LICENSE', + isUrl: true + } + }, + { + name: 'handles license.md file names', + nodePack: createNodePack({ + license: 'license.md', + repository: 'https://github.com/user/repo' + }), + expected: { + text: 'https://github.com/user/repo/blob/main/license.md', + isUrl: true + } + }, + { + name: 'handles JSON license objects with text property', + nodePack: createNodePack({ + license: JSON.stringify({ text: 'GPL-3.0' }), + repository: 'https://github.com/user/repo' + }), + expected: { + text: 'GPL-3.0', + isUrl: false + } + }, + { + name: 'handles JSON license objects with file property', + nodePack: createNodePack({ + license: JSON.stringify({ file: 'LICENSE.md' }), + repository: 'https://github.com/user/repo' + }), + expected: { + text: 'https://github.com/user/repo/blob/main/LICENSE.md', + isUrl: true + } + }, + { + name: 'handles missing repository URL', + nodePack: createNodePack({ + license: 'LICENSE' + }), + expected: { + text: 'LICENSE', + isUrl: false + } + }, + { + name: 'handles non-GitHub repository URLs', + nodePack: createNodePack({ + license: 'LICENSE', + repository: 'https://gitlab.com/user/repo' + }), + expected: { + text: 'https://gitlab.com/user/repo/blob/main/LICENSE', + isUrl: true + } + } + ] + + describe('license formatting', () => { + licenseTests.forEach((test) => { + it(test.name, () => { + const wrapper = mountComponent({ nodePack: test.nodePack }) + const licenseSection = getSectionByTitle(wrapper, TRANSLATIONS.license) + expect(licenseSection).toBeDefined() + expect(licenseSection.text).toBe(test.expected.text) + expect(licenseSection.isUrl).toBe(test.expected.isUrl) + }) + }) + }) + + describe('description sections', () => { + it('shows description section', () => { + const wrapper = mountComponent({ + nodePack: createNodePack() + }) + const descriptionSection = getSectionByTitle( + wrapper, + TRANSLATIONS.description + ) + expect(descriptionSection).toBeDefined() + expect(descriptionSection.text).toBe('Test description') + }) + + it('shows repository section when available', () => { + const wrapper = mountComponent({ + nodePack: createNodePack({ + repository: 'https://github.com/user/repo' + }) + }) + const repoSection = getSectionByTitle(wrapper, TRANSLATIONS.repository) + expect(repoSection).toBeDefined() + expect(repoSection.text).toBe('https://github.com/user/repo') + expect(repoSection.isUrl).toBe(true) + }) + + it('shows fallback text when description is missing', () => { + const wrapper = mountComponent({ + nodePack: { + description: undefined + } + }) + expect(wrapper.find('p').text()).toBe(TRANSLATIONS.noDescription) + }) + }) +})