CBG-3697 make sure on demand import errors show up as 404 (#6637)

* write test

* Change validation

* CBG-3697 make sure on demand import errors show up as 404

- The result of this is that blip will send a NoRev message since this
revision is not accessible.

- Delete validation in validateImportBody rather than change the type to
  report 404 since the only caller does not pass a nil pointer into this
  function.

base/error.go

@@ -41,7 +41,7 @@ var (
 	ErrAlreadyExists         = &sgError{"Already exists"}
 	ErrNotFound              = &sgError{"Not Found"}
 	ErrUpdateCancel          = &sgError{"Cancel update"}
-	ErrImportCancelledPurged = &sgError{"Import Cancelled Due to Purge"}
+	ErrImportCancelledPurged = HTTPErrorf(http.StatusNotFound, "Import Cancelled Due to Purge")
 	ErrChannelFeed           = &sgError{"Error while building channel feed"}
 	ErrTimeout               = &sgError{"Operation timed out"}
 	ErrPathNotFound          = sgbucket.ErrPathNotFound

db/import.go

@@ -14,6 +14,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/http"
 	"strconv"
 	"time"
 
@@ -40,7 +41,7 @@ func (db *DatabaseCollectionWithUser) ImportDocRaw(ctx context.Context, docid st
 		err := body.Unmarshal(value)
 		if err != nil {
 			base.InfofCtx(ctx, base.KeyImport, "Unmarshal error during importDoc %v", err)
-			return nil, err
+			return nil, base.HTTPErrorf(http.StatusNotFound, "Error unmarshalling %s: %s", base.UD(docid).Redact(), err)
 		}
 
 		err = validateImportBody(body)

db/validation.go

@@ -51,10 +51,6 @@ func validateAPIDocUpdate(body Body) error {
 
 // validateImportBody validates incoming import bodies
 func validateImportBody(body Body) error {
-	if body == nil {
-		return base.ErrEmptyDocument
-	}
-
 	if isPurged, ok := body[BodyPurged].(bool); ok && isPurged {
 		return base.ErrImportCancelledPurged
 	}
@@ -63,7 +59,7 @@ func validateImportBody(body Body) error {
 	disallowed := []string{BodyId, BodyRev, BodyExpiry, BodyRevisions}
 	for _, prop := range disallowed {
 		if _, ok := body[prop]; ok {
-			return base.HTTPErrorf(http.StatusBadRequest, "top-level property '"+prop+"' is a reserved internal property therefore cannot be imported")
+			return base.HTTPErrorf(http.StatusNotFound, "top-level property '"+prop+"' is a reserved internal property therefore cannot be imported")
 		}
 	}
 	// TODO: Validate attachment data to ensure user is not setting invalid attachments
@@ -80,7 +76,7 @@ func validateBlipBody(ctx context.Context, rawBody []byte, doc *Document) error
 		// Only unmarshal if raw body contains the disallowed property
 		if bytes.Contains(rawBody, []byte(`"`+prop+`"`)) {
 			if _, ok := doc.Body(ctx)[prop]; ok {
-				return base.HTTPErrorf(http.StatusBadRequest, "top-level property '"+prop+"' is a reserved internal property")
+				return base.HTTPErrorf(http.StatusNotFound, "top-level property '"+prop+"' is a reserved internal property")
 			}
 		}
 	}

rest/blip_api_crud_test.go

@@ -2887,3 +2887,82 @@ func TestBlipRefreshUser(t *testing.T) {
 		require.NotContains(t, string(body), "Panic:")
 	})
 }
+
+func TestOnDemandImportBlipFailure(t *testing.T) {
+	base.SetUpTestLogging(t, base.LevelDebug, base.KeyHTTP, base.KeySync, base.KeySyncMsg)
+	btcRunner := NewBlipTesterClientRunner(t)
+	btcRunner.Run(func(t *testing.T, SupportedBLIPProtocols []string) {
+		rt := NewRestTester(t, &RestTesterConfig{PersistentConfig: true, GuestEnabled: true})
+		defer rt.Close()
+		config := rt.NewDbConfig()
+		config.AutoImport = false
+		RequireStatus(t, rt.CreateDatabase("db", config), http.StatusCreated)
+		testCases := []struct {
+			name        string
+			invalidBody []byte
+		}{
+			{
+				name:        "_id property",
+				invalidBody: []byte(`{"_id": "doc1"}`),
+			},
+			{
+				name:        "_exp property",
+				invalidBody: []byte(`{"_exp": 1}`),
+			},
+			{
+				name:        "_rev property",
+				invalidBody: []byte(`{"_rev": "abc1"}`),
+			},
+			{
+				name:        "_revisions property",
+				invalidBody: []byte(`{"_revisions": {"start": 0, "ids": ["foo", "def]"}}`),
+			},
+
+			{
+				name:        "_purged property",
+				invalidBody: []byte(`{"_purged": true}`),
+			},
+			{
+				name:        "invalid json",
+				invalidBody: []byte(``),
+			},
+		}
+		for i, testCase := range testCases {
+			t.Run(testCase.name, func(t *testing.T) {
+				docID := fmt.Sprintf("doc%d,", i)
+				markerDoc := fmt.Sprintf("markerDoc%d", i)
+				validBody := `{"foo":"bar"}`
+				_ = rt.PutDoc(docID, validBody)
+				btc := btcRunner.NewBlipTesterClientOptsWithRT(rt, &BlipTesterClientOpts{
+					Username:               "user",
+					Channels:               []string{"*"},
+					SupportedBLIPProtocols: SupportedBLIPProtocols,
+				})
+				defer btc.Close()
+				require.NoError(t, btcRunner.StartOneshotPull(btc.id))
+
+				output, found := btcRunner.WaitForDoc(btc.id, docID)
+				require.True(t, found)
+				require.JSONEq(t, validBody, string(output))
+
+				err := rt.GetSingleDataStore().SetRaw(docID, 0, nil, testCase.invalidBody)
+				require.NoError(t, err)
+
+				rt.CreateTestDoc(markerDoc)
+
+				rt.GetSingleTestDatabaseCollection().FlushRevisionCacheForTest()
+
+				btc2 := btcRunner.NewBlipTesterClientOptsWithRT(rt, &BlipTesterClientOpts{
+					Username:               "user",
+					Channels:               []string{"*"},
+					SupportedBLIPProtocols: SupportedBLIPProtocols,
+				})
+				defer btc2.Close()
+
+				require.NoError(t, btcRunner.StartOneshotPull(btc2.id))
+
+				btcRunner.WaitForDoc(btc2.id, markerDoc)
+			})
+		}
+	})
+}

rest/importtest/import_test.go

@@ -2196,7 +2196,7 @@ func TestImportInternalPropertiesHandling(t *testing.T) {
 			name:               "Valid _id",
 			importBody:         map[string]interface{}{"_id": "documentid"},
 			expectReject:       true,
-			expectedStatusCode: base.IntPtr(400),
+			expectedStatusCode: base.IntPtr(http.StatusNotFound),
 		},
 		{
 			name:         "Valid _rev",