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

(WIP)Implement Lab 07 : Custom Errors #664

Closed
wants to merge 3 commits into from
Closed
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
99 changes: 99 additions & 0 deletions 07_errors/sirigithub/dbstore.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package main

import (
"encoding/json"
"fmt"
"strconv"

"github.com/syndtr/goleveldb/leveldb"
)

type dbStore struct {
dbfilePath string
currID int
}

func NewdbStore(filePath string) *dbStore {

Choose a reason for hiding this comment

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

exported func NewdbStore returns unexported type *main.dbStore, which can be annoying to use (from golint)

var store = dbStore{}
store.dbfilePath = filePath
return &store
Copy link
Contributor

Choose a reason for hiding this comment

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

You can implement this function with a single statement:

return &dbStore{dbfilePath: filePath}

However, it would probably be better to open the database when the dbStore is created, and add a Close() error method so the caller can close it later (which implements the io.Closer interface:

type dbStore struct {
  conn *db.leveldb.DB
  currID int
}

func NewdbStore(filePath string) *dbStore {
    return &dbStore{conn: dbConn(filePath)} // one day I'll handle errors here :-)
}

func (s *dbStore) Close() error {
    return s.conn.Close()
}

But you'll also need a different approach for currID. Because the database is persistent, if the process restarts, you start allocating IDs from 1 again. These will conflict with existing IDs.

A different approach is to use github.com/google/uuid to generate random IDs. That way do you not need to maintain any state to allocate IDs.

}
func (s *dbStore) CreatePuppy(puppy *Puppy) (int, error) {
if puppy.Value < 0 {
return -1, NewError(ErrInvalidValue, "Puppy value must be greater than 0")
}
s.currID++
puppy.ID = s.currID
db := dbConn(s.dbfilePath)
defer db.Close()

value := getPuppyData(puppy)
err := db.Put([]byte(strconv.Itoa(puppy.ID)), value, nil)
if err != nil {
return -1, NewError(ErrDatabaseWrite, fmt.Sprintf("Error creating Puppy %d in the dbStore", puppy.ID))
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally you always want to wrap errors so you don't lose any detail so I'd write NewError() to take third error parameter (which can be nil) and store that error in your error type. See more detail below.

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use capital letters to start error strings: https://github.com/golang/go/wiki/CodeReviewComments#error-strings

}
return puppy.ID, nil
}

// ReadPuppy reads an existing puppy from the store
func (s *dbStore) ReadPuppy(id int) (*Puppy, error) {
db := dbConn(s.dbfilePath)
defer db.Close()

data, err := db.Get([]byte(strconv.Itoa(id)), nil)
if err != nil {
return nil, NewError(ErrIDNotFound, fmt.Sprintf("puppy ID %d does not exist in the database", id))
}

var p Puppy
e := json.Unmarshal(data, &p)
if e != nil {
return nil, NewError(ErrUrmarshallData, fmt.Sprintf("Cannot marshall puppy value"))
}
return &p, nil
}
func (s *dbStore) UpdatePuppy(p *Puppy) error {
db := dbConn(s.dbfilePath)
defer db.Close()

_, err := db.Get([]byte(strconv.Itoa(p.ID)), nil)
if err != nil {
println("error getting value from db")
return NewError(ErrIDNotFound, fmt.Sprintf("puppy ID %d to update does not exist in the databse", p.ID))
}
value, err := json.Marshal(p)
if err != nil {
return NewError(ErrMarshallData, fmt.Sprintf("Cannot marshall puppy %d", p.ID))
}
e := db.Put([]byte(strconv.Itoa(p.ID)), value, nil)
if e != nil {
return NewError(ErrDatabaseWrite, fmt.Sprintf("Error writing Puppy %d to the databse", p.ID))
}
return nil
}

func (s *dbStore) DeletePuppy(id int) error {
db := dbConn(s.dbfilePath)
defer db.Close()
err := db.Delete([]byte(strconv.Itoa(id)), nil)
if err != nil {
return NewError(ErrDatabseDelete, fmt.Sprintf("Error deleting Puppy %d from the databse", id))
}
return nil
}

func dbConn(dbfilePath string) (db *leveldb.DB) {

Choose a reason for hiding this comment

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

dbConn - dbfilePath is unused (from unparam)

db, err := leveldb.OpenFile("/tmp/foo.db", nil)
if err != nil {
panic(err)
}
return db
}

func getPuppyData(puppy *Puppy) []byte {
value, err := json.Marshal(puppy)
if err != nil {
panic(err)
}
return value
}
104 changes: 104 additions & 0 deletions 07_errors/sirigithub/dbstore_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package main

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
)

