Merge pull request #182 from dave-tucker/server-panic

cache: Return IndexExistsError immediately

cache/cache.go

@@ -111,7 +111,6 @@ func (r *RowCache) create(uuid string, m model.Model) error {
 		return err
 	}
 	newIndexes := newColumnToValue(r.schema.Indexes)
-	var errs []error
 	for index := range r.indexes {
 
 		val, err := valueFromIndex(info, index)
@@ -120,14 +119,11 @@ func (r *RowCache) create(uuid string, m model.Model) error {
 			return err
 		}
 		if existing, ok := r.indexes[index][val]; ok {
-			errs = append(errs,
-				NewIndexExistsError(r.name, val, index, uuid, existing))
+			return NewIndexExistsError(r.name, val, index, uuid, existing)
 		}
 		newIndexes[index][val] = uuid
 	}
-	if len(errs) != 0 {
-		return fmt.Errorf("%v", errs)
-	}
+
 	// write indexes
 	for k1, v1 := range newIndexes {
 		for k2, v2 := range v1 {

cache/cache_test.go

@@ -179,40 +179,46 @@ func TestRowCacheCreateMultiIndex(t *testing.T) {
 	tc, err := NewTableCache(&schema, db, testData)
 	require.Nil(t, err)
 	tests := []struct {
-		name    string
-		uuid    string
-		model   *testModel
-		wantErr bool
+		name               string
+		uuid               string
+		model              *testModel
+		wantErr            bool
+		wantIndexExistsErr bool
 	}{
 		{
 			"inserts a new row",
 			"foo",
 			&testModel{Foo: "foo", Bar: "foo"},
 			false,
+			false,
 		},
 		{
 			"error duplicate uuid",
 			"bar",
 			&testModel{Foo: "bar", Bar: "bar"},
 			true,
+			false,
 		},
 		{
 			"error duplicate index",
 			"baz",
 			&testModel{Foo: "foo", Bar: "foo"},
 			true,
+			true,
 		},
 		{
 			"new row with one duplicate value",
 			"baz",
 			&testModel{Foo: "foo", Bar: "bar"},
 			false,
+			false,
 		},
 		{
 			"new row with other duplicate value",
 			"quux",
 			&testModel{Foo: "bar", Bar: "baz"},
 			false,
+			false,
 		},
 	}
 	for _, tt := range tests {
@@ -222,6 +228,9 @@ func TestRowCacheCreateMultiIndex(t *testing.T) {
 			err := rc.Create(tt.uuid, tt.model)
 			if tt.wantErr {
 				assert.Error(t, err)
+				if tt.wantIndexExistsErr {
+					assert.IsType(t, &IndexExistsError{}, err)
+				}
 			} else {
 				assert.Nil(t, err)
 				mapperInfo, err := mapper.NewInfo(tSchema, tt.model)

server/server_integration_test.go

@@ -322,3 +322,58 @@ func TestClientServerInsertAndDelete(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Equal(t, 1, reply[0].Count)
 }
+
+func TestClientServerInsertDuplicate(t *testing.T) {
+	defDB, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{
+		"Open_vSwitch": &ovsType{},
+		"Bridge":       &bridgeType{}})
+	require.Nil(t, err)
+
+	schema, err := getSchema()
+	require.Nil(t, err)
+
+	ovsDB := NewInMemoryDatabase(map[string]*model.DBModel{"Open_vSwitch": defDB})
+	rand.Seed(time.Now().UnixNano())
+	tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000))
+	defer os.Remove(tmpfile)
+	server, err := NewOvsdbServer(ovsDB, DatabaseModel{
+		Model:  defDB,
+		Schema: schema,
+	})
+	assert.Nil(t, err)
+
+	go func(t *testing.T, o *OvsdbServer) {
+		if err := o.Serve("unix", tmpfile); err != nil {
+			t.Error(err)
+		}
+	}(t, server)
+	defer server.Close()
+	require.Eventually(t, func() bool {
+		return server.Ready()
+	}, 1*time.Second, 10*time.Millisecond)
+
+	ovs, err := client.NewOVSDBClient(defDB, client.WithEndpoint(fmt.Sprintf("unix:%s", tmpfile)))
+	require.NoError(t, err)
+	err = ovs.Connect(context.Background())
+	require.NoError(t, err)
+
+	bridgeRow := &bridgeType{
+		Name:        "foo",
+		ExternalIds: map[string]string{"go": "awesome", "docker": "made-for-each-other"},
+	}
+
+	ops, err := ovs.Create(bridgeRow)
+	require.Nil(t, err)
+	reply, err := ovs.Transact(ops...)
+	require.Nil(t, err)
+	_, err = ovsdb.CheckOperationResults(reply, ops)
+	require.Nil(t, err)
+
+	// duplicate
+	reply, err = ovs.Transact(ops...)
+	require.Nil(t, err)
+	opErrs, err := ovsdb.CheckOperationResults(reply, ops)
+	require.Error(t, err)
+	require.Error(t, opErrs[0])
+	require.IsTypef(t, &ovsdb.ConstraintViolation{}, opErrs[0], opErrs[0].Error())
+}