Skip to content

Commit e7bfbb7

Browse files
GH-153 - Fix disappearing empty folders (#154)
* Resolves an issue where empty folders would disappear in some circumstances * Increases test coverage
1 parent 20b1a61 commit e7bfbb7

File tree

3 files changed

+301
-67
lines changed

3 files changed

+301
-67
lines changed

src/storage/utils/list.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ export default function formatList(resp, daCtx) {
5050
// Do not show any props sidecar files
5151
if (props) return;
5252

53-
// See if the folder is already in the list
5453
if (ext === 'props') {
55-
if (combined.some((item) => item.name === name)) return;
54+
// Do not add if it already exists as a folder (does not have an extension)
55+
if (combined.some((item) => item.name === name && !item.ext)) return;
5656

5757
// Remove props from the key so it can look like a folder
5858
// eslint-disable-next-line no-param-reassign

test/storage/utils.t.js

Lines changed: 0 additions & 65 deletions
This file was deleted.

test/storage/utils/list.test.js

Lines changed: 299 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,299 @@
1+
/* eslint-env mocha */
2+
import assert from 'assert';
3+
import sinon from 'sinon';
4+
5+
import getDaCtx from '../../../src/utils/daCtx.js';
6+
import formatList, { listCommand } from '../../../src/storage/utils/list.js';
7+
8+
const MOCK = {
9+
CommonPrefixes: [
10+
{ Prefix: 'da-aem-boilerplate/' },
11+
{ Prefix: 'blog/' },
12+
{ Prefix: 'da/' },
13+
{ Prefix: 'dac/' },
14+
{ Prefix: 'milo/' },
15+
{ Prefix: 'dark-alley.jpg/' },
16+
],
17+
Contents: [
18+
{
19+
Key: 'blog.props',
20+
LastModified: new Date('2025-01-01'),
21+
},
22+
{
23+
Key: 'da.props',
24+
LastModified: new Date('2025-01-01'),
25+
},
26+
{
27+
Key: 'folder-only.props',
28+
LastModified: new Date('2025-01-01'),
29+
},
30+
{
31+
Key: 'test.html',
32+
LastModified: new Date('2025-01-01'),
33+
},
34+
{
35+
Key: 'dark-alley.jpg.props',
36+
LastModified: new Date('2025-01-01'),
37+
},
38+
{
39+
Key: 'dark-alley.jpg',
40+
LastModified: new Date('2025-01-01'),
41+
},
42+
{
43+
Key: 'empty-folder-with-sibling-file.props',
44+
LastModified: new Date('2025-01-01'),
45+
},
46+
{
47+
Key: 'empty-folder-with-sibling-file.html',
48+
LastModified: new Date('2025-01-01'),
49+
}
50+
],
51+
};
52+
53+
const req = new Request('https://example.com/source/adobecom');
54+
55+
const daCtx = getDaCtx(req, {});
56+
57+
describe('Format object list', () => {
58+
const list = formatList(MOCK, daCtx);
59+
60+
it('should return a true folder / common prefix', () => {
61+
assert.strictEqual(list[0].name, 'blog');
62+
});
63+
64+
it('should return a contents-based folder', () => {
65+
const folderOnly = list.find((item) => { return item.name === 'folder-only' });
66+
assert.strictEqual(folderOnly.name, 'folder-only');
67+
});
68+
69+
it('should not return a props file of same folder name', () => {
70+
const found = list.reduce((acc, item) => {
71+
if (item.name === 'blog') acc.push(item);
72+
return acc;
73+
},[]);
74+
75+
assert.strictEqual(found.length, 1);
76+
});
77+
78+
it('should not have a filename props file in the list', () => {
79+
const propsSidecar = list.find((item) => { return item.name === 'dark-alley.jpg.props' });
80+
assert.strictEqual(propsSidecar, undefined);
81+
});
82+
83+
it('should handle empty folders with sibling file names of same name', () => {
84+
const filtered = list.filter((item) => { return item.name === 'empty-folder-with-sibling-file' });
85+
assert.strictEqual(filtered.length, 2);
86+
});
87+
88+
it('should handle empty CommonPrefixes', () => {
89+
const emptyMock = { Contents: MOCK.Contents };
90+
const result = formatList(emptyMock, daCtx);
91+
assert(Array.isArray(result));
92+
assert(result.length > 0);
93+
});
94+
95+
it('should handle empty Contents', () => {
96+
const emptyMock = { CommonPrefixes: MOCK.CommonPrefixes };
97+
const result = formatList(emptyMock, daCtx);
98+
assert(Array.isArray(result));
99+
assert(result.length > 0);
100+
});
101+
102+
it('should handle both empty CommonPrefixes and Contents', () => {
103+
const emptyMock = {};
104+
const result = formatList(emptyMock, daCtx);
105+
assert(Array.isArray(result));
106+
assert.strictEqual(result.length, 0);
107+
});
108+
109+
it('should filter out extension folders from CommonPrefixes', () => {
110+
const mockWithExtensionFolder = {
111+
CommonPrefixes: [
112+
{ Prefix: 'file.jpg/' },
113+
{ Prefix: 'normal-folder/' }
114+
]
115+
};
116+
const result = formatList(mockWithExtensionFolder, daCtx);
117+
const extensionFolder = result.find(item => item.name === 'file.jpg');
118+
assert.strictEqual(extensionFolder, undefined);
119+
const normalFolder = result.find(item => item.name === 'normal-folder');
120+
assert(normalFolder);
121+
});
122+
123+
it('should handle files with more than 2 dot separators', () => {
124+
const mockWithComplexFile = {
125+
Contents: [
126+
{
127+
Key: 'file.name.with.multiple.dots',
128+
LastModified: new Date('2025-01-01'),
129+
}
130+
]
131+
};
132+
const result = formatList(mockWithComplexFile, daCtx);
133+
assert.strictEqual(result.length, 0);
134+
});
135+
136+
it('should handle hidden files (starting with dot)', () => {
137+
const mockWithHiddenFile = {
138+
Contents: [
139+
{
140+
Key: '.hidden-file',
141+
LastModified: new Date('2025-01-01'),
142+
}
143+
]
144+
};
145+
const result = formatList(mockWithHiddenFile, daCtx);
146+
assert.strictEqual(result.length, 0);
147+
});
148+
149+
it('should handle files with props extension correctly', () => {
150+
const mockWithProps = {
151+
Contents: [
152+
{
153+
Key: 'test.props',
154+
LastModified: new Date('2025-01-01'),
155+
}
156+
]
157+
};
158+
const result = formatList(mockWithProps, daCtx);
159+
const propsItem = result.find(item => item.name === 'test');
160+
assert(propsItem);
161+
assert.strictEqual(propsItem.ext, undefined);
162+
assert.strictEqual(propsItem.lastModified, undefined);
163+
});
164+
165+
it('should not add props file if folder already exists', () => {
166+
const mockWithBoth = {
167+
CommonPrefixes: [{ Prefix: 'test/' }],
168+
Contents: [
169+
{
170+
Key: 'test.props',
171+
LastModified: new Date('2025-01-01'),
172+
}
173+
]
174+
};
175+
const result = formatList(mockWithBoth, daCtx);
176+
const testItems = result.filter(item => item.name === 'test');
177+
assert.strictEqual(testItems.length, 1);
178+
});
179+
180+
it('should sort results alphabetically', () => {
181+
const mockForSorting = {
182+
Contents: [
183+
{ Key: 'zebra.html', LastModified: new Date('2025-01-01') },
184+
{ Key: 'alpha.html', LastModified: new Date('2025-01-01') },
185+
{ Key: 'beta.html', LastModified: new Date('2025-01-01') }
186+
]
187+
};
188+
const result = formatList(mockForSorting, daCtx);
189+
assert.strictEqual(result[0].name, 'alpha');
190+
assert.strictEqual(result[1].name, 'beta');
191+
assert.strictEqual(result[2].name, 'zebra');
192+
});
193+
});
194+
195+
describe('listCommand', () => {
196+
let mockS3Client;
197+
let testDaCtx;
198+
199+
beforeEach(() => {
200+
mockS3Client = {
201+
send: sinon.stub()
202+
};
203+
204+
// Create a proper daCtx object for testing
205+
testDaCtx = {
206+
bucket: 'test-bucket',
207+
org: 'adobecom',
208+
key: 'test',
209+
ext: undefined
210+
};
211+
});
212+
213+
afterEach(() => {
214+
sinon.restore();
215+
});
216+
217+
it('should return sourceKeys array when item has extension', async () => {
218+
const daCtxWithExt = { ...testDaCtx, ext: 'html' };
219+
const result = await listCommand(daCtxWithExt, {}, mockS3Client);
220+
221+
assert.deepStrictEqual(result, { sourceKeys: [testDaCtx.key] });
222+
assert.strictEqual(mockS3Client.send.callCount, 0);
223+
});
224+
225+
it('should call S3 list command when no extension', async () => {
226+
const mockResponse = {
227+
Contents: [
228+
{ Key: 'adobecom/test/file1.html' },
229+
{ Key: 'adobecom/test/file2.html' }
230+
],
231+
NextContinuationToken: 'next-token'
232+
};
233+
234+
mockS3Client.send.resolves(mockResponse);
235+
236+
const result = await listCommand(testDaCtx, {}, mockS3Client);
237+
238+
assert.strictEqual(mockS3Client.send.callCount, 1);
239+
assert.deepStrictEqual(result, {
240+
sourceKeys: [testDaCtx.key, `${testDaCtx.key}.props`, 'adobecom/test/file1.html', 'adobecom/test/file2.html'],
241+
continuationToken: 'next-token'
242+
});
243+
});
244+
245+
it('should handle continuation token', async () => {
246+
const mockResponse = {
247+
Contents: [
248+
{ Key: 'adobecom/test/file3.html' }
249+
]
250+
};
251+
252+
mockS3Client.send.resolves(mockResponse);
253+
254+
const details = { continuationToken: 'prev-token' };
255+
const result = await listCommand(testDaCtx, details, mockS3Client);
256+
257+
assert.strictEqual(mockS3Client.send.callCount, 1);
258+
const callArgs = mockS3Client.send.firstCall.args[0];
259+
console.log('Call args:', JSON.stringify(callArgs, null, 2));
260+
// The command should have the continuation token
261+
assert.strictEqual(callArgs.input.ContinuationToken, 'prev-token');
262+
assert.deepStrictEqual(result, {
263+
sourceKeys: ['adobecom/test/file3.html'],
264+
continuationToken: undefined
265+
});
266+
});
267+
268+
it('should handle empty Contents response', async () => {
269+
const mockResponse = {
270+
Contents: []
271+
};
272+
273+
mockS3Client.send.resolves(mockResponse);
274+
275+
const result = await listCommand(testDaCtx, {}, mockS3Client);
276+
277+
assert.deepStrictEqual(result, {
278+
sourceKeys: [testDaCtx.key, `${testDaCtx.key}.props`],
279+
continuationToken: undefined
280+
});
281+
});
282+
283+
it('should handle response without NextContinuationToken', async () => {
284+
const mockResponse = {
285+
Contents: [
286+
{ Key: 'adobecom/test/file1.html' }
287+
]
288+
};
289+
290+
mockS3Client.send.resolves(mockResponse);
291+
292+
const result = await listCommand(testDaCtx, {}, mockS3Client);
293+
294+
assert.deepStrictEqual(result, {
295+
sourceKeys: [testDaCtx.key, `${testDaCtx.key}.props`, 'adobecom/test/file1.html'],
296+
continuationToken: undefined
297+
});
298+
});
299+
});

0 commit comments

Comments
 (0)