From 25824f8ba1a8f3862257c0655797079b204ced23 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 28 Oct 2024 11:31:57 +0000 Subject: [PATCH 1/5] Fix challengehound duplication bug. --- src/Stores/MemoryStorageProvider.ts | 5 +++++ src/Stores/RedisStorageProvider.ts | 2 +- src/Stores/StorageProvider.ts | 1 + src/hound/reader.ts | 30 +++++++++++++++++------------ 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/Stores/MemoryStorageProvider.ts b/src/Stores/MemoryStorageProvider.ts index a6336e594..e2d65be11 100644 --- a/src/Stores/MemoryStorageProvider.ts +++ b/src/Stores/MemoryStorageProvider.ts @@ -122,11 +122,16 @@ export class MemoryStorageProvider extends MSP implements IBridgeStorageProvider set.pop(); } } + async hasSeenHoundActivity(challengeId: string, ...activityIds: string[]): Promise { const existing = this.houndActivityIds.get(challengeId); return existing ? activityIds.filter((existingGuid) => existing.includes(existingGuid)) : []; } + public async hasSeenHoundChallenge(challengeId: string): Promise { + return this.houndActivityIds.has(challengeId); + } + public async storeHoundActivityEvent(challengeId: string, activityId: string, eventId: string): Promise { this.houndActivityIdToEvent.set(`${challengeId}.${activityId}`, eventId); } diff --git a/src/Stores/RedisStorageProvider.ts b/src/Stores/RedisStorageProvider.ts index 657b0b538..4cf1b8d63 100644 --- a/src/Stores/RedisStorageProvider.ts +++ b/src/Stores/RedisStorageProvider.ts @@ -23,7 +23,7 @@ const STORED_FILES_EXPIRE_AFTER = 24 * 60 * 60; // 24 hours const COMPLETED_TRANSACTIONS_EXPIRE_AFTER = 24 * 60 * 60; // 24 hours const ISSUES_EXPIRE_AFTER = 7 * 24 * 60 * 60; // 7 days const ISSUES_LAST_COMMENT_EXPIRE_AFTER = 14 * 24 * 60 * 60; // 7 days -const HOUND_EVENT_CACHE = 90 * 24 * 60 * 60; // 30 days +const HOUND_EVENT_CACHE = 90 * 24 * 60 * 60; // 90 days const WIDGET_TOKENS = "widgets.tokens."; diff --git a/src/Stores/StorageProvider.ts b/src/Stores/StorageProvider.ts index dbff7f3b1..74a0f345a 100644 --- a/src/Stores/StorageProvider.ts +++ b/src/Stores/StorageProvider.ts @@ -32,6 +32,7 @@ export interface IBridgeStorageProvider extends IAppserviceStorageProvider, ISto hasSeenFeedGuids(url: string, ...guids: string[]): Promise; storeHoundActivity(challengeId: string, ...activityHashes: string[]): Promise; + hasSeenHoundChallenge(challengeId: string): Promise; hasSeenHoundActivity(challengeId: string, ...activityHashes: string[]): Promise; storeHoundActivityEvent(challengeId: string, activityId: string, eventId: string): Promise; getHoundActivity(challengeId: string, activityId: string): Promise; diff --git a/src/hound/reader.ts b/src/hound/reader.ts index 3c2633f2c..378ce0763 100644 --- a/src/hound/reader.ts +++ b/src/hound/reader.ts @@ -83,20 +83,26 @@ export class HoundReader { const resAct = await this.houndClient.get(`https://api.challengehound.com/v1/activities?challengeId=${challengeId}&size=10`); const activites = (resAct.data["results"] as HoundActivity[]).map(a => ({...a, hash: HoundReader.hashActivity(a)})); const seen = await this.storage.hasSeenHoundActivity(challengeId, ...activites.map(a => a.hash)); - for (const activity of activites) { - if (seen.includes(activity.hash)) { - continue; - } - this.queue.push({ - eventName: "hound.activity", - sender: "HoundReader", - data: { - challengeId, - activity: activity, + + // Don't emit anything if our cache is empty, as we'll probably create duplicates. + const hasSeenChallenge = await this.storage.hasSeenHoundChallenge(challengeId); + if (hasSeenChallenge) { + for (const activity of activites) { + if (seen.includes(activity.hash)) { + continue; } - }); + this.queue.push({ + eventName: "hound.activity", + sender: "HoundReader", + data: { + challengeId, + activity: activity, + } + }); + } } - await this.storage.storeHoundActivity(challengeId, ...activites.map(a => a.hash)) + // Ensure we don't add duplicates to the storage. + await this.storage.storeHoundActivity(challengeId, ...activites.filter(s => !seen.includes(s.hash)).map(a => a.hash)) } public async pollChallenges(): Promise { From 1d07dea7301aaa8f5894d09680b6cd2821a705e9 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 28 Oct 2024 11:37:43 +0000 Subject: [PATCH 2/5] Missed a bit --- src/Stores/RedisStorageProvider.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Stores/RedisStorageProvider.ts b/src/Stores/RedisStorageProvider.ts index 4cf1b8d63..d4d327bb2 100644 --- a/src/Stores/RedisStorageProvider.ts +++ b/src/Stores/RedisStorageProvider.ts @@ -250,6 +250,11 @@ export class RedisStorageProvider extends RedisStorageContextualProvider impleme await this.redis.ltrim(key, 0, MAX_FEED_ITEMS); } + public async hasSeenHoundChallenge(challengeId: string): Promise { + const key = `${HOUND_GUIDS}${challengeId}`; + return (await this.redis.exists(key)) === 1; + } + public async hasSeenHoundActivity(challengeId: string, ...activityHashes: string[]): Promise { let multi = this.redis.multi(); const key = `${HOUND_GUIDS}${challengeId}`; From e7e872d5e8870a56581b79ba3889c8d1953971e3 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 28 Oct 2024 11:49:40 +0000 Subject: [PATCH 3/5] No-op if no hashes need to be pushed. --- src/Stores/RedisStorageProvider.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Stores/RedisStorageProvider.ts b/src/Stores/RedisStorageProvider.ts index d4d327bb2..bd045cd54 100644 --- a/src/Stores/RedisStorageProvider.ts +++ b/src/Stores/RedisStorageProvider.ts @@ -245,6 +245,9 @@ export class RedisStorageProvider extends RedisStorageContextualProvider impleme } public async storeHoundActivity(challengeId: string, ...activityHashes: string[]): Promise { + if (activityHashes.length === 0) { + return; + } const key = `${HOUND_GUIDS}${challengeId}`; await this.redis.lpush(key, ...activityHashes); await this.redis.ltrim(key, 0, MAX_FEED_ITEMS); From 32f5b4ec33ec9625c84338d9a935f93ec3f53867 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 28 Oct 2024 11:51:02 +0000 Subject: [PATCH 4/5] changelog --- changelog.d/982.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/982.bugfix diff --git a/changelog.d/982.bugfix b/changelog.d/982.bugfix new file mode 100644 index 000000000..aa5bf45c8 --- /dev/null +++ b/changelog.d/982.bugfix @@ -0,0 +1 @@ +Fix Challenge Hound activities being duplicated if the cache layer (e.g Redis) goes away. From e423c67171b671100fc07f70ad281a80460ad743 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 29 Oct 2024 11:01:08 +0000 Subject: [PATCH 5/5] lint --- src/config/permissions.rs | 9 +++------ src/format_util.rs | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/config/permissions.rs b/src/config/permissions.rs index 94cfa9873..63997a191 100644 --- a/src/config/permissions.rs +++ b/src/config/permissions.rs @@ -113,13 +113,10 @@ impl BridgePermissions { continue; } for actor_service in actor_permission.services.iter() { - match &actor_service.service { - Some(actor_service_service) => { - if actor_service_service != &service && actor_service_service != "*" { - continue; - } + if let Some(actor_service_service) = &actor_service.service { + if actor_service_service != &service && actor_service_service != "*" { + continue; } - None => {} } if permission_level_to_int(actor_service.level.clone())? >= permission_int { return Ok(true); diff --git a/src/format_util.rs b/src/format_util.rs index dc11b7ffc..0c6123970 100644 --- a/src/format_util.rs +++ b/src/format_util.rs @@ -174,9 +174,9 @@ pub fn hash_id(id: String) -> Result { #[napi(js_name = "sanitizeHtml")] pub fn hookshot_sanitize_html(html: String) -> String { - return sanitize_html( + sanitize_html( html.as_str(), HtmlSanitizerMode::Compat, RemoveReplyFallback::No, - ); + ) }