type dbstoresSuite struct {
suite.Suite
store Storer
newStorer func() Storer
puppy1 Puppy
}

func (suite *dbstoresSuite) SetupTest() {
suite.store = suite.newStorer()
suite.puppy1 = Puppy{Breed: "cavalier", Color: "White", Value: 1700}
_, err := suite.store.CreatePuppy(&suite.puppy1)
if err != nil {
suite.FailNow("Test setup failed to create puppy", err)
}

}
func (suite *dbstoresSuite) TestCreatePuppySuccess() {
//given
assert := assert.New(suite.T())
newPuppy := Puppy{Breed: "Dobberman", Color: "Black", Value: 800}
//when
id, err := suite.store.CreatePuppy(&newPuppy)
//then
assert.NoError(err, "Puppy creation should be successful")
assert.True(newPuppy.ID == suite.puppy1.ID+1, "Puppy Id increments sequentially")
puppy, err := suite.store.ReadPuppy(id)
if assert.NoError(err, "Read of created puppy should be successful") {
assert.Equal(puppy, &newPuppy)
}
}

func (suite *dbstoresSuite) TestCreatePuppyNegativeValue() {
//given
assert := assert.New(suite.T())
newPuppy := Puppy{Breed: "New", Color: "Black", Value: -1}
//when
_, err := suite.store.CreatePuppy(&newPuppy)
//then
assert.Error(err, "Negative Value should throw an error")
}
func (suite *dbstoresSuite) TestReadPuppySuccess() {
//given
assert := assert.New(suite.T())
//when
puppy, err := suite.store.ReadPuppy(suite.puppy1.ID)
//then
if assert.NoError(err, "Read of puppy should be successful") {
assert.Equal(puppy, &suite.puppy1)
}
}

func (suite *dbstoresSuite) TestReadNonExistingPuppy() {
assert := assert.New(suite.T())
_, err := suite.store.ReadPuppy(100)
assert.Error(err, "Read of non existing puppy Id should throw an error")
}

func (suite *dbstoresSuite) TestUpdatePuppySuccess() {
//given
assert := assert.New(suite.T())
existingPuppy := suite.puppy1
//when
existingPuppy.Color = "Brown"
existingPuppy.Breed = "Dobberman"
err := suite.store.UpdatePuppy(&existingPuppy)
//then
assert.NoError(err)
updatedPuppy, err := suite.store.ReadPuppy(existingPuppy.ID)
assert.NoError(err)
assert.NotEqual(updatedPuppy, &suite.puppy1, "Puppy values are updated")
}

func (suite *dbstoresSuite) TestUpdateNonExistingPuppy() {
assert := assert.New(suite.T())
puppy := Puppy{ID: 100, Breed: "cavalier", Color: "White", Value: 1700}
err := suite.store.UpdatePuppy(&puppy)
assert.Error(err, "Update of non existing puppy Id should throw an error")
}
func (suite *dbstoresSuite) TestDeletePuppySuccess() {
assert := assert.New(suite.T())
//when
err := suite.store.DeletePuppy(suite.puppy1.ID)
//then
assert.NoError(err, "Delete should successfully delete existing puppy")
_, err = suite.store.ReadPuppy(suite.puppy1.ID)
assert.Error(err, "Read of non existing puppy Id should throw an error")
}
func (suite *dbstoresSuite) TestDeleteNonExistingPuppy() {
assert := assert.New(suite.T())
err := suite.store.DeletePuppy(12)
assert.NoError(err, "Delete non existing puppy does not throw error")
}
func TestDBSuite(t *testing.T) {
suite.Run(t, &dbstoresSuite{newStorer: func() Storer { return NewdbStore("/tmp/test25.db") }})
}
26 changes: 26 additions & 0 deletions 07_errors/sirigithub/error.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package main

import "fmt"

