Skip to content

Commit 7130ad5

Browse files
committed
refactor(project): Detect file replacement via inode in cache check
Extend matchResourceMetadataStrict to compare the resource inode against the cached inode. When both sides expose an inode and they disagree, the file has been replaced (atomic rename, cp, tar extraction, ...) — the mtime+size short-circuit is suppressed and the comparison falls through to the integrity check. Inode is now always populated by createResourceIndex; the optional includeInode flag is removed. When either side has no inode (virtual resources, caches written before this change), the inode check is skipped and behavior matches the previous mtime+size path. Falling through to integrity rather than returning false avoids unnecessary task re-execution when the replacement preserves content (e.g. an idempotent rsync). When the replacement also alters content, the integrity check correctly classifies the resource as changed, closing a stale-cache blind spot that mtime+size alone could not detect.
1 parent b145f55 commit 7130ad5

2 files changed

Lines changed: 63 additions & 32 deletions

File tree

packages/project/lib/build/cache/utils.js

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const perfCounters = {
1111
calls: 0,
1212
shortCircuitTrue: 0,
1313
sizeMismatch: 0,
14+
inodeMismatch: 0,
1415
integrityFallback: 0,
1516
};
1617
export {perfCounters as matchResourceMetadataStrictCounters};
@@ -82,14 +83,7 @@ export async function matchResourceMetadataStrict(resource, cachedMetadata, inde
8283
}
8384
if (PERF_TRACKING) perfCounters.calls++;
8485

85-
// Check 1: Inode mismatch would indicate file replacement (comparison only if inodes are provided)
86-
// const currentInode = resource.getInode();
87-
// if (cachedMetadata.inode !== undefined && currentInode !== undefined &&
88-
// currentInode !== cachedMetadata.inode) {
89-
// return false;
90-
// }
91-
92-
// Check 2: Size mismatch indicates definite content change. Required before any
86+
// Size mismatch indicates a definite content change. Required before any
9387
// "unchanged" decision because mtime preservation (cp -p, tar -x, rsync -t,
9488
// atomic rename) does not imply content unchanged.
9589
const currentSize = await resource.getSize();
@@ -98,21 +92,30 @@ export async function matchResourceMetadataStrict(resource, cachedMetadata, inde
9892
return false;
9993
}
10094

101-
// Check 3: Modification time + size both match → unchanged, unless we're in
102-
// the racy-git window (mtime === indexTimestamp), where content may have
103-
// changed during indexing without mtime moving.
95+
// Inode mismatch indicates the file was replaced (atomic rename, cp, tar
96+
// extraction, ...). Content may still be identical, so fall through to the
97+
// integrity check rather than returning false. Inode is optional on both
98+
// sides (e.g. virtual resources, older caches without inode); skip the
99+
// check when either is undefined.
100+
const currentInode = resource.getInode();
101+
const inodeMismatch =
102+
cachedMetadata.inode !== undefined && currentInode !== undefined &&
103+
currentInode !== cachedMetadata.inode;
104+
if (inodeMismatch && PERF_TRACKING) perfCounters.inodeMismatch++;
105+
106+
// mtime + size both match and the file has not been replaced → unchanged,
107+
// unless we are in the racy-git window (mtime === indexTimestamp), where
108+
// content may have changed during indexing without mtime moving.
104109
const currentLastModified = resource.getLastModified();
105-
if (currentLastModified === cachedMetadata.lastModified) {
110+
if (!inodeMismatch && currentLastModified === cachedMetadata.lastModified) {
106111
if (indexTimestamp && currentLastModified !== indexTimestamp) {
107112
if (PERF_TRACKING) perfCounters.shortCircuitTrue++;
108113
return true;
109-
} // else: Edge case. File modified exactly at index time
110-
// Race condition possible - content may have changed during indexing
111-
// Fall through to integrity check
114+
}
115+
// Race condition possible — fall through to integrity check
112116
}
113117

