CBG-4412: Add ability to pass custom import filter into diagnostic API #7935

RIT3shSapata · 2025-12-19T10:37:29Z

CBG-4412

Describe your PR here...

Upstream PR for CBG-4411
Adds ability to pass custom import filter into diagnostic API
Refactor unit tests

Pre-review checklist

Removed debug logging (fmt.Print, log.Print, ...)
Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Dependencies (if applicable)

Link upstream PRs
Update Go module dependencies when merged

Integration Tests

GSI=true,xattrs=true https://jenkins.sgwdev.com/job/SyncGatewayIntegration/208/
GSI=true,xattrs=true https://jenkins.sgwdev.com/job/SyncGatewayIntegration/211/

bbrks

Mostly looks good 👍

rest/diagnostic_doc_api.go

base/error.go

github-actions · 2025-12-24T11:27:13Z

Redocly previews

Copilot

Pull request overview

This PR adds the ability to pass a custom import filter function to the diagnostic API's /_import_filter endpoint, allowing users to test import filter behavior with different filter implementations without modifying the database configuration.

Key changes:

Changed the HTTP method from GET to POST for the /_import_filter endpoint to support request body
Added support for custom import filter functions via the request body
Refactored and expanded unit tests to cover various scenarios with and without custom import filters

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
rest/routing.go	Changed HTTP method from GET to POST for the `/_import_filter` endpoint
rest/diagnostic_doc_api_test.go	Removed old test function and added comprehensive table-driven tests for import filter dry run
rest/diagnostic_doc_api.go	Refactored handler to accept custom import filter in request body and improved error handling
docs/api/paths/diagnostic/keyspace-import_filter.yaml	Updated API specification to reflect POST method and document new request body parameters
db/import.go	Modified `ImportFilterDryRun` to accept custom import filter function and handle both default and custom filters
base/error.go	Added new error type `ImportFilterDryRunError` for import filter dry run errors

Copilot · 2025-12-24T11:27:50Z

base/error.go

 	// ErrInvalidJSON is returned when the JSON being unmarshalled cannot be parsed.
 	ErrInvalidJSON = HTTPErrorf(http.StatusBadRequest, "Invalid JSON")
+
+	ErrImportDryRun = &sgError{"Error occured during import dry run: "}


Corrected spelling of 'occured' to 'occurred'.

Suggested change

ErrImportDryRun = &sgError{"Error occured during import dry run: "}

ErrImportDryRun = &sgError{"Error occurred during import dry run: "}

Copilot · 2025-12-24T11:27:51Z

db/import.go

