Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix challengehound duplication bug. #982

Merged
merged 5 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/982.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix Challenge Hound activities being duplicated if the cache layer (e.g Redis) goes away.
5 changes: 5 additions & 0 deletions src/Stores/MemoryStorageProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,16 @@ export class MemoryStorageProvider extends MSP implements IBridgeStorageProvider
set.pop();
}
}

async hasSeenHoundActivity(challengeId: string, ...activityIds: string[]): Promise<string[]> {
const existing = this.houndActivityIds.get(challengeId);
return existing ? activityIds.filter((existingGuid) => existing.includes(existingGuid)) : [];
}

public async hasSeenHoundChallenge(challengeId: string): Promise<boolean> {
return this.houndActivityIds.has(challengeId);
}

public async storeHoundActivityEvent(challengeId: string, activityId: string, eventId: string): Promise<void> {
this.houndActivityIdToEvent.set(`${challengeId}.${activityId}`, eventId);
}
Expand Down
10 changes: 9 additions & 1 deletion src/Stores/RedisStorageProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.";
Expand Down Expand Up @@ -245,11 +245,19 @@ export class RedisStorageProvider extends RedisStorageContextualProvider impleme
}

public async storeHoundActivity(challengeId: string, ...activityHashes: string[]): Promise<void> {
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<boolean> {
const key = `${HOUND_GUIDS}${challengeId}`;
return (await this.redis.exists(key)) === 1;
}

public async hasSeenHoundActivity(challengeId: string, ...activityHashes: string[]): Promise<string[]> {
let multi = this.redis.multi();
const key = `${HOUND_GUIDS}${challengeId}`;
Expand Down
1 change: 1 addition & 0 deletions src/Stores/StorageProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export interface IBridgeStorageProvider extends IAppserviceStorageProvider, ISto
hasSeenFeedGuids(url: string, ...guids: string[]): Promise<string[]>;

storeHoundActivity(challengeId: string, ...activityHashes: string[]): Promise<void>;
hasSeenHoundChallenge(challengeId: string): Promise<boolean>;
hasSeenHoundActivity(challengeId: string, ...activityHashes: string[]): Promise<string[]>;
storeHoundActivityEvent(challengeId: string, activityId: string, eventId: string): Promise<void>;
getHoundActivity(challengeId: string, activityId: string): Promise<string|null>;
Expand Down
9 changes: 3 additions & 6 deletions src/config/permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/format_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@ pub fn hash_id(id: String) -> Result<String> {

#[napi(js_name = "sanitizeHtml")]
pub fn hookshot_sanitize_html(html: String) -> String {
return sanitize_html(
sanitize_html(
html.as_str(),
HtmlSanitizerMode::Compat,
RemoveReplyFallback::No,
);
)
}
30 changes: 18 additions & 12 deletions src/hound/reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<HoundPayload>({
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<HoundPayload>({
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<void> {
Expand Down
Loading