Skip to content
Open
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
6 changes: 4 additions & 2 deletions apps/api/src/modules/webhook/webhook.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Injectable, Logger, NotFoundException, ForbiddenException } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the webhook.service.ts file for the import and ConfigService usage
echo "=== webhook.service.ts content (lines 1-70) ===" 
sed -n '1,70p' apps/api/src/modules/webhook/webhook.service.ts | cat -n

echo -e "\n=== webhook.service.ts content (lines 805-815) ===" 
sed -n '805,815p' apps/api/src/modules/webhook/webhook.service.ts | cat -n

Repository: refly-ai/refly

Length of output: 3115


🏁 Script executed:

#!/bin/bash
# Check if ConfigModule is imported in webhook.module.ts
echo "=== webhook.module.ts content ===" 
cat apps/api/src/modules/webhook/webhook.module.ts | cat -n

Repository: refly-ai/refly

Length of output: 1751


Add ConfigModule to webhook.module.ts imports

ConfigService is properly injected in the service, but the module doesn't import ConfigModule, which is required to provide ConfigService to WebhookService. Without it, dependency injection will fail at runtime.

Update apps/api/src/modules/webhook/webhook.module.ts to import ConfigModule:

Required fix
import { ConfigModule } from '@nestjs/config';

`@Module`({
  imports: [
    ConfigModule,
    CommonModule,
    forwardRef(() => AuthModule),
    forwardRef(() => WorkflowModule),
    WorkflowAppModule,
    CanvasModule,
  ],
  // ...
})
export class WebhookModule {}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/api/src/modules/webhook/webhook.service.ts` at line 2, The WebhookModule
is not importing ConfigModule even though WebhookService injects ConfigService;
update the WebhookModule (class WebhookModule in webhook.module.ts) to add
ConfigModule to its imports array so Nest can provide ConfigService at
runtime—specifically include ConfigModule alongside CommonModule, forwardRef(()
=> AuthModule), forwardRef(() => WorkflowModule), WorkflowAppModule, and
CanvasModule in the `@Module`({ imports: [...] }) declaration.

import { PrismaService } from '../common/prisma.service';
import { RedisService } from '../common/redis.service';
import { WorkflowAppService } from '../workflow-app/workflow-app.service';
Expand Down Expand Up @@ -60,6 +61,7 @@ export class WebhookService {
private readonly redis: RedisService,
private readonly workflowAppService: WorkflowAppService,
private readonly canvasService: CanvasService,
private readonly config: ConfigService,
) {}

/**
Expand Down Expand Up @@ -806,8 +808,8 @@ export class WebhookService {
* Generate webhook URL
*/
private generateWebhookUrl(webhookId: string): string {
// TODO: Get base URL from config
return `https://api.refly.ai/v1/openapi/webhook/${webhookId}/run`;
const baseUrl = this.config.get<string>('WEBHOOK_BASE_URL') || 'https://api.refly.ai';
return `${baseUrl}/v1/openapi/webhook/${webhookId}/run`;
}
}

Expand Down
265 changes: 265 additions & 0 deletions apps/api/src/utils/web-search/cached-searcher.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,265 @@
import { Test, TestingModule } from '@nestjs/testing';
import { CachedWebSearcher, WebSearchCacheConfig } from './cached-searcher';
import { BaseWebSearcher, WebSearchConfig } from './base';
import { RedisService } from '../../modules/common/redis.service';
import { WebSearchRequest, WebSearchResult } from '@refly/openapi-schema';

/**
* Mock Redis Service for testing
*/
class MockRedisService {
private store: Map<string, string> = new Map();

async get(key: string): Promise<string | null> {
return this.store.get(key) || null;
}

async setex(key: string, seconds: number, value: string): Promise<void> {
this.store.set(key, value);
}

async del(key: string): Promise<void> {
this.store.delete(key);
}

async set(
key: string,
value: string,
mode?: string,
seconds?: number,
flag?: string,
): Promise<string | null> {
if (flag === 'NX' && this.store.has(key)) {
return null;
}
this.store.set(key, value);
return 'OK';
}

async eval(
script: string,
numKeys: number,
...args: string[]
): Promise<unknown> {
const key = args[0];
const token = args[1];
const stored = this.store.get(key);
if (stored === token) {
this.store.delete(key);
return 1;
}
return 0;
}

async scan(
cursor: string,
...args: (string | number)[]
): Promise<[string, string[]]> {
const pattern = args[args.indexOf('MATCH') + 1] as string;
const keys: string[] = [];

for (const [key] of this.store.entries()) {
if (key.startsWith(pattern.replace('*', ''))) {
keys.push(key);
}
}

return ['0', keys];
}

clear(): void {
this.store.clear();
}
}

