Skip to content
Draft
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
67 changes: 67 additions & 0 deletions webui/src/lib/components/repository/mergeResults.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,73 @@ describe('mergeResults', () => {
});
});

describe('added entries with committed-only listing (branch@)', () => {
it('includes added entries when committed results are empty (fresh repo upload)', () => {
const results: Entry[] = [];
const changesData: ChangesData = {
results: [{ path: 'uploaded.txt', type: 'added', path_type: 'object' }],
};

const merged = mergeResults(results, changesData, false);

expect(merged).toEqual([
{ path: 'uploaded.txt', path_type: 'object', type: 'added', diff_type: 'added' },
]);
});

it('includes added entries that sort after last committed object on last page', () => {
const results: Entry[] = [
{ path: 'a.txt', path_type: 'object' },
{ path: 'lakes.parquet', path_type: 'object' },
];
const changesData: ChangesData = {
results: [{ path: 'test-upload.txt', type: 'added', path_type: 'object' }],
};

// hasMore=false (last page)
const merged = mergeResults(results, changesData, false, false);

expect(merged).toHaveLength(3);
expect(merged.find((r) => r.path === 'test-upload.txt')).toMatchObject({
path: 'test-upload.txt',
diff_type: 'added',
});
});

it('excludes added entries beyond page range when there are more pages', () => {
const results: Entry[] = [
{ path: 'a.txt', path_type: 'object' },
{ path: 'b.txt', path_type: 'object' },
];
const changesData: ChangesData = {
results: [{ path: 'z.txt', type: 'added', path_type: 'object' }],
};

// hasMore=true (not last page)
const merged = mergeResults(results, changesData, false, true);

expect(merged).toHaveLength(2);
expect(merged.find((r) => r.path === 'z.txt')).toBeUndefined();
});

it('includes added entries within page range regardless of hasMore', () => {
const results: Entry[] = [
{ path: 'a.txt', path_type: 'object' },
{ path: 'z.txt', path_type: 'object' },
];
const changesData: ChangesData = {
results: [{ path: 'new-file.txt', type: 'added', path_type: 'object' }],
};

const merged = mergeResults(results, changesData, false, true);

expect(merged).toHaveLength(3);
expect(merged.find((r) => r.path === 'new-file.txt')).toMatchObject({
diff_type: 'added',
});
});
});

describe('sorting', () => {
it('sorts merged results lexicographically', () => {
const results: Entry[] = [
Expand Down
23 changes: 19 additions & 4 deletions webui/src/lib/components/repository/mergeResults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ import { compareLexicographically } from '../../utils';
* @param results - Array of object entries from the main listing
* @param changesData - Changes data containing results array with change information
* @param showChangesOnly - Whether to show only changes (if true, no merging needed)
* @param hasMore - Whether there are more pages of results after the current one
* @returns Merged and sorted results with diff_type annotations
*/
export function mergeResults(
results: Entry[] | undefined | null,
changesData: ChangesData | undefined | null,
showChangesOnly = false,
hasMore = false,
): EntryWithDiff[] {
if (showChangesOnly || !results || !changesData?.results) {
// Ensure regular results are also sorted lexicographically
Expand Down Expand Up @@ -46,12 +48,25 @@ export function mergeResults(
}
});

// Add missing items only for removed entries or deleted prefixes
// Avoid adding items that come after the last result path (both are sorted lexicographically)
// Add missing items for removed entries, added entries, and prefixes
// When using committed-only ref (branch@) for objects.list, added entries are not in the list
// On paginated results, only add items within the current page range to avoid duplicates
const lastResultPath = last(results)?.path;
const inPageRange = (path: string) => lastResultPath && path <= lastResultPath;
const missingItems = changesData.results
.filter((change) => change.type === 'removed' || change.path_type === 'common_prefix')
.filter((change) => lastResultPath && change.path <= lastResultPath)
.filter(
(change) => change.type === 'removed' || change.type === 'added' || change.path_type === 'common_prefix',
)
.filter((change) => {
if (change.type === 'added') {
// Added entries never appear in committed results (branch@),
// so include them on the last page even if beyond lastResultPath
return inPageRange(change.path) || !hasMore;
}
// Removed/changed entries exist in committed results on some page,
// only show within the current page range
return inPageRange(change.path);
})
.filter((change) => !results.find((result) => result.path === change.path));

// Merge regular results with change info
Expand Down
22 changes: 18 additions & 4 deletions webui/src/pages/repositories/repository/objects.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -993,16 +993,30 @@ const TreeContainer = ({
);
} else {
// Show all objects
return objects.list(repo.id, reference.id, path, after, config.pre_sign_support_ui);
// Use committed-only ref (branch@) for branches to avoid scanning staging area twice
// This significantly improves performance when there are many uncommitted deletes
const listRef = reference.type === RefTypeBranch ? reference.id + '@' : reference.id;
return objects.list(repo.id, listRef, path, after, config.pre_sign_support_ui);
}
// TODO: Review and remove this eslint-disable once dependencies are validated
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [repo.id, reference.id, path, after, refreshToken, showChangesOnly, internalRefresh, lastSeenPath, delimiter]);
}, [
repo.id,
reference.id,
reference.type,
path,
after,
refreshToken,
showChangesOnly,
internalRefresh,
lastSeenPath,
delimiter,
]);

// Merge changes with objects for highlighting
const mergedResults = React.useMemo(
() => mergeResults(results, changesData, showChangesOnly),
[results, changesData, showChangesOnly],
() => mergeResults(results, changesData, showChangesOnly, !!nextPage),
[results, changesData, showChangesOnly, nextPage],
);

const initialState = {
Expand Down
Loading