From 10fd0f28278bc464a2385f13ecebea70ee0f3df4 Mon Sep 17 00:00:00 2001
From: Michael Kriese <michael.kriese@visualon.de>
Date: Fri, 26 Aug 2022 14:25:35 +0200
Subject: [PATCH] fix(cache/repository): don't log error for non-existant files
(#17427)
---
lib/util/cache/repository/impl/local.spec.ts | 16 ++++++++++++++++
lib/util/cache/repository/impl/local.ts | 6 +++++-
lib/util/fs/index.spec.ts | 9 +++++++++
lib/util/fs/index.ts | 10 ++++++++++
4 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/lib/util/cache/repository/impl/local.spec.ts b/lib/util/cache/repository/impl/local.spec.ts
index 91c23d6194..d0702ccd79 100644
--- a/lib/util/cache/repository/impl/local.spec.ts
+++ b/lib/util/cache/repository/impl/local.spec.ts
@@ -37,15 +37,23 @@ describe('util/cache/repository/impl/local', () => {
it('skip when receives non-string data', async () => {
const localRepoCache = CacheFactory.get('some/repo', 'local');
+ fs.cachePathExists.mockResolvedValueOnce(true);
await localRepoCache.load(); // readCacheFile is mocked but has no return value set - therefore returns undefined
expect(logger.debug).toHaveBeenCalledWith(
"RepoCacheBase.load() - expecting data of type 'string' received 'undefined' instead - skipping"
);
});
+ it('skip when not found', async () => {
+ const localRepoCache = CacheFactory.get('some/repo', 'local');
+ await localRepoCache.load(); // readCacheFile is mocked but has no return value set - therefore returns undefined
+ expect(logger.debug).not.toHaveBeenCalledWith();
+ });
+
it('loads previously stored cache from disk', async () => {
const data: RepoCacheData = { semanticCommits: 'enabled' };
const cacheRecord = await createCacheRecord(data);
+ fs.cachePathExists.mockResolvedValueOnce(true);
fs.readCacheFile.mockResolvedValue(JSON.stringify(cacheRecord));
const localRepoCache = CacheFactory.get('some/repo', 'local');
@@ -55,6 +63,7 @@ describe('util/cache/repository/impl/local', () => {
});
it('migrates revision from 10 to 12', async () => {
+ fs.cachePathExists.mockResolvedValueOnce(true);
fs.readCacheFile.mockResolvedValue(
JSON.stringify({
revision: 10,
@@ -75,6 +84,7 @@ describe('util/cache/repository/impl/local', () => {
});
it('migrates revision from 11 to 12', async () => {
+ fs.cachePathExists.mockResolvedValueOnce(true);
fs.readCacheFile.mockResolvedValue(
JSON.stringify({
revision: 11,
@@ -95,6 +105,7 @@ describe('util/cache/repository/impl/local', () => {
});
it('does not migrate from older revisions to 11', async () => {
+ fs.cachePathExists.mockResolvedValueOnce(true);
fs.readCacheFile.mockResolvedValueOnce(
JSON.stringify({
revision: 9,
@@ -110,6 +121,7 @@ describe('util/cache/repository/impl/local', () => {
});
it('handles invalid data', async () => {
+ fs.cachePathExists.mockResolvedValueOnce(true);
fs.readCacheFile.mockResolvedValue(JSON.stringify({ foo: 'bar' }));
const localRepoCache = CacheFactory.get('some/repo', 'local');
@@ -119,6 +131,7 @@ describe('util/cache/repository/impl/local', () => {
});
it('handles file read error', async () => {
+ fs.cachePathExists.mockResolvedValueOnce(true);
fs.readCacheFile.mockRejectedValue(new Error('unknown error'));
const localRepoCache = CacheFactory.get('some/repo', 'local');
@@ -129,6 +142,7 @@ describe('util/cache/repository/impl/local', () => {
});
it('handles invalid json', async () => {
+ fs.cachePathExists.mockResolvedValueOnce(true);
fs.readCacheFile.mockResolvedValue('{1');
const localRepoCache = CacheFactory.get('some/repo', 'local');
@@ -139,6 +153,7 @@ describe('util/cache/repository/impl/local', () => {
it('resets if repository does not match', async () => {
const cacheRecord = createCacheRecord({ semanticCommits: 'enabled' });
+ fs.cachePathExists.mockResolvedValueOnce(true);
fs.readCacheFile.mockResolvedValueOnce(JSON.stringify(cacheRecord));
const localRepoCache = CacheFactory.get('some/repo', 'local');
@@ -150,6 +165,7 @@ describe('util/cache/repository/impl/local', () => {
it('saves modified cache data to file', async () => {
const oldCacheRecord = createCacheRecord({ semanticCommits: 'enabled' });
const cacheType = 'protocol://domain/path';
+ fs.cachePathExists.mockResolvedValueOnce(true);
fs.readCacheFile.mockResolvedValueOnce(JSON.stringify(oldCacheRecord));
const localRepoCache = CacheFactory.get('some/repo', cacheType);
await localRepoCache.load();
diff --git a/lib/util/cache/repository/impl/local.ts b/lib/util/cache/repository/impl/local.ts
index c01f95058e..14e5fcb11e 100644
--- a/lib/util/cache/repository/impl/local.ts
+++ b/lib/util/cache/repository/impl/local.ts
@@ -1,7 +1,7 @@
import upath from 'upath';
import { GlobalConfig } from '../../../../config/global';
import { logger } from '../../../../logger';
-import { outputCacheFile, readCacheFile } from '../../../fs';
+import { cachePathExists, outputCacheFile, readCacheFile } from '../../../fs';
import type { RepoCacheRecord } from '../types';
import { RepoCacheBase } from './base';
@@ -13,6 +13,10 @@ export class RepoCacheLocal extends RepoCacheBase {
protected async read(): Promise<string | null> {
const cacheFileName = this.getCacheFileName();
try {
+ // suppress debug logs with errros
+ if (!(await cachePathExists(cacheFileName))) {
+ return null;
+ }
return await readCacheFile(cacheFileName, 'utf8');
} catch (err) {
logger.debug({ err, cacheFileName }, 'Repository local cache not found');
diff --git a/lib/util/fs/index.spec.ts b/lib/util/fs/index.spec.ts
index 6a257f0127..3670a58bdc 100644
--- a/lib/util/fs/index.spec.ts
+++ b/lib/util/fs/index.spec.ts
@@ -5,6 +5,7 @@ import { join, resolve } from 'upath';
import { mockedFunction } from '../../../test/util';
import { GlobalConfig } from '../../config/global';
import {
+ cachePathExists,
chmodLocalFile,
createCacheWriteStream,
deleteLocalFile,
@@ -426,6 +427,14 @@ describe('util/fs/index', () => {
});
});
+ describe('cachePathExists', () => {
+ it('reads file', async () => {
+ await fs.outputFile(`${cacheDir}/foo/bar/file.txt`, 'foobar');
+ expect(await cachePathExists(`foo/bar/file.txt1`)).toBeFalse();
+ expect(await cachePathExists(`foo/bar/file.txt`)).toBeTrue();
+ });
+ });
+
describe('readCacheFile', () => {
it('reads file', async () => {
await fs.outputFile(`${cacheDir}/foo/bar/file.txt`, 'foobar');
diff --git a/lib/util/fs/index.ts b/lib/util/fs/index.ts
index 6f65fecc80..b029c41efe 100644
--- a/lib/util/fs/index.ts
+++ b/lib/util/fs/index.ts
@@ -246,6 +246,16 @@ export async function rmCache(path: string): Promise<void> {
await fs.rm(fullPath, { recursive: true });
}
+export async function cachePathExists(pathName: string): Promise<boolean> {
+ const path = ensureCachePath(pathName);
+ try {
+ const s = await fs.stat(path);
+ return !!s;
+ } catch (_) {
+ return false;
+ }
+}
+
export async function readCacheFile(fileName: string): Promise<Buffer>;
export async function readCacheFile(
fileName: string,
--
GitLab