/**
* Mock Web Searcher for testing
*/
class MockWebSearcher extends BaseWebSearcher {
public searchCount = 0;
private mockResults: WebSearchResult[];

constructor(
config?: WebSearchConfig,
mockResults?: WebSearchResult[],
) {
super(config);
this.mockResults = mockResults || [
{
name: 'Test Result',
url: 'https://example.com',
snippet: 'Test snippet',
locale: 'en',
},
];
}

async search(): Promise<WebSearchResult[]> {
this.searchCount++;
return this.mockResults;
}
}

describe('CachedWebSearcher', () => {
let cachedSearcher: CachedWebSearcher;
let mockSearcher: MockWebSearcher;
let mockRedis: MockRedisService;

beforeEach(() => {
mockRedis = new MockRedisService();
mockSearcher = new MockWebSearcher();
cachedSearcher = new CachedWebSearcher(mockSearcher, mockRedis as any, {
ttl: 60,
Comment on lines +111 to +112
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n apps/api/src/utils/web-search/cached-searcher.spec.ts | head -220 | tail -120

Repository: refly-ai/refly

Length of output: 4816


🏁 Script executed:

# Also check total file size
wc -l apps/api/src/utils/web-search/cached-searcher.spec.ts

Repository: refly-ai/refly

Length of output: 115


🏁 Script executed:

# Search for all `as any` in the file with context
rg -n -B2 -A2 '\bas any\b' apps/api/src/utils/web-search/cached-searcher.spec.ts

Repository: refly-ai/refly

Length of output: 1039


🏁 Script executed:

# Find MockRedisService definition
rg -n 'class MockRedisService' apps/api/src/utils/web-search/cached-searcher.spec.ts -A 20

Repository: refly-ai/refly

Length of output: 589


🏁 Script executed:

# Find CachedWebSearcher constructor to see what it expects
fd -e ts cached-searcher.ts | head -5

Repository: refly-ai/refly

Length of output: 106


🏁 Script executed:

# Check the interface/type expected by CachedWebSearcher
rg -n 'constructor' apps/api/src/utils/web-search/cached-searcher.ts -A 5

Repository: refly-ai/refly

Length of output: 276


🏁 Script executed:

# Find RedisService interface/type definition
rg -n 'interface RedisService|type RedisService|class RedisService' --type=ts

Repository: refly-ai/refly

Length of output: 174


🏁 Script executed:

# Get more context around MockRedisService to see its full interface
rg -n 'class MockRedisService' apps/api/src/utils/web-search/cached-searcher.spec.ts -A 40

Repository: refly-ai/refly

Length of output: 1079


🏁 Script executed:

# Check RedisService class definition and what methods it has
cat -n apps/api/src/modules/common/redis.service.ts | head -100

Repository: refly-ai/refly

Length of output: 4083


🏁 Script executed:

# Check if MockRedisService implements RedisService or has public store property
rg -n 'store|implements|extends' apps/api/src/utils/web-search/cached-searcher.spec.ts | head -20

Repository: refly-ai/refly

Length of output: 754


🏁 Script executed:

# Check if there's a public interface or base class for RedisService
rg -n 'interface.*Redis|export.*Redis' apps/api/src/modules/common/redis.service.ts -B2 -A5

Repository: refly-ai/refly

Length of output: 414


🏁 Script executed:

# Look at the broader structure of RedisService and its public methods
sed -n '27,200p' apps/api/src/modules/common/redis.service.ts

Repository: refly-ai/refly

Length of output: 5465


🏁 Script executed:

# Check what interface CachedWebSearcher expects for redisService parameter
sed -n '48,55p' apps/api/src/utils/web-search/cached-searcher.ts

Repository: refly-ai/refly

Length of output: 288


🏁 Script executed:

# Look for any interface definition in the cached-searcher or related files
rg -n 'interface.*Redis|type.*Redis' apps/api/src/utils/web-search/ --type=ts

Repository: refly-ai/refly

Length of output: 40


🏁 Script executed:

# Check if MockRedisService is explicitly implementing any interface
sed -n '10,75p' apps/api/src/utils/web-search/cached-searcher.spec.ts

Repository: refly-ai/refly

Length of output: 1403


Make MockRedisService implement RedisService interface to eliminate constructor casts.

Lines 111 and 191 cast mockRedis as any when passing to CachedWebSearcher, which expects RedisService. Since MockRedisService provides all the required methods (get, setex, del, set, eval, scan), it should directly satisfy the type contract without casting.

For lines 200, 202, and 214 that access store for assertions, expose a public method on MockRedisService (e.g., getStore()) to avoid needing as any casts to access internals.

Per coding guidelines: "Avoid using any type whenever possible - use unknown type instead with proper type guards."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/api/src/utils/web-search/cached-searcher.spec.ts` around lines 111 -
112, Make MockRedisService explicitly implement the RedisService interface so
instances can be passed to CachedWebSearcher without casting; update the
MockRedisService class signature to implement RedisService and ensure methods
get, setex, del, set, eval, scan match the interface types (prefer unknown over
any in signatures). Add a public accessor like getStore(): Record<string,string>
to expose the internal store for tests so assertions can call
mockRedis.getStore() without using casts. Then remove the remaining "as any"
casts where mockRedis is passed to CachedWebSearcher and where store is accessed
in the tests.

maxCachedResults: 10,
});
});

afterEach(() => {
mockRedis.clear();
});

describe('Basic Caching', () => {
it('should fetch from origin on cache miss', async () => {
const req: WebSearchRequest = { q: 'nestjs tutorial', limit: 5 };

const results = await cachedSearcher.search(req);

expect(results).toHaveLength(1);
expect(results[0].name).toBe('Test Result');
expect(mockSearcher.searchCount).toBe(1);
});

it('should return cached results on cache hit', async () => {
const req: WebSearchRequest = { q: 'nestjs tutorial', limit: 5 };

// First call - cache miss
await cachedSearcher.search(req);
expect(mockSearcher.searchCount).toBe(1);

// Second call - cache hit
const results = await cachedSearcher.search(req);
expect(results).toHaveLength(1);
expect(mockSearcher.searchCount).toBe(1); // Should not increment
});

it('should generate consistent cache keys for same query', async () => {
const req1: WebSearchRequest = { q: 'NestJS Tutorial', limit: 5 };
const req2: WebSearchRequest = { q: 'nestjs tutorial', limit: 5 };

await cachedSearcher.search(req1);
await cachedSearcher.search(req2);

// Both should hit the same cache (case insensitive normalization)
expect(mockSearcher.searchCount).toBe(1);
});
});

describe('Cache Key Normalization', () => {
it('should normalize batch requests consistently', async () => {
const req = {
queries: [
{ q: 'query B', limit: 5 },
{ q: 'query A', limit: 5 },
],
limit: 10,
};

await cachedSearcher.search(req);

// Same queries in different order should hit cache
const req2 = {
queries: [
{ q: 'query A', limit: 5 },
{ q: 'query B', limit: 5 },
],
limit: 10,
};

await cachedSearcher.search(req2);
expect(mockSearcher.searchCount).toBe(1);
});
});

describe('Cache Configuration', () => {
it('should respect maxCachedResults limit', async () => {
mockSearcher = new MockWebSearcher({}, [
{ name: 'Result 1', url: 'https://1.com', snippet: '1', locale: 'en' },
{ name: 'Result 2', url: 'https://2.com', snippet: '2', locale: 'en' },
{ name: 'Result 3', url: 'https://3.com', snippet: '3', locale: 'en' },
]);

cachedSearcher = new CachedWebSearcher(mockSearcher, mockRedis as any, {
ttl: 60,
maxCachedResults: 2, // Only cache 2 results
});

const req: WebSearchRequest = { q: 'test', limit: 10 };
await cachedSearcher.search(req);

// Check cached value
const keys = Array.from((mockRedis as any).store.keys());
const cacheKey = keys.find((k: string) => k.startsWith('websearch:'));
const cached = JSON.parse((mockRedis as any).store.get(cacheKey));

expect(cached).toHaveLength(2);
});

it('should not cache empty results when cacheEmptyResults is false', async () => {
mockSearcher = new MockWebSearcher({}, []);

const req: WebSearchRequest = { q: 'test', limit: 10 };
await cachedSearcher.search(req);

// Should not have cached anything
const keys = Array.from((mockRedis as any).store.keys());
const cacheKeys = keys.filter((k: string) => k.startsWith('websearch:') && !k.includes(':lock'));

expect(cacheKeys).toHaveLength(0);
});
});

describe('Cache Invalidation', () => {
it('should invalidate cache correctly', async () => {
const req: WebSearchRequest = { q: 'test', limit: 5 };

await cachedSearcher.search(req);
expect(mockSearcher.searchCount).toBe(1);

await cachedSearcher.invalidateCache(req);

// After invalidation, should fetch from origin again
await cachedSearcher.search(req);
expect(mockSearcher.searchCount).toBe(2);
});
});

describe('Error Handling', () => {
it('should fallback to origin on Redis error', async () => {
// Simulate Redis failure
mockRedis.get = async () => {
throw new Error('Redis connection failed');
};

const req: WebSearchRequest = { q: 'test', limit: 5 };
const results = await cachedSearcher.search(req);

expect(results).toHaveLength(1);
expect(mockSearcher.searchCount).toBe(1);
});
});

describe('Cache Statistics', () => {
it('should return cache statistics', async () => {
const req1: WebSearchRequest = { q: 'query1', limit: 5 };
const req2: WebSearchRequest = { q: 'query2', limit: 5 };

await cachedSearcher.search(req1);
await cachedSearcher.search(req2);

const stats = await cachedSearcher.getCacheStats();

expect(stats.totalKeys).toBe(2);
expect(stats.pattern).toBe('websearch:*');
});
});
});
Loading