Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add function to check error on unique index. #74

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion db.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import (
"encoding/json"
"errors"
"fmt"

"reflect"
"regexp"

"github.com/surrealdb/surrealdb.go/pkg/websocket"
)
Expand Down Expand Up @@ -319,3 +319,14 @@ func isSlice(possibleSlice interface{}) bool {

return slice
}

// IsDuplicateUniqueIdx returns true if the error was caused by
// trying to create a record with a field that is duplicated
// in an unique index.
func IsDuplicateUniqueIdx(err error) bool {
if err == nil {
return false
}
match, _ := regexp.MatchString(`Database index .* already contains .*, with record .*`, err.Error())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ToDo:
Create ticket on SurrealDB project to include error code in error response.

return match
}
94 changes: 90 additions & 4 deletions db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/surrealdb/surrealdb.go"
gorilla "github.com/surrealdb/surrealdb.go/pkg/gorilla"
"github.com/surrealdb/surrealdb.go/pkg/gorilla"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ToDo:

Add missing dependencies to depgaurd.

"github.com/surrealdb/surrealdb.go/pkg/logger"
"github.com/surrealdb/surrealdb.go/pkg/websocket"
)
Expand Down Expand Up @@ -145,7 +145,7 @@ func (s *SurrealDBTestSuite) TestCreate() {

s.Run("Single create works", func() {
userData, err := s.db.Create("users", testUser{
Username: "johnny",
Username: "tim",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related to a test?
If not, please revert to original text.

Password: "123",
})
s.Require().NoError(err)
Expand All @@ -156,7 +156,7 @@ func (s *SurrealDBTestSuite) TestCreate() {
s.Require().NoError(err)
s.Len(userSlice, 1)

s.Equal("johnny", userSlice[0].Username)
s.Equal("tim", userSlice[0].Username)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

s.Equal("123", userSlice[0].Password)
})

Expand All @@ -165,7 +165,7 @@ func (s *SurrealDBTestSuite) TestCreate() {
data := make([]testUser, 0)
data = append(data,
testUser{
Username: "johnny",
Username: "lu",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Password: "123"},
testUser{
Username: "joe",
Expand Down Expand Up @@ -437,3 +437,89 @@ func assertContains[K fmt.Stringer](s *SurrealDBTestSuite, input []K, matcher fu
s.NotEmptyf(matching, "Input %+v did not contain matching element", fmt.Sprintf("%+v", input))
return matching
}

func (s *SurrealDBTestSuite) TestIsDuplicateUniqueIdx() {
query := `DEFINE INDEX uniqueNameIdx ON TABLE users COLUMNS username UNIQUE;`
_, err := s.db.Query(query, nil) // because of this query, I modified the 'username' field in other tests.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests should be independent. Please declare every table and data in this test.

s.Require().NoError(err)

userNoSpecialChars := testUser{
Username: "johnny",
Password: "1234",
}
data, err := s.db.Create("users", userNoSpecialChars) // true
s.Require().NoError(err)
s.Require().NotNil(data) // we collect this result, so we can get the record it to use it for other test

var createdUser []testUser
s.Require().NoError(surrealdb.Unmarshal(data, &createdUser))
s.Require().NotEmpty(createdUser[0].ID)

userWithSpecialChars := testUser{
Username: "jim",
Password: "abcd",
}
_, err = s.db.Create("users", userWithSpecialChars) // true
s.Require().NoError(err)

userWithObjectID := map[string]interface{}{
"username": struct {
Field1 string
}{
Field1: "nestedID",
},
"password": "4321",
}
_, err = s.db.Create("users", userWithObjectID)
s.Require().NoError(err)

// now, the actual tests...
msg := "the operation must fail"
s.Run("normal characters work", func() {
_, err = s.db.Create("users", userNoSpecialChars)
s.Require().NotNil(err, msg) // this validation is redundant, the function already checks for nil, but just in case
s.True(surrealdb.IsDuplicateUniqueIdx(err))
})
err = nil

s.Run("special characters work", func() {
_, err = s.db.Create("users", userWithSpecialChars)
s.Require().NotNil(err, msg)
s.True(surrealdb.IsDuplicateUniqueIdx(err))
})
err = nil

s.Run("records with objects as id work", func() {
_, err = s.db.Create("users", userWithObjectID)
s.Require().NotNil(err, msg)
s.True(surrealdb.IsDuplicateUniqueIdx(err))
})
err = nil

// try other typical errors
s.Run("it returns false on other errors", func() {
_, err = s.db.Create("users", testUser{ID: createdUser[0].ID})
s.Require().NotNil(err)
s.False(surrealdb.IsDuplicateUniqueIdx(err))

err = nil

_, err = s.db.Select("users:notexists")
s.Equal(err, surrealdb.ErrNoRow)
s.False(surrealdb.IsDuplicateUniqueIdx(err))
})
err = nil

// and finally, check with a tricky ID; this is the only way the function can give a false positive
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change this to an ignored testcase? That's because we want to verify important behaviour rather than verify implementation. Also it's also ok to have to a test in the comments rather than a working test.

trickyID := "Database index `userNameIdx` already contains 'Mark', with record `user:⟨2⟩`"
trickyUser := testUser{ID: trickyID}
_, err = s.db.Create("users", trickyUser)
s.Require().NoError(err)
s.Run("since the function works with regexp, this one should be true", func() {
_, err = s.db.Create("users", trickyUser) // we force a duplicate ID error
s.Require().NotNil(err)
// the matched string is contained in the error (although it's not the error itself,
// but the id of the record causing the problem), the function will give us a true.
s.True(surrealdb.IsDuplicateUniqueIdx(err))
})
}