From 8a25d33478a61d919e4565b10dc13dcbedd9f3b5 Mon Sep 17 00:00:00 2001
From: Sergei Zharinov <zharinov@users.noreply.github.com>
Date: Tue, 19 Sep 2023 07:40:05 +0300
Subject: [PATCH] fix(github): Fix update detection for GraphQL incremental
cache (#24503)
---
.../abstract-cache-strategy.ts | 31 ++++++++++--------
.../memory-cache-strategy.spec.ts | 13 +++-----
.../package-cache-strategy.spec.ts | 10 ++----
.../package-cache-strategy.ts | 32 +++++++++----------
4 files changed, 41 insertions(+), 45 deletions(-)
diff --git a/lib/util/github/graphql/cache-strategies/abstract-cache-strategy.ts b/lib/util/github/graphql/cache-strategies/abstract-cache-strategy.ts
index ed23730c88..98ddffcd3f 100644
--- a/lib/util/github/graphql/cache-strategies/abstract-cache-strategy.ts
+++ b/lib/util/github/graphql/cache-strategies/abstract-cache-strategy.ts
@@ -38,10 +38,9 @@ export abstract class AbstractGithubGraphqlCacheStrategy<
protected createdAt = this.now;
/**
- * This flag helps to indicate whether the cache record
- * should be persisted after the current cache access cycle.
+ * This flag indicates whether there is any new or updated items
*/
- protected hasUpdatedItems = false;
+ protected hasNovelty = false;
/**
* Loading and persisting data is delegated to the concrete strategy.
@@ -107,18 +106,18 @@ export abstract class AbstractGithubGraphqlCacheStrategy<
let isPaginationDone = false;
for (const item of items) {
const { version } = item;
+ const oldItem = cachedItems[version];
// If we reached previously stored item that is stabilized,
// we assume the further pagination will not yield any new items.
- const oldItem = cachedItems[version];
- if (oldItem) {
- if (this.isStabilized(oldItem)) {
- isPaginationDone = true;
- }
-
- if (!dequal(oldItem, item)) {
- this.hasUpdatedItems = true;
- }
+ if (oldItem && this.isStabilized(oldItem)) {
+ isPaginationDone = true;
+ break;
+ }
+
+ // Check if item is new or updated
+ if (!oldItem || !dequal(oldItem, item)) {
+ this.hasNovelty = true;
}
cachedItems[version] = item;
@@ -137,13 +136,19 @@ export abstract class AbstractGithubGraphqlCacheStrategy<
const cachedItems = await this.getItems();
const resultItems: Record<string, GithubItem> = {};
+ let hasDeletedItems = false;
for (const [version, item] of Object.entries(cachedItems)) {
if (this.isStabilized(item) || this.reconciledVersions.has(version)) {
resultItems[version] = item;
+ } else {
+ hasDeletedItems = true;
}
}
- await this.store(resultItems);
+ if (this.hasNovelty || hasDeletedItems) {
+ await this.store(resultItems);
+ }
+
return Object.values(resultItems);
}
diff --git a/lib/util/github/graphql/cache-strategies/memory-cache-strategy.spec.ts b/lib/util/github/graphql/cache-strategies/memory-cache-strategy.spec.ts
index 13bd58bceb..4dcf3d1ac7 100644
--- a/lib/util/github/graphql/cache-strategies/memory-cache-strategy.spec.ts
+++ b/lib/util/github/graphql/cache-strategies/memory-cache-strategy.spec.ts
@@ -68,10 +68,7 @@ describe('util/github/graphql/cache-strategies/memory-cache-strategy', () => {
expect(res).toEqual([]);
expect(isPaginationDone).toBe(false);
- expect(memCache.get('github-graphql-cache:foo:bar')).toEqual({
- items: {},
- createdAt: isoTs(now),
- });
+ expect(memCache.get('github-graphql-cache:foo:bar')).toEqual(cacheRecord);
});
it('reconciles old cache record with new items', async () => {
@@ -136,7 +133,7 @@ describe('util/github/graphql/cache-strategies/memory-cache-strategy', () => {
expect(isPaginationDone).toBe(true);
});
- it('reconciles entire page', async () => {
+ it('reconciles only not stabilized items in page', async () => {
const oldItems = {
'1': { releaseTimestamp: isoTs('2020-01-01 00:00'), version: '1' },
'2': { releaseTimestamp: isoTs('2020-01-01 01:00'), version: '2' },
@@ -164,9 +161,9 @@ describe('util/github/graphql/cache-strategies/memory-cache-strategy', () => {
expect(isPaginationDone).toBe(true);
expect(memCache.get('github-graphql-cache:foo:bar')).toMatchObject({
items: {
- '1': { releaseTimestamp: isoTs('2022-12-31 10:00') },
- '2': { releaseTimestamp: isoTs('2022-12-31 11:00') },
- '3': { releaseTimestamp: isoTs('2022-12-31 12:00') },
+ '1': { releaseTimestamp: isoTs('2020-01-01 00:00') },
+ '2': { releaseTimestamp: isoTs('2020-01-01 01:00') },
+ '3': { releaseTimestamp: isoTs('2020-01-01 02:00') },
'4': { releaseTimestamp: isoTs('2022-12-31 13:00') },
},
});
diff --git a/lib/util/github/graphql/cache-strategies/package-cache-strategy.spec.ts b/lib/util/github/graphql/cache-strategies/package-cache-strategy.spec.ts
index 80c02add16..b07e7cc762 100644
--- a/lib/util/github/graphql/cache-strategies/package-cache-strategy.spec.ts
+++ b/lib/util/github/graphql/cache-strategies/package-cache-strategy.spec.ts
@@ -36,21 +36,17 @@ describe('util/github/graphql/cache-strategies/package-cache-strategy', () => {
const now = '2022-10-30 12:00';
mockTime(now);
- const updatedItem = {
- ...item3,
- releaseTimestamp: isoTs('2020-01-01 12:30'),
- };
const newItem = {
version: '4',
releaseTimestamp: isoTs('2022-10-15 18:00'),
};
- const page = [newItem, updatedItem];
+ const page = [newItem, item3, item2, item1];
const strategy = new GithubGraphqlPackageCacheStrategy('foo', 'bar');
const isPaginationDone = await strategy.reconcile(page);
const res = await strategy.finalize();
- expect(res).toEqual([item1, item2, updatedItem, newItem]);
+ expect(res).toEqual([item1, item2, item3, newItem]);
expect(isPaginationDone).toBe(true);
expect(cacheSet.mock.calls).toEqual([
[
@@ -60,7 +56,7 @@ describe('util/github/graphql/cache-strategies/package-cache-strategy', () => {
items: {
'1': item1,
'2': item2,
- '3': updatedItem,
+ '3': item3,
'4': newItem,
},
createdAt: isoTs('2022-10-15 12:00'),
diff --git a/lib/util/github/graphql/cache-strategies/package-cache-strategy.ts b/lib/util/github/graphql/cache-strategies/package-cache-strategy.ts
index 8e9f6f25e9..f503207ca0 100644
--- a/lib/util/github/graphql/cache-strategies/package-cache-strategy.ts
+++ b/lib/util/github/graphql/cache-strategies/package-cache-strategy.ts
@@ -15,23 +15,21 @@ export class GithubGraphqlPackageCacheStrategy<
async persist(
cacheRecord: GithubGraphqlCacheRecord<GithubItem>
): Promise<void> {
- if (this.hasUpdatedItems) {
- const expiry = this.createdAt
- .plus({
- // Not using 'days' as it does not handle adjustments for Daylight Saving time.
- // The offset in the resulting DateTime object does not match that of the expiry or this.now.
- hours: AbstractGithubGraphqlCacheStrategy.cacheTTLDays * 24,
- })
- .toUTC();
- const ttlMinutes = expiry.diff(this.now, ['minutes']).as('minutes');
- if (ttlMinutes && ttlMinutes > 0) {
- await packageCache.set(
- this.cacheNs,
- this.cacheKey,
- cacheRecord,
- ttlMinutes
- );
- }
+ const expiry = this.createdAt
+ .plus({
+ // Not using 'days' as it does not handle adjustments for Daylight Saving time.
+ // The offset in the resulting DateTime object does not match that of the expiry or this.now.
+ hours: AbstractGithubGraphqlCacheStrategy.cacheTTLDays * 24,
+ })
+ .toUTC();
+ const ttlMinutes = expiry.diff(this.now, ['minutes']).as('minutes');
+ if (ttlMinutes && ttlMinutes > 0) {
+ await packageCache.set(
+ this.cacheNs,
+ this.cacheKey,
+ cacheRecord,
+ ttlMinutes
+ );
}
}
}
--
GitLab