114-
// Check 4: Compare integrity (expensive)
115-
// lastModified has changed, but the content might be the same. E.g. in case of a metadata-only update
118+
// mtime differs, file was replaced, or racy window — verify content.
116119
if (PERF_TRACKING) perfCounters.integrityFallback++;
117120
const currentIntegrity = await resource.getIntegrity();
118121
return currentIntegrity === cachedMetadata.integrity;
@@ -123,27 +126,23 @@ export async function matchResourceMetadataStrict(resource, cachedMetadata, inde
123126
* Creates an index of resource metadata from an array of resources
124127
*
125128
* Processes all resources in parallel, extracting their metadata including
126-
* path, integrity, lastModified timestamp, and size. Optionally includes inode information.
129+
* path, integrity, lastModified timestamp, size and inode (when available).
127130
*
128131
* @public
129132
* @param {Array<@ui5/fs/Resource>} resources Array of resources to index
130-
* @param {boolean} [includeInode=false] Whether to include inode information in the metadata
131133
* @returns {Promise<Array<@ui5/project/build/cache/index/HashTree~ResourceMetadata>>}
132134
* Array of resource metadata objects
133135
*/
134-
export async function createResourceIndex(resources, includeInode = false) {
136+
export async function createResourceIndex(resources) {
135137
return await Promise.all(resources.map(async (resource) => {
136-
const resourceMetadata = {
138+
return {
137139
path: resource.getOriginalPath(),
138140
integrity: await resource.getIntegrity(),
139141
lastModified: resource.getLastModified(),
140142
size: await resource.getSize(),
143+
inode: resource.getInode(),
141144
tags: resource.getTags(),
142145
};
143-
if (includeInode) {
144-
resourceMetadata.inode = resource.getInode();
145-
}
146-
return resourceMetadata;
147146
}));
148147
}
149148

packages/project/test/lib/build/cache/utils.js

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -158,12 +158,44 @@ test("matchResourceMetadataStrict: no indexTimestamp - falls through on matching
158158
t.true(await matchResourceMetadataStrict(resource, metadata));
159159
});
160160

161+
// An atomic rename with preserved mtime (cp -p, mv tmp, ...) yields a new inode
162+
// without changing size or lastModified. The size+mtime cheap path would
163+
// otherwise short-circuit and serve a stale cache hit; force the integrity
164+
// check whenever the inode disagrees.
165+
test("matchResourceMetadataStrict: inode mismatch forces integrity check (content unchanged)",
166+
async (t) => {
167+
const resource = createMockResource({lastModified: 1000, size: 100, inode: 2, integrity: "hash-a"});
168+
const metadata = {lastModified: 1000, size: 100, inode: 1, integrity: "hash-a"};
169+
t.true(await matchResourceMetadataStrict(resource, metadata, 2000));
170+
});
171+
172+
test("matchResourceMetadataStrict: inode mismatch detects same-size content change",
173+
async (t) => {
174+
const resource = createMockResource({lastModified: 1000, size: 100, inode: 2, integrity: "hash-different"});
175+
const metadata = {lastModified: 1000, size: 100, inode: 1, integrity: "hash-a"};
176+
t.false(await matchResourceMetadataStrict(resource, metadata, 2000));
177+
});
178+
179+
test("matchResourceMetadataStrict: skips inode check when cached metadata has no inode",
180+
async (t) => {
181+
const resource = createMockResource({lastModified: 1000, size: 100, inode: 7, integrity: "hash-a"});
182+
const metadata = {lastModified: 1000, size: 100, integrity: "hash-a"};
183+
t.true(await matchResourceMetadataStrict(resource, metadata, 2000));
184+
});
185+
186+
test("matchResourceMetadataStrict: skips inode check when resource has no inode",
187+
async (t) => {
188+
const resource = createMockResource({lastModified: 1000, size: 100, inode: undefined, integrity: "hash-a"});
189+
const metadata = {lastModified: 1000, size: 100, inode: 5, integrity: "hash-a"};
190+
t.true(await matchResourceMetadataStrict(resource, metadata, 2000));
191+
});
192+
161193
// === createResourceIndex ===
162194

163195
test("createResourceIndex: indexes resources correctly", async (t) => {
164196
const resources = [
165-
createMockResource({path: "/a.js", integrity: "hash-a", lastModified: 1000, size: 50}),
166-
createMockResource({path: "/b.js", integrity: "hash-b", lastModified: 2000, size: 60}),
197+
createMockResource({path: "/a.js", integrity: "hash-a", lastModified: 1000, size: 50, inode: 7}),
198+
createMockResource({path: "/b.js", integrity: "hash-b", lastModified: 2000, size: 60, inode: 8}),
167199
];
168200

169201
const result = await createResourceIndex(resources);
@@ -173,17 +205,17 @@ test("createResourceIndex: indexes resources correctly", async (t) => {
173205
t.is(result[0].integrity, "hash-a");
174206
t.is(result[0].lastModified, 1000);
175207
t.is(result[0].size, 50);
176-
t.is(result[0].inode, undefined);
208+
t.is(result[0].inode, 7);
177209
});
178210

179-
test("createResourceIndex: includes inode when requested", async (t) => {
211+
test("createResourceIndex: leaves inode undefined when resource has none", async (t) => {
180212
const resources = [
181-
createMockResource({path: "/a.js", inode: 42}),
213+
createMockResource({path: "/a.js", inode: undefined}),
182214
];
183215

184-
const result = await createResourceIndex(resources, true);
216+
const result = await createResourceIndex(resources);
185217

186-
t.is(result[0].inode, 42);
218+
t.is(result[0].inode, undefined);
187219
});
188220

189221
test("createResourceIndex: includes tags", async (t) => {

0 commit comments

Comments
 (0)