From 6571b9f7108aed8210f8e0ab7d8dba83d22acf59 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Tue, 29 Oct 2024 11:07:56 +0000 Subject: [PATCH] Fix challengehound duplication bug. (#982) * Fix challengehound duplication bug. * Missed a bit * No-op if no hashes need to be pushed. * changelog * lint --- changelog.d/982.bugfix | 1 + src/Stores/MemoryStorageProvider.ts | 5 +++++ src/Stores/RedisStorageProvider.ts | 10 +++++++++- src/Stores/StorageProvider.ts | 1 + src/config/permissions.rs | 9 +++------ src/format_util.rs | 4 ++-- src/hound/reader.ts | 30 +++++++++++++++++------------ 7 files changed, 39 insertions(+), 21 deletions(-) create mode 100644 changelog.d/982.bugfix diff --git a/changelog.d/982.bugfix b/changelog.d/982.bugfix new file mode 100644 index 00000000..aa5bf45c --- /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. diff --git a/src/Stores/MemoryStorageProvider.ts b/src/Stores/MemoryStorageProvider.ts index a6336e59..e2d65be1 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 657b0b53..bd045cd5 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."; @@ -245,11 +245,19 @@ 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); } + 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}`; diff --git a/src/Stores/StorageProvider.ts b/src/Stores/StorageProvider.ts index dbff7f3b..74a0f345 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/config/permissions.rs b/src/config/permissions.rs index 94cfa987..63997a19 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 dc11b7ff..0c612397 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, - ); + ) } diff --git a/src/hound/reader.ts b/src/hound/reader.ts index 3c2633f2..378ce076 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 {