Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 36 additions & 12 deletions lib/ContentPluginModule.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,12 @@ class ContentPluginModule extends AbstractApiModule {
for (const i of await this.framework.runCliCommand('getPluginUpdateInfos')) {
if (dbInfo[i.name]?.version !== i.matchedVersion) {
this.log('debug', 'SYNC', i.name, 'local:', dbInfo[i.name]?.version, 'fw:', i.matchedVersion)
await this.insertOrUpdate({ ...(await i.getInfo()), type: await i.getType(), isLocalInstall: i.isLocalSource })
const pluginInfo = { ...(await i.getInfo()), type: await i.getType(), isLocalInstall: i.isLocalSource }
if (i.isGitSource) {
pluginInfo.gitUrl = i.gitUrl
pluginInfo.gitRef = i.gitRef ?? null
}
await this.insertOrUpdate(pluginInfo)
}
}
}
Expand All @@ -180,6 +185,7 @@ class ContentPluginModule extends AbstractApiModule {
// For local installs, check if backup exists if main plugin directory doesn't
const pluginsWithPaths = await Promise.all(missingPlugins.map(async (p) => {
if (!p.isLocalInstall) {
if (p.gitUrl) return p.gitUrl + (p.gitRef ? `#${p.gitRef}` : '')
return `${p.name}@${p.version}`
}
const pluginDir = this.getConfig('pluginDir')
Expand Down Expand Up @@ -330,29 +336,47 @@ class ContentPluginModule extends AbstractApiModule {
* @returns Resolves with plugin DB data
*/
async installPlugin (pluginName, versionOrPath, options = { strict: false, force: false }) {
const pluginData = await this.findOne({ name: String(pluginName) }, { includeUpdateInfo: true, strict: false })
const { name, version, sourcePath, isLocalInstall } = await this.processPluginFiles({ ...pluginData, sourcePath: versionOrPath })
const existingPlugin = await this.findOne({ name }, { strict: false })
const isGitUrl = /^https?:\/\//.test(versionOrPath)

let name, version, sourcePath, isLocalInstall, gitUrl, gitRef

if (isGitUrl) {
name = pluginName
sourcePath = null
isLocalInstall = false
gitUrl = versionOrPath.split('#')[0]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gitRef is stripped and never storedversionOrPath.split('#')[0] discards the branch/tag before saving. When getMissingPlugins returns p.gitUrl on a restart, the CLI installs from the default branch HEAD rather than the originally-pinned ref.

The schema needs a gitRef field alongside gitUrl, and it should be stored here:

gitUrl = versionOrPath.split('#')[0]
gitRef = versionOrPath.includes('#') ? versionOrPath.split('#')[1] : null

Then in dbData: if (isGitUrl) { dbData.gitUrl = gitUrl; dbData.gitRef = gitRef }

And getMissingPlugins should reconstruct the full URL:

if (p.gitUrl) return p.gitUrl + (p.gitRef ? `#${p.gitRef}` : '')

This also fixes the NO_CHANGE detection issue in adaptframework-extras PR #10 once it compares existing?.gitRef === (ref ?? null) as well.

gitRef = versionOrPath.includes('#') ? versionOrPath.split('#')[1] : null
} else {
const pluginData = await this.findOne({ name: String(pluginName) }, { strict: false })
;({ name, version, sourcePath, isLocalInstall } = await this.processPluginFiles({ ...pluginData, sourcePath: versionOrPath }))
}
const existingPlugin = isGitUrl ? null : await this.findOne({ name }, { strict: false })

if (existingPlugin) {
if (!options.force && semver.lte(version, existingPlugin.version)) {
if (!isGitUrl && !options.force && semver.lte(version, existingPlugin.version)) {
throw this.app.errors.CONTENTPLUGIN_ALREADY_EXISTS
.setData({ name: existingPlugin.name, version: existingPlugin.version })
}
}
const [data] = await this.framework.runCliCommand('installPlugins', { plugins: [`${name}@${sourcePath ?? version}`] })
const info = await this.insertOrUpdate({
const pluginArg = isGitUrl ? versionOrPath : `${name}@${sourcePath ?? version}`
const [data] = await this.framework.runCliCommand('installPlugins', { plugins: [pluginArg] })
if (!data.isInstallSuccessful) {
throw this.app.errors.CONTENTPLUGIN_CLI_INSTALL_FAILED
.setData({ name: data.name || pluginName })
}
const dbData = {
...(await data.getInfo()),
type: await data.getType(),
isLocalInstall
})
if (!data.isInstallSuccessful) {
throw this.app.errors.CONTENTPLUGIN_CLI_INSTALL_FAILED
.setData({ name })
}
if (isGitUrl) {
dbData.gitUrl = gitUrl
dbData.gitRef = gitRef
}
const info = await this.insertOrUpdate(dbData)
if (!info.targetAttribute) {
throw this.app.errors.CONTENTPLUGIN_ATTR_MISSING
.setData({ name })
.setData({ name: info.name || data.name || pluginName })
}
await this.processPluginSchemas(data)
return info
Expand Down
9 changes: 9 additions & 0 deletions schema/contentplugin.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@
"description": "Whether the plugin has been installed locally (as opposed to with the CLI)",
"type": "boolean"
},
"gitUrl": {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add "format": "uri" to match the repository field in the adaptframework-extras schema and reject malformed values before they reach the CLI:

"gitUrl": {
  "description": "The HTTPS git URL this plugin was installed from, if applicable",
  "type": "string",
  "format": "uri"
}

"description": "The HTTPS git URL this plugin was installed from, if applicable",
"type": "string",
"format": "uri"
},
"gitRef": {
"description": "The git branch, tag, or commit this plugin was installed from, if applicable",
"type": "string"
},
"isEnabled": {
"description": "",
"type": "boolean",
Expand Down
163 changes: 163 additions & 0 deletions tests/ContentPluginModule.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
import { describe, it, mock } from 'node:test'
import assert from 'node:assert/strict'
import ContentPluginModule from '../lib/ContentPluginModule.js'

describe('ContentPluginModule.installPlugin()', () => {
it('should install git URLs directly and persist gitUrl/gitRef', async () => {
const runCliCommand = mock.fn(async () => [{
isInstallSuccessful: true,
getInfo: async () => ({ name: 'adapt-hotgrid', version: '2.0.0', targetAttribute: '_component' }),
getType: async () => 'component'
}])
const insertOrUpdate = mock.fn(async (data) => data)
const processPluginFiles = mock.fn(async () => {
throw new Error('processPluginFiles should not be called for git installs')
})
const context = {
framework: { runCliCommand },
processPluginFiles,
insertOrUpdate,
findOne: mock.fn(async () => ({ name: 'adapt-hotgrid', version: '999.0.0' })),
processPluginSchemas: mock.fn(async () => {}),
app: {
errors: {
CONTENTPLUGIN_ALREADY_EXISTS: { setData: (data) => Object.assign(new Error('already exists'), { data }) },
CONTENTPLUGIN_CLI_INSTALL_FAILED: { setData: (data) => Object.assign(new Error('cli failed'), { data }) },
CONTENTPLUGIN_ATTR_MISSING: { setData: (data) => Object.assign(new Error('attr missing'), { data }) }
}
}
}

const result = await ContentPluginModule.prototype.installPlugin.call(
context,
'',
'https://github.com/org/adapt-hotgrid.git#v2.0.0',
{ force: false }
)

assert.equal(runCliCommand.mock.calls[0].arguments[0], 'installPlugins')
assert.deepEqual(runCliCommand.mock.calls[0].arguments[1], {
plugins: ['https://github.com/org/adapt-hotgrid.git#v2.0.0']
})
assert.equal(context.findOne.mock.callCount(), 0)
assert.equal(processPluginFiles.mock.callCount(), 0)
assert.equal(insertOrUpdate.mock.calls[0].arguments[0].gitUrl, 'https://github.com/org/adapt-hotgrid.git')
assert.equal(insertOrUpdate.mock.calls[0].arguments[0].gitRef, 'v2.0.0')
assert.equal(result.name, 'adapt-hotgrid')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error paths are not covered — this test only exercises the happy path (isInstallSuccessful: true, targetAttribute present). Neither CONTENTPLUGIN_CLI_INSTALL_FAILED nor CONTENTPLUGIN_ATTR_MISSING is tested, so the stale-name bug (see inline comment on line 368) would not be caught by CI.

Consider adding two cases:

  1. isInstallSuccessful: false → assert the thrown error carries the real plugin name (not '')
  2. targetAttribute missing from getInfo() → same assertion

})

it('should throw install failure with CLI plugin name and not persist failed install', async () => {
const runCliCommand = mock.fn(async () => [{
name: 'adapt-hotgrid',
isInstallSuccessful: false
}])
const insertOrUpdate = mock.fn(async (data) => data)
const context = {
framework: { runCliCommand },
insertOrUpdate,
processPluginSchemas: mock.fn(async () => {}),
app: {
errors: {
CONTENTPLUGIN_CLI_INSTALL_FAILED: { setData: (data) => Object.assign(new Error('cli failed'), { data }) },
CONTENTPLUGIN_ATTR_MISSING: { setData: (data) => Object.assign(new Error('attr missing'), { data }) }
}
}
}

await assert.rejects(
ContentPluginModule.prototype.installPlugin.call(context, '', 'https://github.com/org/adapt-hotgrid.git#v2.0.0'),
e => {
assert.equal(e.message, 'cli failed')
assert.equal(e.data.name, 'adapt-hotgrid')
return true
}
)
assert.equal(insertOrUpdate.mock.callCount(), 0)
})

it('should throw missing attr with resolved plugin name', async () => {
const runCliCommand = mock.fn(async () => [{
name: 'adapt-hotgrid',
isInstallSuccessful: true,
getInfo: async () => ({ name: 'adapt-hotgrid', version: '2.0.0' }),
getType: async () => 'component'
}])
const insertOrUpdate = mock.fn(async (data) => data)
const context = {
framework: { runCliCommand },
insertOrUpdate,
processPluginSchemas: mock.fn(async () => {}),
app: {
errors: {
CONTENTPLUGIN_CLI_INSTALL_FAILED: { setData: (data) => Object.assign(new Error('cli failed'), { data }) },
CONTENTPLUGIN_ATTR_MISSING: { setData: (data) => Object.assign(new Error('attr missing'), { data }) }
}
}
}

await assert.rejects(
ContentPluginModule.prototype.installPlugin.call(context, '', 'https://github.com/org/adapt-hotgrid.git#v2.0.0'),
e => {
assert.equal(e.message, 'attr missing')
assert.equal(e.data.name, 'adapt-hotgrid')
return true
}
)
})
})

describe('ContentPluginModule.getMissingPlugins()', () => {
it('should return gitUrl and gitRef for missing git-installed plugins', async () => {
const context = {
find: async () => ([
{
name: 'adapt-hotgrid',
version: '2.0.0',
isLocalInstall: false,
gitUrl: 'https://github.com/org/adapt-hotgrid.git',
gitRef: 'v2.0.0'
},
{ name: 'adapt-text', version: '1.0.0', isLocalInstall: false }
]),
framework: {
getManifestPlugins: async () => [],
getInstalledPlugins: async () => []
}
}
const result = await ContentPluginModule.prototype.getMissingPlugins.call(context)
assert.deepEqual(result, [
'https://github.com/org/adapt-hotgrid.git#v2.0.0',
'adapt-text@1.0.0'
])
})
})

describe('ContentPluginModule.syncPluginData()', () => {
it('should persist gitUrl and gitRef for git sources', async () => {
const insertOrUpdate = mock.fn(async () => {})
const context = {
log: mock.fn(),
find: async () => [],
insertOrUpdate,
framework: {
runCliCommand: async () => ([
{
name: 'adapt-hotgrid',
matchedVersion: '2.0.0',
isLocalSource: false,
isGitSource: true,
gitUrl: 'https://github.com/org/adapt-hotgrid.git',
gitRef: 'v2.0.0',
getInfo: async () => ({ name: 'adapt-hotgrid', version: '2.0.0' }),
getType: async () => 'component'
}
])
}
}

await ContentPluginModule.prototype.syncPluginData.call(context)

assert.equal(insertOrUpdate.mock.calls[0].arguments[0].gitUrl, 'https://github.com/org/adapt-hotgrid.git')
assert.equal(insertOrUpdate.mock.calls[0].arguments[0].gitRef, 'v2.0.0')
})
})
Loading