Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
150 changes: 150 additions & 0 deletions apps/app/src/features/search/utils/disable-user-pages.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
import type { QueryTerms } from '~/server/interfaces/search';

import { excludeUserPagesFromQuery } from './disable-user-pages';

describe('excludeUserPagesFromQuery()', () => {
it('should exclude user page strings from query prefix', () => {
const userString = '/user';
const userStringSlash = '/user/';
const userStringSubPath = '/user/settings';
const userStringSubPathDeep = '/user/profile/edit';
const userStringSubPathQuery = '/user/settings?ref=top';

const query: QueryTerms = {
match: [],
not_match: [],
phrase: [],
not_phrase: [],
prefix: [
userString,
userStringSlash,
userStringSubPath,
userStringSubPathDeep,
userStringSubPathQuery,
],
not_prefix: [],
tag: [],
not_tag: [],
};

excludeUserPagesFromQuery(query);

expect(query.prefix).not.toContain(userString);
// Should only contain '/user'
expect(query.not_prefix).toContain(userString);

expect(query.prefix).not.toContain(userStringSlash);
expect(query.not_prefix).not.toContain(userStringSlash);

expect(query.prefix).not.toContain(userStringSubPath);
expect(query.not_prefix).not.toContain(userStringSubPath);

expect(query.prefix).not.toContain(userStringSubPathDeep);
expect(query.not_prefix).not.toContain(userStringSubPathDeep);

expect(query.prefix).not.toContain(userStringSubPathQuery);
expect(query.not_prefix).not.toContain(userStringSubPathQuery);
});

it('should not exclude strings similar to /user from query prefix', () => {
const userRouter = '/userouter';
const userRoot = '/useroot';
const userSettings = '/user-settings';
const apiUser = '/api/user';
const emptyString = '';
const rootOnly = '/';
const upperCase = '/USER';
const doubleSlashStart = '//user/new';
const doubleSlashSub = '/user//new';

const query: QueryTerms = {
match: [],
not_match: [],
phrase: [],
not_phrase: [],
prefix: [
userRouter,
userRoot,
userSettings,
apiUser,
emptyString,
rootOnly,
upperCase,
doubleSlashStart,
doubleSlashSub,
],
not_prefix: [],
tag: [],
not_tag: [],
};

excludeUserPagesFromQuery(query);

expect(query.prefix).toContain(userRouter);
expect(query.not_prefix).not.toContain(userRouter);

expect(query.prefix).toContain(userRoot);
expect(query.not_prefix).not.toContain(userRoot);

expect(query.prefix).toContain(userSettings);
expect(query.not_prefix).not.toContain(userSettings);

expect(query.prefix).toContain(apiUser);
expect(query.not_prefix).not.toContain(apiUser);

expect(query.prefix).toContain(emptyString);
expect(query.not_prefix).not.toContain(emptyString);

expect(query.prefix).toContain(rootOnly);
expect(query.not_prefix).not.toContain(rootOnly);

expect(query.prefix).toContain(upperCase);
expect(query.not_prefix).not.toContain(upperCase);

expect(query.prefix).toContain(doubleSlashStart);
expect(query.not_prefix).not.toContain(doubleSlashStart);

expect(query.prefix).toContain(doubleSlashSub);
expect(query.not_prefix).not.toContain(doubleSlashSub);
});

it('should add /user to not_prefix when it is empty', () => {
const query: QueryTerms = {
match: [],
not_match: [],
phrase: [],
not_phrase: [],
prefix: [],
not_prefix: [],
tag: [],
not_tag: [],
};

excludeUserPagesFromQuery(query);

expect(query.prefix).toHaveLength(0);
expect(query.not_prefix).toContain('/user');
expect(query.not_prefix).toHaveLength(1);
});

it('should remove existing /user strings and leave not_prefix with just one /user string', () => {
const userString = '/user';

const query: QueryTerms = {
match: [],
not_match: [],
phrase: [],
not_phrase: [],
prefix: [userString, userString],
not_prefix: [userString, userString],
tag: [],
not_tag: [],
};

excludeUserPagesFromQuery(query);

expect(query.prefix).toHaveLength(0);
expect(query.not_prefix).toContain('/user');
expect(query.not_prefix).toHaveLength(1);
});
});
10 changes: 10 additions & 0 deletions apps/app/src/features/search/utils/disable-user-pages.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

単体テスト追加してください

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import type { QueryTerms } from '~/server/interfaces/search';

export function excludeUserPagesFromQuery(terms: QueryTerms): void {
const userRegex: RegExp = /^\/user($|\/(?!\/))/;

terms.prefix = terms.prefix.filter((p) => !userRegex.test(p));
terms.not_prefix = terms.not_prefix.filter((p) => !userRegex.test(p));

terms.not_prefix.push('/user');
}
150 changes: 150 additions & 0 deletions apps/app/src/server/service/search-query.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
import { vi } from 'vitest';
import { type MockProxy, mock } from 'vitest-mock-extended';

import { SearchDelegatorName } from '~/interfaces/named-query';
import type Crowi from '~/server/crowi';
import { configManager } from '~/server/service/config-manager/config-manager';

import type { SearchDelegator } from '../interfaces/search';
import NamedQuery from '../models/named-query';
import SearchService from './search';
import type ElasticsearchDelegator from './search-delegator/elasticsearch';

// Mock NamedQuery
vi.mock('~/server/models/named-query', () => {
const mockModel = {
findOne: vi.fn(),
};
return {
NamedQuery: mockModel,
default: mockModel,
};
});

