a #25
a #25
Conversation
Walkthrough
Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AdvancedSearchService
participant NoteDatabase
User->>AdvancedSearchService: searchNote(me, opts, pagination, q)
AdvancedSearchService->>NoteDatabase: filter(notes, visibility, indexability)
NoteDatabase-->>AdvancedSearchService: return filtered notes
AdvancedSearchService-->>User: return search results
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
이 PR에 의한 api.json 차이 차이점은 여기에서 볼 수 있음 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
packages/backend/test/e2e/search-notes.ts (1)
Line range hint
315-340: コメントアウトされたテストコードがありますセンシティブオプションに関するテストがコメントアウトされています。実装予定で一時的にコメントアウトしている場合は問題ありませんが、不要であれば削除を検討してください。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/backend/src/core/AdvancedSearchService.ts (3 hunks)
- packages/backend/test/e2e/search-notes.ts (4 hunks)
🧰 Additional context used
🔇 Additional comments (10)
packages/backend/test/e2e/search-notes.ts (7)
218-225: テストケースが正しく追加されています'可視性 followers, specified でてこない' のテストで、フォロワー以外や指定ユーザー以外にはノートが検索結果に表示されないことを確認しています。
Line range hint
226-236: テストケースが正しく追加されています'可視性 followers, specified でてくる' のテストで、フォロワーや指定ユーザーにはノートが検索結果に表示されることを確認しています。
240-278: isIndexable フラグの影響を正しくテストしていますユーザーの
isIndexableフラグをfalseに設定した場合に、検索結果からノートが除外されることを確認しています。また、isIndexableをtrueに戻すことで、再びノートが検索結果に表示されることを確認しています。
279-288: ミュートユーザーのノートの表示を確認していますミュートしていない場合、ノートが検索結果に表示されることを確認しています。
Line range hint
289-296: ミュートしたユーザーのノートが検索結果に表示されないことを確認していますミュートしたユーザーのノートが検索結果に表示されないことを正しくテストしています。
Line range hint
307-314: ブロックされている場合のノートが検索結果に表示されないことを確認していますブロックされている場合、ノートが検索結果に表示されないことを正しくテストしています。
Line range hint
341-491: indexable が false の場合の挙動を詳細にテストしていますユーザーがリアクション、リノート、返信、お気に入り、クリップ、投票といったアクションを行った場合に、
isIndexableがfalseのノートでも検索結果に表示されることを確認しています。各ケースでの検索結果の変化を包括的にテストしており、テストの網羅性が高いです。packages/backend/src/core/AdvancedSearchService.ts (3)
1106-1107: LGTM！ユーザーIDのチェックが適切に行われています
1185-1187: LGTM！可視性のチェックが正しく実装されています
1122-1122:⚠️ Potential issue論理エラー：ダイレクトメッセージの条件式が逆になっています
Line 1122の条件式で、'specified'な可視性のノートに対して、自分が宛先に含まれている場合に
nullを返しています。しかし、これは意図と逆であり、自分が宛先に含まれていない場合にnullを返すべきです。以下の差分を適用して論理エラーを修正してください：
- if (Note._source.visibility === 'specified' && Note._source.visibleUserIds && Note._source.visibleUserIds.includes(meUserId)) return null; //ダイレクトで自分が宛先に含まれていない + if (Note._source.visibility === 'specified' && Note._source.visibleUserIds && !Note._source.visibleUserIds.includes(meUserId)) return null; //ダイレクトで自分が宛先に含まれていないLikely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/backend/src/core/AdvancedSearchService.ts (2)
Line range hint
1123-1181: ユーザーインタラクションのチェックロジックが複雑です。現在のインタラクションチェック（リアクション、投票、クリップ、お気に入り、Renote、返信）は、各チェックごとにOpenSearchクエリを実行しています。これは以下の問題を引き起こす可能性があります：
- パフォーマンスの低下：多数のOpenSearchクエリが実行されます。
- コードの重複：各チェックで似たようなクエリ構造が繰り返されています。
以下の改善を検討してください：
- 複数のインタラクションタイプを1つのクエリでチェックする。
- クエリ結果をキャッシュして再利用する。
- 共通のクエリ生成ロジックを抽出し、再利用可能な関数にする。
例えば：
private async checkUserInteractions(noteId: string, userId: string): Promise<boolean> { const interactionTypes = ['reaction', 'pollVote', 'favorite', 'renote', 'reply']; const queries = interactionTypes.map(type => ({ index: this.getIndexForType(type), body: { query: { bool: { must: [ { term: { [type === 'renote' ? 'renoteId' : 'noteId']: noteId } }, { term: { userId: userId } }, ], }, }, }, size: 0, })); const results = await this.opensearch.msearch({ body: queries.flatMap(q => [q, q.body]) }); return results.responses.some(response => response.hits.total.value > 0); } // 使用例 const hasInteraction = await this.checkUserInteractions(Note._id, meUserId); if (hasInteraction) return Note;このアプローチにより、コードの重複が減り、パフォーマンスが向上する可能性があります。
Line range hint
1106-1189: 全体的な改善が見られますが、さらなる最適化の余地があります。
filterメソッドの変更により、ユーザーの権限とノートの可視性に関するより包括的なチェックが導入されました。これらの変更は機能性を向上させていますが、以下の点でさらなる改善が可能です：
- 非インデックス可能なユーザーの処理ロジックを簡略化し、可読性を向上させる。
- ユーザーインタラクションのチェックを最適化し、複数のOpenSearchクエリを統合する。
- 可視性チェックのロジックに関するコメントを追加し、特に'public'と'home'の扱いについて説明を加える。
これらの改善により、コードの保守性とパフォーマンスが向上する可能性があります。
さらに、将来的にはユーザーインタラクションのチェックロジックを別のサービスに分離することを検討してください。これにより、
AdvancedSearchServiceの責務がより明確になり、コードの組織化が改善される可能性があります。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/backend/src/core/AdvancedSearchService.ts (3 hunks)
- packages/backend/test/e2e/search-notes.ts (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/backend/test/e2e/search-notes.ts
🧰 Additional context used
🔇 Additional comments (3)
packages/backend/src/core/AdvancedSearchService.ts (3)
1106-1108: ユーザー自身のノートの処理が改善されました。ユーザー自身のノートを即座に返すようになり、不要なチェックを回避しています。これは効率的な改善です。
1182-1189: インデックス可能なユーザーの可視性チェックが簡略化されています。インデックス可能なユーザーの投稿に対する可視性チェックが簡潔になっていますが、以下の点に注意が必要です：
- 'public'と'home'の可視性を同じように扱っていますが、これは意図的ですか？
- フォロワー限定とダイレクト投稿のチェックが適切に行われています。
この部分のロジックは明確で理解しやすくなっています。ただし、'public'と'home'の扱いが同じである理由について、コメントで説明を追加することを検討してください。
1119-1122:⚠️ Potential issue非インデックス可能なユーザーの処理に注意が必要です。
非インデックス可能なユーザーの投稿に対する可視性チェックが追加されていますが、以下の点に注意が必要です：
user.isIndexable === false && meUserId !== undefined && user.id !== meUserIdの条件が複雑で、理解しづらい可能性があります。- フォロワー限定投稿とダイレクト投稿の可視性チェックが行われていますが、これらのチェックは後のインタラクションチェックの前に行われるべきではないでしょうか。
これらのチェックの順序を再考し、可読性を向上させることを検討してください。例えば：
- if (user.isIndexable === false && meUserId !== undefined && user.id !== meUserId) { + if (!user.isIndexable && meUserId && user.id !== meUserId) { + // 可視性チェックを最初に行う + if (Note._source.visibility === 'followers' && !Followings.includes(Note._source.userId)) return null; + if (Note._source.visibility === 'specified' && Note._source.visibleUserIds && !Note._source.visibleUserIds.includes(meUserId)) return null; + + // その後でインタラクションチェックを行う // ... (インタラクションチェックのコード) }Likely invalid or redundant comment.
What
Why
Additional info (optional)
Checklist
Summary by CodeRabbit
新機能
バグ修正
テスト