CBG-342 - Check for explicit KeyNotFound in gocb Incr's amt=0 handling (

#4095)

* CBG-342 Check for explicit KeyNotFoundError in gocbbucket.Incr

Prevents Incr from reporting incorrect zero value under cases where the
Get fails for other unexpected reasons (like persistent network timeouts)

* Add test to cover amt=0 in Incr for gocb bucket

* Use existing test bucket pattern for TestIncrAmtZero

* Make TestIncrAmtZero run on all bucket types - change def param to expected SG usage

base/bucket_gocb.go

@@ -1796,9 +1796,14 @@ func (bucket *CouchbaseBucketGoCB) Incr(k string, amt, def uint64, exp uint32) (
 	if amt == 0 {
 		var result uint64
 		_, err := bucket.Get(k, &result)
-		if err != nil {
+		if bucket.IsKeyNotFoundError(err) {
+			// Return Incr value as zero
 			return uint64(0), nil
+		} else if err != nil {
+			// Got an error when trying to fetch the value
+			return 0, err
 		}
+		// Got a non-zero value to return
 		return result, nil
 	}
 

base/bucket_gocb_test.go

@@ -473,6 +473,38 @@ func TestIncrCounter(t *testing.T) {
 
 }
 
+// TestIncrAmtZero covers the special handling in Incr for when amt=0 on an unknown key
+func TestIncrAmtZero(t *testing.T) {
+
+	testBucket := GetTestBucketOrPanic()
+	defer testBucket.Close()
+	bucket := testBucket.Bucket
+
+	key := "TestIncrAmtZero"
+
+	defer func() {
+		err := bucket.Delete(key)
+		if err != nil {
+			t.Errorf("Error removing counter from bucket")
+		}
+	}()
+
+	// key hasn't been created, so we'll fall into the special 'Get' handling in Incr
+	val, err := bucket.Incr(key, 0, 0, 0)
+	assert.NoError(t, err)
+	assert.Equal(t, uint64(0), val)
+
+	// Actually increment key to create it
+	val, err = bucket.Incr(key, 1, 1, 0)
+	assert.NoError(t, err)
+	assert.Equal(t, uint64(1), val)
+
+	// Do another amt=0 to make sure we get the new incremented value
+	val, err = bucket.Incr(key, 0, 0, 0)
+	assert.NoError(t, err)
+	assert.Equal(t, uint64(1), val)
+}
+
 func TestGetAndTouchRaw(t *testing.T) {
 
 	// There's no easy way to validate the expiry time of a doc (that I know of)

base/leaky_bucket.go

@@ -21,7 +21,7 @@ type LeakyBucket struct {
 // The config object that controls the LeakyBucket behavior
 type LeakyBucketConfig struct {
 
-	// Incr() fails 3 times before finally succeeding
+	// Incr() fails N times before finally succeeding
 	IncrTemporaryFailCount uint16
 
 	// Allows us to force a number of failed executions of GetDDoc, DeleteDDoc and DropIndex. It will fail the