diff --git a/src/content/da-api.js b/src/content/da-api.js index 8d697c40f..fa6029ed1 100644 --- a/src/content/da-api.js +++ b/src/content/da-api.js @@ -162,7 +162,8 @@ export class DaClient { } /** - * Deletes a file or folder. Idempotent. + * Deletes a file or folder. Idempotent: 404 is treated as success. + * Throws on transport or server errors so callers don't silently treat them as success. */ async deleteSource(org, repo, daPath) { const url = `${DA_ADMIN}/source/${org}/${repo}${daPath}`; @@ -173,7 +174,10 @@ export class DaClient { if (res.status === 401) { throw new Error('Unauthorized: invalid or missing token'); } - return res.ok || res.status === 204; + if (res.ok || res.status === 204 || res.status === 404) { + return true; + } + throw new Error(`DELETE failed for ${daPath}: ${res.status} ${res.statusText}`); } /** diff --git a/src/content/merge.cmd.js b/src/content/merge.cmd.js index 181b56eef..731bb9688 100644 --- a/src/content/merge.cmd.js +++ b/src/content/merge.cmd.js @@ -96,7 +96,7 @@ export default class MergeCommand { return; } - const token = await getValidToken(log, this._token, contentDir); + const token = await getValidToken(log, this._token, this._dir); const client = new DaClient(token); // Get the HEAD commit to read base blobs diff --git a/src/content/push.cmd.js b/src/content/push.cmd.js index 1df7aafc5..fc84f231c 100644 --- a/src/content/push.cmd.js +++ b/src/content/push.cmd.js @@ -108,16 +108,16 @@ export default class PushCommand { * @param {string} repo * @param {string} contentDir * @param {string[]} targets - * @returns {Promise<{ pushed: number, errors: number, successfullyPushed: Set }>} + * @returns {Promise<{ pushed: number, errors: number }>} */ async _uploadFiles(client, org, repo, contentDir, targets) { const { log } = this; let pushed = 0; let errors = 0; - const successfullyPushed = new Set(); + // processQueue consumes its input array via shift(); copy so the caller's list survives. const results = await processQueue( - targets, + [...targets], async (daPath) => { const localPath = path.join(contentDir, ...daPath.split('/').filter(Boolean)); const ext = daPath.split('.').pop(); @@ -125,7 +125,7 @@ export default class PushCommand { const buffer = await fse.readFile(localPath); await client.putSource(org, repo, daPath, buffer, getContentType(ext)); log.info(` ✓ ${daPath}`); - return { ok: true, daPath }; + return { ok: true }; } catch (err) { log.warn(` ✗ ${daPath}: ${err.message}`); return { ok: false }; @@ -136,13 +136,12 @@ export default class PushCommand { for (const r of results) { if (r.ok) { pushed += 1; - successfullyPushed.add(r.daPath); } else { errors += 1; } } - return { pushed, errors, successfullyPushed }; + return { pushed, errors }; } /** @@ -151,21 +150,21 @@ export default class PushCommand { * @param {string} org * @param {string} repo * @param {string[]} deleted - * @returns {Promise<{ pushed: number, errors: number, successfullyDeleted: Set }>} + * @returns {Promise<{ pushed: number, errors: number }>} */ async _deleteFiles(client, org, repo, deleted) { const { log } = this; let pushed = 0; let errors = 0; - const successfullyDeleted = new Set(); + // processQueue consumes its input array via shift(); copy so the caller's list survives. const results = await processQueue( - deleted, + [...deleted], async (daPath) => { try { await client.deleteSource(org, repo, daPath); log.info(` ✓ deleted ${daPath}`); - return { ok: true, daPath }; + return { ok: true }; } catch (err) { log.warn(` ✗ ${daPath}: ${err.message}`); return { ok: false }; @@ -176,13 +175,12 @@ export default class PushCommand { for (const r of results) { if (r.ok) { pushed += 1; - successfullyDeleted.add(r.daPath); } else { errors += 1; } } - return { pushed, errors, successfullyDeleted }; + return { pushed, errors }; } async run() { @@ -207,12 +205,21 @@ export default class PushCommand { const syncedOid = await resolveSyncedOid(fs, contentDir); const lastSyncTime = await getCommitCommitterTimeMs(fs, contentDir, syncedOid); - let { added, modified, deleted } = await diffCommitTrees(fs, contentDir, syncedOid, headOid); + const fullChanges = await diffCommitTrees(fs, contentDir, syncedOid, headOid); + + const scope = this._pushPath ? this._pushPath.replace(/\/+$/, '') : null; + const inScope = (daPath) => scope === null + || daPath === scope + || daPath.startsWith(`${scope}/`); + const added = fullChanges.added.filter(inScope); + const modified = fullChanges.modified.filter(inScope); + const deleted = fullChanges.deleted.filter(inScope); - const inScope = (daPath) => !this._pushPath || daPath.startsWith(this._pushPath); - added = added.filter(inScope); - modified = modified.filter(inScope); - deleted = deleted.filter(inScope); + const fullCount = fullChanges.added.length + + fullChanges.modified.length + + fullChanges.deleted.length; + const scopeCount = added.length + modified.length + deleted.length; + const scopeIsComplete = fullCount === scopeCount; if (added.length === 0 && modified.length === 0 && deleted.length === 0) { log.info('Nothing to push. No commits ahead of the last da.live sync.'); @@ -261,27 +268,27 @@ export default class PushCommand { return; } - const putTargets = [...added, ...modified]; const { pushed: putPushed, errors: putErrors, - successfullyPushed, - } = await this._uploadFiles(client, org, repo, contentDir, putTargets); + } = await this._uploadFiles(client, org, repo, contentDir, [...added, ...modified]); const { pushed: deletePushed, errors: deleteErrors, - successfullyDeleted, } = await this._deleteFiles(client, org, repo, deleted); const pushed = putPushed + deletePushed; const pushErrors = putErrors + deleteErrors; + const allOk = pushErrors === 0; - const allPutsOk = putTargets.every((p) => successfullyPushed.has(p)); - const allDeletesOk = deleted.every((p) => successfullyDeleted.has(p)); - - if (allPutsOk && allDeletesOk) { + if (allOk && scopeIsComplete) { await writeSyncedRef(fs, contentDir, headOid); + } else if (allOk) { + log.info( + '\n--path filter held back other changes; sync ref not advanced. ' + + 'Run \'aem content push\' (without --path) to fully sync.', + ); } else { log.warn('\nSync ref not updated: fix errors and push again to finish syncing this commit.'); } diff --git a/test/content/da-api.test.js b/test/content/da-api.test.js index 2cdb4af8b..dc7be64e0 100644 --- a/test/content/da-api.test.js +++ b/test/content/da-api.test.js @@ -343,6 +343,20 @@ describe('DaClient', () => { await assert.rejects(() => client.deleteSource('org', 'repo', '/file.html'), /Unauthorized/); }); + it('throws on 5xx so callers cannot treat server failure as success', async () => { + client.fetch = async () => mockResponse(500, 'Server Error', false); + await assert.rejects( + () => client.deleteSource('org', 'repo', '/file.html'), + /DELETE failed/, + ); + }); + + it('treats 404 as success (idempotent)', async () => { + client.fetch = async () => mockResponse(404, 'Not Found', false); + const result = await client.deleteSource('org', 'repo', '/missing.html'); + assert.strictEqual(result, true); + }); + it('calls correct source URL', async () => { let calledUrl; client.fetch = async (url) => { diff --git a/test/content/merge.cmd.test.js b/test/content/merge.cmd.test.js index 6511cef0f..7b28f5a05 100644 --- a/test/content/merge.cmd.test.js +++ b/test/content/merge.cmd.test.js @@ -201,5 +201,31 @@ describe('MergeCommand', () => { const cmd = new Cmd(makeLogger()).withDirectory(testRoot).withFilePath('index.html'); await cmd.run(); }); + + it('passes the project root (not content/) to getValidToken so the cached login is reused', async () => { + const contentDir = await setupContentDir(testRoot); + await fse.writeFile( + path.join(contentDir, 'index.html'), + '

index

local extra

', + ); + + let capturedProjectDir; + const mod = await esmock('../../src/content/merge.cmd.js', { + '../../src/content/da-auth.js': { + getValidToken: async (_log, _override, projectDir) => { + capturedProjectDir = projectDir; + return 'token'; + }, + }, + '../../src/content/da-api.js': { + DaClient: createDaClientClass({ sourceContent: '

remote

' }), + }, + }); + const Cmd = mod.default; + const cmd = new Cmd(makeLogger()).withDirectory(testRoot); + await cmd.run(); + + assert.strictEqual(capturedProjectDir, testRoot); + }); }); }); diff --git a/test/content/push.cmd.test.js b/test/content/push.cmd.test.js index 0e1448f0f..924c1da53 100644 --- a/test/content/push.cmd.test.js +++ b/test/content/push.cmd.test.js @@ -12,10 +12,13 @@ /* eslint-env mocha */ import assert from 'assert'; +import fs from 'fs'; import path from 'path'; import fse from 'fs-extra'; +import git from 'isomorphic-git'; import esmock from 'esmock'; import { createTestRoot } from '../utils.js'; +import { DA_SYNCED_REF } from '../../src/content/content-git.js'; import { makeLogger, setupContentDir, @@ -249,6 +252,118 @@ describe('PushCommand', () => { assert.ok(!pushed.includes('/index.html')); }); + it('--path /blog does not match /blog2/* (no prefix bleed)', async () => { + const contentDir = await setupContentDir(testRoot); + await fse.ensureDir(path.join(contentDir, 'blog2')); + await fse.writeFile(path.join(contentDir, 'blog', 'post.html'), 'changed blog'); + await fse.writeFile(path.join(contentDir, 'blog2', 'post.html'), 'new blog2'); + await stageAllAndCommit(contentDir, 'edit blog and add blog2'); + + const pushed = []; + const DaClientClass = createDaClientClass({ onPut: (p) => pushed.push(p) }); + const log = makeLogger(); + const mod = await esmock('../../src/content/push.cmd.js', { + '../../src/content/da-auth.js': { getValidToken: async () => 'token' }, + '../../src/content/da-api.js': { + DaClient: DaClientClass, + getContentType: () => 'text/html', + }, + }); + const Cmd = mod.default; + const cmd = new Cmd(log).withDirectory(testRoot).withPath('/blog'); + await cmd.run(); + + assert.ok(pushed.includes('/blog/post.html')); + assert.ok(!pushed.includes('/blog2/post.html')); + }); + + it('does not advance synced ref when --path filter excludes other ahead changes', async () => { + const contentDir = await setupContentDir(testRoot); + await fse.writeFile(path.join(contentDir, 'index.html'), 'changed'); + await fse.writeFile(path.join(contentDir, 'blog', 'post.html'), 'changed blog'); + await stageAllAndCommit(contentDir, 'edit pages'); + + const pushed = []; + const DaClientClass = createDaClientClass({ onPut: (p) => pushed.push(p) }); + const log = makeLogger(); + const mod = await esmock('../../src/content/push.cmd.js', { + '../../src/content/da-auth.js': { getValidToken: async () => 'token' }, + '../../src/content/da-api.js': { + DaClient: DaClientClass, + getContentType: () => 'text/html', + }, + }); + const Cmd = mod.default; + const cmd = new Cmd(log).withDirectory(testRoot).withPath('/blog'); + await cmd.run(); + + // The in-scope file must still be uploaded successfully… + assert.ok(pushed.includes('/blog/post.html')); + assert.ok(!pushed.includes('/index.html')); + // …but the synced ref must not advance to HEAD, otherwise the unfiltered + // /index.html change would be silently considered already-synced next time. + const headOid = await git.resolveRef({ fs, dir: contentDir, ref: 'HEAD' }); + let syncedOid; + try { + syncedOid = await git.resolveRef({ fs, dir: contentDir, ref: DA_SYNCED_REF }); + } catch { + syncedOid = null; + } + assert.notStrictEqual(syncedOid, headOid); + assert.ok(log.logs.some((l) => l.msg.includes('held back') || l.msg.includes('without --path'))); + }); + + it('advances synced ref to HEAD when --path covers all ahead changes', async () => { + const contentDir = await setupContentDir(testRoot); + await fse.writeFile(path.join(contentDir, 'blog', 'post.html'), 'changed blog only'); + await stageAllAndCommit(contentDir, 'edit blog'); + + const mod = await esmock('../../src/content/push.cmd.js', { + '../../src/content/da-auth.js': { getValidToken: async () => 'token' }, + '../../src/content/da-api.js': { + DaClient: createDaClientClass(), + getContentType: () => 'text/html', + }, + }); + const Cmd = mod.default; + const cmd = new Cmd(makeLogger()).withDirectory(testRoot).withPath('/blog'); + await cmd.run(); + + const headOid = await git.resolveRef({ fs, dir: contentDir, ref: 'HEAD' }); + const syncedOid = await git.resolveRef({ fs, dir: contentDir, ref: DA_SYNCED_REF }); + assert.strictEqual(syncedOid, headOid); + }); + + it('does not advance synced ref when a delete fails on the server', async () => { + const contentDir = await setupContentDir(testRoot); + await fse.remove(path.join(contentDir, 'index.html')); + await stageAllAndCommit(contentDir, 'drop index'); + + const DaClientClass = createDaClientClass({ deleteFails: true }); + const log = makeLogger(); + const mod = await esmock('../../src/content/push.cmd.js', { + '../../src/content/da-auth.js': { getValidToken: async () => 'token' }, + '../../src/content/da-api.js': { + DaClient: DaClientClass, + getContentType: () => 'text/html', + }, + }); + const Cmd = mod.default; + const cmd = new Cmd(log).withDirectory(testRoot); + await cmd.run(); + + const headOid = await git.resolveRef({ fs, dir: contentDir, ref: 'HEAD' }); + let syncedOid; + try { + syncedOid = await git.resolveRef({ fs, dir: contentDir, ref: DA_SYNCED_REF }); + } catch { + syncedOid = null; + } + assert.notStrictEqual(syncedOid, headOid); + assert.ok(log.logs.some((l) => l.msg.includes('Sync ref not updated'))); + assert.ok(log.logs.some((l) => l.msg.includes('error') || l.msg.includes('✗'))); + }); + it('shows dry-run added/modified/deleted sections', async () => { const contentDir = await setupContentDir(testRoot); await fse.writeFile(path.join(contentDir, 'index.html'), 'changed');