feat: filter _design docs from all replication#215
Conversation
because we manage them client-side
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces DocumentFilterService to exclude documents from replication by ID prefixes (default "_design/"), reads REPLICATION_IGNORED_PREFIXES from config, and integrates this filter into ChangesController and BulkDocumentService to skip non‑replicable IDs before permission checks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChangesController
participant DocumentFilterService
participant PermissionService
participant CouchDB
Client->>ChangesController: request changes stream
ChangesController->>DocumentFilterService: isReplicable(change.id)?
alt not replicable
DocumentFilterService-->>ChangesController: false
ChangesController-->>Client: skip change (advance seq)
else replicable
DocumentFilterService-->>ChangesController: true
ChangesController->>PermissionService: check read permission(change.id)
alt permitted
PermissionService-->>ChangesController: allowed
ChangesController-->>Client: emit change
else denied
PermissionService-->>ChangesController: denied
ChangesController-->>Client: record lostPermission / skip
end
ChangesController->>CouchDB: fetch docs/metadata (if needed)
CouchDB-->>ChangesController: docs/metadata
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements filtering of CouchDB design documents from replication endpoints to support client-side management of design documents. A new DocumentFilterService is introduced with configurable prefix-based filtering that defaults to excluding _design/ documents.
Changes:
- Added
DocumentFilterServicewith environment-configurable prefix filtering for replication - Applied document filtering to all replication endpoints: changes, bulk_get, all_docs, bulk_docs, and find
- Added comprehensive test coverage for design document filtering across all affected endpoints
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/restricted-endpoints/replication/document-filter/document-filter.service.ts | New service implementing prefix-based document filtering with REPLICATION_IGNORED_PREFIXES configuration |
| src/restricted-endpoints/replication/document-filter/document-filter.service.spec.ts | Comprehensive tests for DocumentFilterService covering default and custom configurations |
| src/restricted-endpoints/replication/replication.module.ts | Registers DocumentFilterService as a provider in the replication module |
| src/restricted-endpoints/replication/changes/changes.controller.ts | Applies document filtering to the changes feed endpoint |
| src/restricted-endpoints/replication/changes/changes.controller.spec.ts | Adds test verifying design documents are filtered from changes feed |
| src/restricted-endpoints/replication/bulk-document/bulk-document.service.ts | Applies document filtering to bulk_get, all_docs, bulk_docs, and find operations |
| src/restricted-endpoints/replication/bulk-document/bulk-document.service.spec.ts | Adds tests verifying design documents are filtered from all bulk operations |
| .env.template | Documents the REPLICATION_IGNORED_PREFIXES configuration option |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/restricted-endpoints/replication/document-filter/document-filter.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/restricted-endpoints/replication/changes/changes.controller.ts (1)
104-106:lastProcessedSeqis not advanced when a design doc is skipped.Because
continuebypasses thelastProcessedSeq = change.seqassignment at Line 128, in a paginated batch where a design doc sits between the last processed change and an overflow point, the returnedlast_seqremains behind the design doc's sequence number. The next page request re-includes the design doc from CouchDB, which is immediately skipped again — functionally correct but a needless round-trip for each page boundary that crosses a design doc.♻️ Advance `lastProcessedSeq` for skipped docs
- if (!this.documentFilter.isReplicable(change.id)) { - continue; - } + if (!this.documentFilter.isReplicable(change.id)) { + lastProcessedSeq = change.seq; + continue; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/restricted-endpoints/replication/changes/changes.controller.ts` around lines 104 - 106, The loop in changes.controller.ts skips design docs via documentFilter.isReplicable(change.id) using "continue" but fails to advance lastProcessedSeq, causing repeated re-fetches; update the loop so lastProcessedSeq (the variable tracking progress) is set to change.seq for every change before skipping (i.e., set lastProcessedSeq = change.seq when documentFilter.isReplicable(change.id) returns false) or move the assignment of lastProcessedSeq = change.seq outside/above the continue check so it always advances even for skipped docs (refer to lastProcessedSeq, documentFilter.isReplicable(change.id), and change.seq).src/restricted-endpoints/replication/bulk-document/bulk-document.service.ts (1)
86-113:filterBulkDocsRequest: design doc IDs are included in the_all_docsquery unnecessarily.
allDocsRequest.keysis built from allrequest.docsbefore theisReplicablefilter is applied, so design doc IDs are sent to CouchDB even though they'll be discarded in the finaldocs.filter(). Pre-filtering eliminates the unnecessary lookup.♻️ Pre-filter replicable docs before building the CouchDB query
async filterBulkDocsRequest(...): Promise<BulkDocsRequest> { const ability = this.permissionService.getAbilityFor(user); + const replicableDocs = request.docs.filter((doc) => + this.documentFilter.isReplicable(doc._id), + ); const allDocsRequest: AllDocsRequest = { - keys: request.docs.map((doc) => doc._id), + keys: replicableDocs.map((doc) => doc._id), }; const response = await firstValueFrom( this.couchdbService.post<AllDocsResponse>(db, '_all_docs', allDocsRequest, { include_docs: true, }), ); return { new_edits: request.new_edits, - docs: request.docs.filter( - (doc) => - this.documentFilter.isReplicable(doc._id) && - this.hasPermissionsForDoc( - doc, - response.rows.find((responseDoc) => responseDoc.id === doc._id)?.doc, - ability, - ), + docs: replicableDocs.filter((doc) => + this.hasPermissionsForDoc( + doc, + response.rows.find((responseDoc) => responseDoc.id === doc._id)?.doc, + ability, + ), ), }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/restricted-endpoints/replication/bulk-document/bulk-document.service.ts` around lines 86 - 113, In filterBulkDocsRequest, avoid sending design/doc IDs to CouchDB by pre-filtering request.docs with this.documentFilter.isReplicable before building allDocsRequest.keys; create a filteredDocs array (from request.docs.filter(this.documentFilter.isReplicable)), use filteredDocs.map(doc => doc._id) for allDocsRequest.keys when calling this.couchdbService.post, and then base the returned docs array and permission checks (this.hasPermissionsForDoc) on filteredDocs and their matching response.rows entries (matching by id) instead of the original request.docs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.template:
- Around line 13-15: Clarify that REPLICATION_IGNORED_PREFIXES replaces the
default rather than extends it: update the .env.template text around the
REPLICATION_IGNORED_PREFIXES entry to state that if the variable is set it
overrides the default "_design/" filter (not appended), that it accepts a
comma-separated list to specify multiple prefixes, and include an example
showing both defaults preserved and additional prefixes (e.g.
REPLICATION_IGNORED_PREFIXES=_design/,_local/) so users know to include
"_design/" when adding others.
---
Nitpick comments:
In `@src/restricted-endpoints/replication/bulk-document/bulk-document.service.ts`:
- Around line 86-113: In filterBulkDocsRequest, avoid sending design/doc IDs to
CouchDB by pre-filtering request.docs with this.documentFilter.isReplicable
before building allDocsRequest.keys; create a filteredDocs array (from
request.docs.filter(this.documentFilter.isReplicable)), use filteredDocs.map(doc
=> doc._id) for allDocsRequest.keys when calling this.couchdbService.post, and
then base the returned docs array and permission checks
(this.hasPermissionsForDoc) on filteredDocs and their matching response.rows
entries (matching by id) instead of the original request.docs.
In `@src/restricted-endpoints/replication/changes/changes.controller.ts`:
- Around line 104-106: The loop in changes.controller.ts skips design docs via
documentFilter.isReplicable(change.id) using "continue" but fails to advance
lastProcessedSeq, causing repeated re-fetches; update the loop so
lastProcessedSeq (the variable tracking progress) is set to change.seq for every
change before skipping (i.e., set lastProcessedSeq = change.seq when
documentFilter.isReplicable(change.id) returns false) or move the assignment of
lastProcessedSeq = change.seq outside/above the continue check so it always
advances even for skipped docs (refer to lastProcessedSeq,
documentFilter.isReplicable(change.id), and change.seq).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.env.templatesrc/restricted-endpoints/replication/bulk-document/bulk-document.service.spec.tssrc/restricted-endpoints/replication/bulk-document/bulk-document.service.tssrc/restricted-endpoints/replication/changes/changes.controller.spec.tssrc/restricted-endpoints/replication/changes/changes.controller.tssrc/restricted-endpoints/replication/document-filter/document-filter.service.spec.tssrc/restricted-endpoints/replication/document-filter/document-filter.service.tssrc/restricted-endpoints/replication/replication.module.ts
…r.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
because we manage them client-side
Summary by CodeRabbit
New Features
Tests
Documentation