Improve coverage of kallax package

Author: Miguel Molina

common_test.go

@@ -149,8 +149,9 @@ var ModelSchema = NewBaseSchema(
 	"__model",
 	f("id"),
 	ForeignKeys{
-		"rel":  NewForeignKey("model_id", false),
-		"rels": NewForeignKey("model_id", false),
+		"rel":     NewForeignKey("model_id", false),
+		"rels":    NewForeignKey("model_id", false),
+		"rel_inv": NewForeignKey("model_id", true),
 	},
 	func() Record {
 		return new(model)

events.go

@@ -66,11 +66,15 @@ func ApplyBeforeEvents(r Record) error {
 	}
 
 	if rec, ok := r.(BeforeInserter); ok && !r.IsPersisted() {
-		return rec.BeforeInsert()
+		if err := rec.BeforeInsert(); err != nil {
+			return err
+		}
 	}
 
 	if rec, ok := r.(BeforeUpdater); ok && r.IsPersisted() {
-		return rec.BeforeUpdate()
+		if err := rec.BeforeUpdate(); err != nil {
+			return err
+		}
 	}
 
 	return nil
@@ -92,7 +96,9 @@ func ApplyAfterEvents(r Record, wasPersisted bool) error {
 	}
 
 	if rec, ok := r.(AfterSaver); ok {
-		return rec.AfterSave()
+		if err := rec.AfterSave(); err != nil {
+			return err
+		}
 	}
 
 	return nil

events_test.go

@@ -15,11 +15,17 @@ type (
 	before struct {
 		model
 		evented
+		errorBeforeSave   bool
+		errorBeforeInsert bool
+		errorBeforeUpdate bool
 	}
 
 	after struct {
 		model
 		evented
+		errorAfterSave   bool
+		errorAfterInsert bool
+		errorAfterUpdate bool
 	}
 )
 
@@ -32,37 +38,55 @@ func (e *evented) setup() {
 func (b *before) BeforeInsert() error {
 	b.setup()
 	b.events["BeforeInsert"]++
+	if b.errorBeforeInsert {
+		return errors.New("foo")
+	}
 	return nil
 }
 
 func (b *before) BeforeUpdate() error {
 	b.setup()
 	b.events["BeforeUpdate"]++
-	return errors.New("foo")
+	if b.errorBeforeUpdate {
+		return errors.New("foo")
+	}
+	return nil
 }
 
 func (b *before) BeforeSave() error {
 	b.setup()
 	b.events["BeforeSave"]++
+	if b.errorBeforeSave {
+		return errors.New("foo")
+	}
 	return nil
 }
 
 func (b *after) AfterInsert() error {
 	b.setup()
 	b.events["AfterInsert"]++
+	if b.errorAfterInsert {
+		return errors.New("foo")
+	}
 	return nil
 }
 
 func (b *after) AfterUpdate() error {
 	b.setup()
 	b.events["AfterUpdate"]++
+	if b.errorAfterUpdate {
+		return errors.New("foo")
+	}
 	return nil
 }
 
 func (b *after) AfterSave() error {
 	b.setup()
 	b.events["AfterSave"]++
-	return errors.New("foo")
+	if b.errorAfterSave {
+		return errors.New("foo")
+	}
+	return nil
 }
 
 func TestApplyBeforeEvents(t *testing.T) {
@@ -71,21 +95,44 @@ func TestApplyBeforeEvents(t *testing.T) {
 	var before before
 	r.Nil(ApplyBeforeEvents(&before))
 	before.setPersisted()
-	r.NotNil(ApplyBeforeEvents(&before))
+	r.Nil(ApplyBeforeEvents(&before))
 
 	r.Equal(1, before.events["BeforeInsert"])
 	r.Equal(1, before.events["BeforeUpdate"])
 	r.Equal(2, before.events["BeforeSave"])
+
+	before.errorBeforeUpdate = true
+	r.NotNil(ApplyBeforeEvents(&before))
+
+	before.errorBeforeInsert = true
+	before.errorBeforeUpdate = false
+	before.persisted = false
+	r.NotNil(ApplyBeforeEvents(&before))
+
+	before.errorBeforeSave = true
+	before.errorBeforeInsert = false
+	r.NotNil(ApplyBeforeEvents(&before))
 }
 
 func TestApplyAfterEvents(t *testing.T) {
 	r := require.New(t)
 
 	var after after
-	r.NotNil(ApplyAfterEvents(&after, false))
-	r.NotNil(ApplyAfterEvents(&after, true))
+	r.Nil(ApplyAfterEvents(&after, false))
+	r.Nil(ApplyAfterEvents(&after, true))
 
 	r.Equal(1, after.events["AfterInsert"])
 	r.Equal(1, after.events["AfterUpdate"])
 	r.Equal(2, after.events["AfterSave"])
+
+	after.errorAfterUpdate = true
+	r.NotNil(ApplyAfterEvents(&after, true))
+
+	after.errorAfterInsert = true
+	after.errorAfterUpdate = false
+	r.NotNil(ApplyAfterEvents(&after, false))
+
+	after.errorAfterSave = true
+	after.errorAfterInsert = false
+	r.NotNil(ApplyAfterEvents(&after, false))
 }

model_test.go

@@ -15,6 +15,12 @@ func TestIDIsEmpty(t *testing.T) {
 	r.False(id.IsEmpty())
 }
 
+func TestID_Value(t *testing.T) {
+	id := NewID()
+	v, _ := id.Value()
+	require.Equal(t, id.String(), v)
+}
+
 func TestID_ThreeNewIDsAreDifferent(t *testing.T) {
 	r := require.New(t)
 	id1 := NewID()
@@ -32,6 +38,9 @@ func TestID_ThreeNewIDsAreDifferent(t *testing.T) {
 func TestVirtualColumn(t *testing.T) {
 	r := require.New(t)
 	record := newModel("", "", 0)
+	record.virtualColumns = nil
+	r.Equal(nil, record.VirtualColumn("foo"))
+
 	record.virtualColumns = nil
 	s := VirtualColumn("foo", record)
 
@@ -41,4 +50,6 @@ func TestVirtualColumn(t *testing.T) {
 	r.NoError(s.Scan(v))
 	r.Len(record.virtualColumns, 1)
 	r.Equal(id, record.VirtualColumn("foo"))
+
+	r.Error(s.Scan(nil))
 }

query.go

@@ -1,11 +1,18 @@
 package kallax
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/Masterminds/squirrel"
 )
 
+var (
+	// ErrManyToManyNotSupported is returned when a many to many relationship
+	// is added to a query.
+	ErrManyToManyNotSupported = errors.New("kallax: many to many relationships are not supported")
+)
+
 // Query returns information about some query settings and compiles the query.
 type Query interface {
 	compile() ([]string, squirrel.SelectBuilder)
@@ -156,7 +163,7 @@ func (q *BaseQuery) selectedColumns() []SchemaField {
 // be passed in the case of one to many relationships.
 func (q *BaseQuery) AddRelation(schema Schema, field string, typ RelationshipType, filter Condition) error {
 	if typ == ManyToMany {
-		return fmt.Errorf("kallax: many to many relationship are not supported, field: %s", field)
+		return ErrManyToManyNotSupported
 	}
 
 	fk, ok := q.schema.ForeignKey(field)

query_test.go

@@ -87,6 +87,20 @@ func (s *QuerySuite) TestAddRelation() {
 	s.Equal("SELECT __model.id, __model.name, __model.email, __model.age, __rel_rel.id, __rel_rel.model_id, __rel_rel.foo FROM model __model LEFT JOIN rel __rel_rel ON (__rel_rel.model_id = __model.id)", s.q.String())
 }
 
+func (s *QuerySuite) TestAddRelation_Inverse() {
+	s.Nil(s.q.AddRelation(RelSchema, "rel_inv", OneToOne, nil))
+	s.Equal("SELECT __model.id, __model.name, __model.email, __model.age, __rel_rel_inv.id, __rel_rel_inv.model_id, __rel_rel_inv.foo FROM model __model LEFT JOIN rel __rel_rel_inv ON (__rel_rel_inv.id = __model.model_id)", s.q.String())
+}
+
+func (s *QuerySuite) TestAddRelation_ManyToMany() {
+	err := s.q.AddRelation(RelSchema, "rel", ManyToMany, nil)
+	s.Equal(ErrManyToManyNotSupported, err)
+}
+
+func (s *QuerySuite) TestAddRelation_FKNotFound() {
+	s.Error(s.q.AddRelation(RelSchema, "fooo", OneToOne, nil))
+}
+
 func (s *QuerySuite) assertSql(sql string) {
 	_, builder := s.q.compile()
 	result, _, err := builder.ToSql()

resultset.go

@@ -3,7 +3,6 @@ package kallax
 import (
 	"database/sql"
 	"errors"
-	"fmt"
 	"io"
 
 	"github.com/src-d/go-kallax/types"
@@ -26,6 +25,10 @@ type ResultSet interface {
 // not allowed.
 var ErrRawScan = errors.New("kallax: result set comes from raw query, use RawScan instead")
 
+// ErrRawScanBatching is an error returned when the `RawScan` method is used
+// with a batching result set.
+var ErrRawScanBatching = errors.New("kallax: cannot perform a raw scan on a batching result set")
+
 // BaseResultSet is a generic collection of rows.
 type BaseResultSet struct {
 	relationships []Relationship
@@ -167,5 +170,5 @@ func (rs *BatchingResultSet) Close() error {
 // RawScan will always throw an error, as this is not a supported operation of
 // a batching result set.
 func (rs *BatchingResultSet) RawScan(_ ...interface{}) error {
-	return fmt.Errorf("kallax: cannot perform a raw scan on a batching result set")
+	return ErrRawScanBatching
 }

store.go

@@ -144,13 +144,8 @@ func (s *Store) Update(schema Schema, record Record, cols ...SchemaField) (int64
 	return cnt, nil
 }
 
-// Save inserts or updates the given record in the table. It requires a record
-// with non empty ID.
+// Save inserts or updates the given record in the table.
 func (s *Store) Save(schema Schema, record Record) (updated bool, err error) {
-	if record.GetID().IsEmpty() {
-		return false, ErrEmptyID
-	}
-
 	if !record.IsPersisted() {
 		return false, s.Insert(schema, record)
 	}
@@ -206,10 +201,6 @@ func (s *Store) RawExec(sql string, params ...interface{}) (int64, error) {
 // Find performs a query and returns a result set with the results.
 func (s *Store) Find(q Query) (ResultSet, error) {
 	rels := q.getRelationships()
-	if containsRelationshipOfType(rels, ManyToMany) {
-		return nil, fmt.Errorf("kallax: many to many relationships are not supported")
-	}
-
 	if containsRelationshipOfType(rels, OneToMany) {
 		return NewBatchingResultSet(newBatchQueryRunner(q.Schema(), s.proxy, q)), nil
 	}