// Mock config manager
vi.mock('~/server/service/config-manager/config-manager', () => {
return {
default: {
getConfig: vi.fn(),
},
configManager: {
getConfig: vi.fn(),
},
};
});

class TestSearchService extends SearchService {
override generateFullTextSearchDelegator(): ElasticsearchDelegator {
return mock<ElasticsearchDelegator>();
}

override generateNQDelegators(): {
[key in SearchDelegatorName]: SearchDelegator;
} {
return {
[SearchDelegatorName.DEFAULT]: mock<SearchDelegator>(),
[SearchDelegatorName.PRIVATE_LEGACY_PAGES]: mock<SearchDelegator>(),
};
}

override registerUpdateEvent(): void {}

override get isConfigured(): boolean {
return false;
}
}

describe('searchParseQuery()', () => {
let searchService: TestSearchService;
let mockCrowi: MockProxy<Crowi>;

beforeEach(() => {
vi.clearAllMocks();

mockCrowi = mock<Crowi>();
mockCrowi.configManager = configManager;
searchService = new TestSearchService(mockCrowi);
});

it('should contain /user in the not_prefix query when user pages are disabled', async () => {
vi.mocked(configManager.getConfig).mockImplementation((key: string) => {
if (key === 'security:disableUserPages') {
return true;
}

return false;
});

const result = await searchService.parseSearchQuery('/user/settings', null);

expect(configManager.getConfig).toHaveBeenCalledWith(
'security:disableUserPages',
);
expect(result.terms.not_prefix).toContain('/user');
expect(result.terms.prefix).toHaveLength(0);
});

it('should contain /user in the not_prefix even when search query is not a user page', async () => {
vi.mocked(configManager.getConfig).mockImplementation((key: string) => {
if (key === 'security:disableUserPages') {
return true;
}

return false;
});

const result = await searchService.parseSearchQuery('/new-task', null);

expect(configManager.getConfig).toHaveBeenCalledWith(
'security:disableUserPages',
);
expect(result.terms.not_prefix).toContain('/user');
expect(result.terms.prefix).toHaveLength(0);
});

it('should add specific user prefixes in the query when user pages are enabled', async () => {
vi.mocked(configManager.getConfig).mockImplementation((key: string) => {
if (key === 'security:disableUserPages') {
return false;
}

return true;
});

const result = await searchService.parseSearchQuery('/user/settings', null);

expect(configManager.getConfig).toHaveBeenCalledWith(
'security:disableUserPages',
);
expect(result.terms.not_prefix).not.toContain('/user');
expect(result.terms.not_prefix).not.toContain('/user/settings');
expect(result.terms.match).toContain('/user/settings');
});

it('should filter user pages even when resolved from a named query alias', async () => {
vi.mocked(configManager.getConfig).mockImplementation((key: string) => {
if (key === 'security:disableUserPages') {
return true;
}

return false;
});

const shortcutName = 'my-shortcut';
const aliasPath = '/user/my-private-page';

// Mock the DB response
vi.mocked(NamedQuery.findOne).mockResolvedValue({
name: shortcutName,
aliasOf: aliasPath,
});

const result = await searchService.parseSearchQuery('dummy', shortcutName);

expect(configManager.getConfig).toHaveBeenCalledWith(
'security:disableUserPages',
);
expect(result.terms.not_prefix).toContain('/user');
expect(result.terms.match).toContain('/user/my-private-page');
});
});
46 changes: 25 additions & 21 deletions apps/app/src/server/service/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
isIncludeAiMenthion,
removeAiMenthion,
} from '~/features/search/utils/ai';
import { excludeUserPagesFromQuery } from '~/features/search/utils/disable-user-pages';
import { SearchDelegatorName } from '~/interfaces/named-query';
import type {
IFormattedSearchResult,
Expand Down Expand Up @@ -328,34 +329,37 @@ class SearchService implements SearchQueryParser, SearchResolver {
_queryString: string,
nqName: string | null,
): Promise<ParsedQuery> {
const disableUserPages = configManager.getConfig(
'security:disableUserPages',
);
const queryString = normalizeQueryString(_queryString);

const terms = this.parseQueryString(queryString);

if (nqName == null) {
return { queryString, terms };
}
let parsedQuery: ParsedQuery = { queryString, terms };

const nq = await NamedQuery.findOne({ name: normalizeNQName(nqName) });
if (nqName != null) {
const nq = await NamedQuery.findOne({ name: normalizeNQName(nqName) });

// will delegate to full-text search
if (nq == null) {
logger.debug(
`Delegated to full-text search since a named query document did not found. (nqName="${nqName}")`,
);
return { queryString, terms };
}
if (nq != null) {
const { aliasOf, delegatorName } = nq;

const { aliasOf, delegatorName } = nq;
if (aliasOf != null) {
parsedQuery = {
queryString: normalizeQueryString(aliasOf),
terms: this.parseQueryString(aliasOf),
};
} else {
parsedQuery = { queryString, terms, delegatorName };
}
} else {
logger.debug(
`Delegated to full-text search since a named query document did not found. (nqName="${nqName}")`,
);
}
}

let parsedQuery: ParsedQuery;
if (aliasOf != null) {
parsedQuery = {
queryString: normalizeQueryString(aliasOf),
terms: this.parseQueryString(aliasOf),
};
} else {
parsedQuery = { queryString, terms, delegatorName };
if (disableUserPages) {
excludeUserPagesFromQuery(parsedQuery.terms);
}

return parsedQuery;
Expand Down
Loading