type Error struct {
Message string
Code int
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you will often wrap other errors, I would include an err error field here too so you can store the error returned from leveldb.


const (
ErrInvalidValue = 400
ErrIDNotFound = 404
ErrMarshallData = 400
ErrUrmarshallData = 400
ErrDatabaseConn = 500

Choose a reason for hiding this comment

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

ErrDatabaseConn is unused (from deadcode)

ErrDatabaseWrite = 500
ErrDatabaseRead = 500

Choose a reason for hiding this comment

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

ErrDatabaseRead is unused (from deadcode)

ErrDatabseDelete = 500
)

func (e *Error) Error() string {
return fmt.Sprintf("Error code %d: %s", e.Code, e.Message)
Copy link
Contributor

Choose a reason for hiding this comment

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

with the err field, this would be fmt.Sprintf("error code %d: %s: %v", e.Code, e.Message, e.err)

Note that I've made the first letter lower case as per https://github.com/golang/go/wiki/CodeReviewComments#error-strings

}
func NewError(errCode int, msg string) *Error {
return &Error{msg, errCode}
}
21 changes: 21 additions & 0 deletions 07_errors/sirigithub/errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package main

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestInvalidValue(t *testing.T) {
err := NewError(ErrInvalidValue, "test invalid value")
assert.Equal(t, ErrInvalidValue, err.Code)
errMessage := err.Error()
assert.Equal(t, "Error code 400: test invalid value", errMessage)
}

func TestIDNotFound(t *testing.T) {
err := NewError(ErrIDNotFound, "test id not found")
assert.Equal(t, ErrIDNotFound, err.Code)
errMessage := err.Error()
assert.Equal(t, "Error code 404: test id not found", errMessage)
}
24 changes: 24 additions & 0 deletions 07_errors/sirigithub/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package main

import (
"fmt"
"io"
"os"
)

var out io.Writer = os.Stdout

func main() {
mapStore := NewMapStore()
puppy1 := Puppy{Breed: "Mix", Color: "White", Value: 300}
id, _ := mapStore.CreatePuppy(&puppy1)
println(id)
syncStore := NewSyncStore()
id2, _ := syncStore.CreatePuppy(&puppy1)
println(id2)

p1, _ := mapStore.ReadPuppy(id)
fmt.Fprintln(out, "Read puppy from Mapstore with ID:", p1.ID)
p2, _ := syncStore.ReadPuppy(id2)
fmt.Fprintln(out, "Read puppy from SyncStore with ID:", p2.ID)
}
17 changes: 17 additions & 0 deletions 07_errors/sirigithub/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package main

import (
"bytes"
"testing"

"github.com/stretchr/testify/assert"
)

func TestMain(t *testing.T) {
var buf bytes.Buffer
out = &buf
main()
expected := "Read puppy from Mapstore with ID: 0\nRead puppy from SyncStore with ID: 0\n"
actual := buf.String()
assert.Equal(t, expected, actual, "Expected and actual values should be the same.")
}
49 changes: 49 additions & 0 deletions 07_errors/sirigithub/mapstore.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package main

import (
"fmt"
)

type MapStore map[int]Puppy

func NewMapStore() MapStore {
return MapStore{}
}

// CreatePuppy takes a user provided puppy, and creates a new Puppy in the store
// returns the ID of the new Puppy.
func (m MapStore) CreatePuppy(puppy *Puppy) (int, error) {
if puppy.Value < 0 {
return -1, NewError(ErrIDNotFound, "Puppy value must be greater than 0")
}
puppy.ID = len(m)
m[puppy.ID] = *puppy
return puppy.ID, nil
}

// UpdatePuppy overrides an existing puppy in the store,
// returns an error if id is not found or does not match the puppy ID
func (m MapStore) UpdatePuppy(p *Puppy) error {
if _, ok := m[p.ID]; !ok {
return NewError(ErrIDNotFound, fmt.Sprintf("puppy ID %d to update does not exist in the map", p.ID))
}
m[p.ID] = *p
return nil
}

// DeletePuppy deletes an existing puppy from the store
func (m MapStore) DeletePuppy(id int) error {
if _, ok := m[id]; ok {
delete(m, id)
return nil
}
return NewError(ErrIDNotFound, fmt.Sprintf("puppy ID %d does not exist in the map", id))
}

// ReadPuppy reads an existing puppy from the store
func (m MapStore) ReadPuppy(id int) (*Puppy, error) {
if puppy, ok := m[id]; ok {
return &puppy, nil
}
return nil, NewError(ErrIDNotFound, fmt.Sprintf("puppy ID %d does not exist in the map", id))
}
Loading