+		importOuput, err := importRunner.Call(ctx, doc)
+		switch result := importOuput.(type) {


Corrected spelling of 'importOuput' to 'importOutput'.

Suggested change

importOuput, err := importRunner.Call(ctx, doc)

switch result := importOuput.(type) {

importOutput, err := importRunner.Call(ctx, doc)

switch result := importOutput.(type) {

Copilot · 2025-12-24T11:27:51Z

db/import.go

+			break
+		case string:
+			boolResult, err := strconv.ParseBool(result)
+			if err != nil {
+				return false, err
 			}
-		} else {
-			return false, err
+			shouldImport = boolResult
+			break


The break statements are unnecessary in a switch statement in Go. The break is implicit at the end of each case unless a fallthrough is explicitly used.

Suggested change

break

case string:

boolResult, err := strconv.ParseBool(result)

if err != nil {

return false, err

}

} else {

return false, err

shouldImport = boolResult

break

case string:

boolResult, err := strconv.ParseBool(result)

if err != nil {

return false, err

}

shouldImport = boolResult

Copilot · 2025-12-24T11:27:52Z

db/import.go

+			break
+		case string:
+			boolResult, err := strconv.ParseBool(result)
+			if err != nil {
+				return false, err
 			}
-		} else {
-			return false, err
+			shouldImport = boolResult
+			break


The break statements are unnecessary in a switch statement in Go. The break is implicit at the end of each case unless a fallthrough is explicitly used.

Suggested change

break

case string:

boolResult, err := strconv.ParseBool(result)

if err != nil {

return false, err

}

} else {

return false, err

shouldImport = boolResult

break

case string:

boolResult, err := strconv.ParseBool(result)

if err != nil {

return false, err

}

shouldImport = boolResult

Copilot · 2025-12-24T11:27:52Z

db/import.go

+			shouldImport = boolResult
+			break
+		default:
+			return false, &base.ImportFilterDryRunError{Err: err}


The error err may be nil at this point since it's only assigned in the string case. This would result in wrapping a nil error. Consider returning an explicit error message indicating that the import filter returned a non-boolean value.

Suggested change

return false, &base.ImportFilterDryRunError{Err: err}

wrappedErr := err

if wrappedErr == nil {

wrappedErr = errors.New("import filter function returned non-boolean value")

}

return false, &base.ImportFilterDryRunError{Err: wrappedErr}

db/import.go

bbrks · 2025-12-24T14:58:35Z

db/import.go

+			break
+		case string:
+			boolResult, err := strconv.ParseBool(result)
+			if err != nil {
+				return false, err
 			}
-		} else {
-			return false, err
+			shouldImport = boolResult
+			break


bbrks · 2025-12-24T14:58:38Z

db/import.go

+		importOuput, err := importRunner.Call(ctx, doc)
+		switch result := importOuput.(type) {


bbrks · 2025-12-24T14:59:39Z

db/import.go

+		if err != nil {
+			return false, &base.ImportFilterDryRunError{Err: err}
+		}
+		shouldImport = output


If we return output here, we can avoid the nested } else { block below since we're returning early.

This is more idiomatic Go than writing if else chains and deep nesting.

RIT3shSapata self-assigned this Dec 19, 2025

RIT3shSapata marked this pull request as ready for review December 23, 2025 12:15

RIT3shSapata requested a review from bbrks December 23, 2025 12:15

RIT3shSapata assigned bbrks and unassigned RIT3shSapata Dec 23, 2025

bbrks requested changes Dec 23, 2025

View reviewed changes

rest/diagnostic_doc_api.go Outdated Show resolved Hide resolved

rest/diagnostic_doc_api.go Outdated Show resolved Hide resolved

rest/diagnostic_doc_api.go Outdated Show resolved Hide resolved

base/error.go Outdated Show resolved Hide resolved

RIT3shSapata requested a review from bbrks December 24, 2025 07:46

RIT3shSapata mentioned this pull request Dec 24, 2025

CBG-4945: Fix default sync function dry run #7937

Merged

6 tasks

Base automatically changed from CBG-4411 to main December 24, 2025 10:38

RIT3shSapata added 7 commits December 24, 2025 16:49

Merge branch 'main' into CBG-4411

ef36319

refactor import dry run to handle custom function and body

ba04a67

Merge branch 'CBG-4411' into CBG-4412

51c2252

refactor unit tests and error handling

47f555e

update API documentation and fix spelling

b8e7403

fixes based on PR comments

33de478

lint fix

bbc67b1

RIT3shSapata force-pushed the CBG-4412 branch from ee1f2a9 to bbc67b1 Compare December 24, 2025 11:27

Copilot AI review requested due to automatic review settings December 24, 2025 11:27

Copilot AI reviewed Dec 24, 2025

View reviewed changes

RIT3shSapata added 3 commits December 24, 2025 17:02

remove unused error

f09f031

refactor the unit tests for import filter dry run

6bf370d

handle no import filter scenario

1e8d8ef

bbrks requested changes Dec 24, 2025

View reviewed changes

RIT3shSapata added 3 commits December 29, 2025 11:38

Merge branch 'main' into CBG-4412

786b9fc

fixes based on pr comments and refactor import filter unit tests

4fd3879

add logging to import filter dry run

0584eb8

RIT3shSapata requested a review from bbrks December 29, 2025 10:04

remove invalid test case

7451981

	ErrImportDryRun = &sgError{"Error occured during import dry run: "}
	ErrImportDryRun = &sgError{"Error occurred during import dry run: "}

		importOuput, err := importRunner.Call(ctx, doc)
		switch result := importOuput.(type) {

-			return false, &base.ImportFilterDryRunError{Err: err}
+			wrappedErr := err
+			if wrappedErr == nil {
+				wrappedErr = errors.New("import filter function returned non-boolean value")
+			}
+			return false, &base.ImportFilterDryRunError{Err: wrappedErr}

CBG-4412: Add ability to pass custom import filter into diagnostic API #7935

Are you sure you want to change the base?

CBG-4412: Add ability to pass custom import filter into diagnostic API #7935

Conversation

RIT3shSapata commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-review checklist

Dependencies (if applicable)

Integration Tests

Uh oh!

bbrks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Redocly previews

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

bbrks Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

bbrks Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bbrks Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

bbrks Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

bbrks Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

RIT3shSapata commented Dec 19, 2025 •

edited

Loading

github-actions bot commented Dec 24, 2025 •

edited

Loading