From 1486110d2e020ef2d9b344848e8cf07ec63dd2ce Mon Sep 17 00:00:00 2001 From: Jim Myhrberg Date: Sun, 4 Dec 2016 19:16:57 +0000 Subject: [PATCH 1/3] Noramlize Store interface Instead of exposing a key/value get/set style interface, design the interface around the actions that are actually required. This is a first step towards supporting adding a Store to support Google Cloud's Datastore. --- storage/goleveldbstore/store.go | 120 ++++++++++++++--- storage/goleveldbstore/store_test.go | 190 +++++++++++++++++---------- storage/inmemorystore/store.go | 81 ++++++++---- storage/inmemorystore/store_test.go | 164 ++++++++++++++++------- storage/store.go | 21 ++- 5 files changed, 413 insertions(+), 163 deletions(-) diff --git a/storage/goleveldbstore/store.go b/storage/goleveldbstore/store.go index 7725b1b..de4d419 100644 --- a/storage/goleveldbstore/store.go +++ b/storage/goleveldbstore/store.go @@ -1,17 +1,19 @@ package goleveldbstore import ( - "errors" "strconv" + "github.com/jimeh/ozu.io/storage" "github.com/syndtr/goleveldb/leveldb" ) +const errLeveldbNotFound = "leveldb: not found" + // DefaultSequenceKey is used by NextSequence(). var DefaultSequenceKey = []byte("__SEQUENCE_ID__") -// ErrNotFound is returned when Get() tries to fetch a non-existent key. -var ErrNotFound = errors.New("not found") +var uidKeyPrefix = []byte("!") +var urlKeyPrefix = []byte("#") // New creates a new Store using given path to persist data. func New(path string) (*Store, error) { @@ -39,33 +41,117 @@ func (s *Store) Close() error { return s.DB.Close() } -// Get a given key's value. -func (s *Store) Get(key []byte) ([]byte, error) { - value, err := s.DB.Get(key, nil) - if err != nil && err.Error() == "leveldb: not found" { - return nil, ErrNotFound +// Create a given Record. +func (s *Store) Create(uid []byte, url []byte) (*storage.Record, error) { + tx, err := s.DB.OpenTransaction() + if err != nil { + return &storage.Record{}, err } - return value, err + err = tx.Put(s.uidKey(uid), url, nil) + if err != nil { + return &storage.Record{}, err + } + + err = tx.Put(s.urlKey(url), uid, nil) + if err != nil { + return &storage.Record{}, err + } + + err = tx.Commit() + if err != nil { + return &storage.Record{}, err + } + + return &storage.Record{UID: uid, URL: url}, nil } -// Set a given key's to the specified value. -func (s *Store) Set(key []byte, value []byte) error { - return s.DB.Put(key, value, nil) +// FindByUID looks up records based on their UID. +func (s *Store) FindByUID(uid []byte) (*storage.Record, error) { + value, err := s.DB.Get(s.uidKey(uid), nil) + if err != nil { + if err.Error() == errLeveldbNotFound { + return &storage.Record{}, storage.ErrNotFound + } + return &storage.Record{}, err + } + + return &storage.Record{UID: uid, URL: value}, nil } -// Delete a given key. -func (s *Store) Delete(key []byte) error { - return s.DB.Delete(key, nil) +// FindByURL looks up records based on their URL. +func (s *Store) FindByURL(url []byte) (*storage.Record, error) { + value, err := s.DB.Get(s.urlKey(url), nil) + if err != nil { + if err.Error() == errLeveldbNotFound { + return &storage.Record{}, storage.ErrNotFound + } + return &storage.Record{}, err + } + + return &storage.Record{UID: value, URL: url}, nil +} + +// DeleteByUID deletes records based on their UID. +func (s *Store) DeleteByUID(uid []byte) (*storage.Record, error) { + record, err := s.FindByUID(uid) + if err != nil { + return &storage.Record{}, err + } + + s.delete(record) + return record, nil +} + +// DeleteByURL deletes records based on their URL. +func (s *Store) DeleteByURL(url []byte) (*storage.Record, error) { + record, err := s.FindByURL(url) + if err != nil { + return &storage.Record{}, err + } + + err = s.delete(record) + if err != nil { + return &storage.Record{}, err + } + + return record, nil +} + +func (s *Store) delete(r *storage.Record) error { + tx, err := s.DB.OpenTransaction() + if err != nil { + return err + } + + err = tx.Delete(s.uidKey(r.UID), nil) + if err != nil && err.Error() == errLeveldbNotFound { + return err + } + + err = tx.Delete(s.urlKey(r.URL), nil) + if err != nil && err.Error() == errLeveldbNotFound { + return err + } + + return tx.Commit() +} + +func (s *Store) uidKey(uid []byte) []byte { + return append(uidKeyPrefix, uid...) +} + +func (s *Store) urlKey(url []byte) []byte { + return append(urlKeyPrefix, url...) } // NextSequence returns a auto-incrementing int. func (s *Store) NextSequence() (int, error) { - return s.Incr(s.SequenceKey) + return s.incr(s.SequenceKey) } // Incr increments a given key (must be numeric-like value) -func (s *Store) Incr(key []byte) (int, error) { +func (s *Store) incr(key []byte) (int, error) { tx, err := s.DB.OpenTransaction() if err != nil { return -1, err diff --git a/storage/goleveldbstore/store_test.go b/storage/goleveldbstore/store_test.go index d17ad0f..b7e2fa7 100644 --- a/storage/goleveldbstore/store_test.go +++ b/storage/goleveldbstore/store_test.go @@ -13,13 +13,10 @@ import ( var testDbPath = "./goleveldb_test_data" -var examples = []struct { - key []byte - value []byte -}{ - {key: []byte("hello"), value: []byte("world")}, - {key: []byte("foo"), value: []byte("bar")}, - {key: []byte("wtf"), value: []byte("dude")}, +var examples = []storage.Record{ + storage.Record{UID: []byte("Kb8X"), URL: []byte("https://google.com/")}, + storage.Record{UID: []byte("h3mz"), URL: []byte("https://github.com/")}, + storage.Record{UID: []byte("3qxs"), URL: []byte("https://twitter.com/")}, } type StoreSuite struct { @@ -42,7 +39,9 @@ func (s *StoreSuite) TearDownTest() { func (s *StoreSuite) Seed() { for _, e := range examples { - err := s.db.Put(e.key, e.value, nil) + err := s.db.Put(append(uidKeyPrefix, e.UID...), e.URL, nil) + s.Require().NoError(err) + err = s.db.Put(append(urlKeyPrefix, e.URL...), e.UID, nil) s.Require().NoError(err) } } @@ -53,62 +52,112 @@ func (s *StoreSuite) TestStoreInterface() { s.Implements(new(storage.Store), new(Store)) } -func (s *StoreSuite) TestSet() { +func (s *StoreSuite) TestCreate() { for _, e := range examples { - err := s.store.Set(e.key, e.value) + record, err := s.store.Create(e.UID, e.URL) s.NoError(err) + s.Equal(e.UID, record.UID) + s.Equal(e.URL, record.URL) } for _, e := range examples { - result, _ := s.db.Get(e.key, nil) - s.Equal(e.value, result) + recordURL, _ := s.db.Get(append(uidKeyPrefix, e.UID...), nil) + s.Equal(e.URL, recordURL) + recordUID, _ := s.db.Get(append(urlKeyPrefix, e.URL...), nil) + s.Equal(e.UID, recordUID) } } -func (s *StoreSuite) TestGetExisting() { +func (s *StoreSuite) TestFindExistingByUID() { s.Seed() for _, e := range examples { - result, err := s.store.Get(e.key) + record, err := s.store.FindByUID(e.UID) s.NoError(err) - s.Equal(e.value, result) + s.Equal(e.UID, record.UID) + s.Equal(e.URL, record.URL) } } -func (s *StoreSuite) TestGetNonExistant() { - result, err := s.store.Get([]byte("does-not-exist")) - s.Nil(result) +func (s *StoreSuite) TestFindNonExistantByUID() { + record, err := s.store.FindByUID([]byte("nope")) + s.Nil(record.UID) + s.Nil(record.URL) s.EqualError(err, "not found") } -func (s *StoreSuite) TestDeleteExisting() { +func (s *StoreSuite) TestFindExistingByURL() { s.Seed() for _, e := range examples { - value, _ := s.db.Get(e.key, nil) - s.Require().Equal(e.value, value) - - value, err := s.store.Get(e.key) - s.Require().NoError(err) - s.Require().Equal(value, e.value) - - err = s.store.Delete(e.key) + record, err := s.store.FindByURL(e.URL) s.NoError(err) - - value, err = s.store.Get(e.key) - s.Nil(value) - s.EqualError(err, "not found") - - has, _ := s.db.Has(e.key, nil) - s.Equal(false, has) - result, _ := s.db.Get(e.key, nil) - s.Equal([]byte{}, result) + s.Equal(e.UID, record.UID) + s.Equal(e.URL, record.URL) } } -func (s *StoreSuite) TestDeleteNonExistant() { - err := s.store.Delete([]byte("does-not-exist")) - s.NoError(err) +func (s *StoreSuite) TestFindNonExistantByURL() { + record, err := s.store.FindByURL([]byte("http://nope.com/")) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") +} + +func (s *StoreSuite) TestDeleteExistingByUID() { + s.Seed() + + for _, e := range examples { + record, err := s.store.DeleteByUID(e.UID) + s.NoError(err) + s.Equal(record.UID, e.UID) + s.Equal(record.URL, e.URL) + + record, err = s.store.FindByUID(e.UID) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") + + record, err = s.store.FindByURL(e.URL) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") + } +} + +func (s *StoreSuite) TestDeleteNonExistantByUID() { + record, err := s.store.DeleteByUID([]byte("nope")) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") +} + +func (s *StoreSuite) TestDeleteExistingByURL() { + s.Seed() + + for _, e := range examples { + record, err := s.store.DeleteByURL(e.URL) + s.NoError(err) + s.Equal(record.UID, e.UID) + s.Equal(record.URL, e.URL) + + record, err = s.store.FindByUID(e.UID) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") + + record, err = s.store.FindByURL(e.URL) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") + } +} + +func (s *StoreSuite) TestDeleteNonExistantByURL() { + record, err := s.store.DeleteByURL([]byte("http://nope/")) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") } func (s *StoreSuite) TestNextSequenceExisting() { @@ -135,14 +184,14 @@ func (s *StoreSuite) TestIncrExisting() { err := s.db.Put(key, []byte("5"), nil) s.Require().NoError(err) - result, err := s.store.Incr(key) + result, err := s.store.incr(key) s.NoError(err) s.Equal(6, result) } func (s *StoreSuite) TestIncrNonExistant() { for i := 1; i < 10; i++ { - result, err := s.store.Incr([]byte("counter")) + result, err := s.store.incr([]byte("counter")) s.NoError(err) s.Equal(i, result) @@ -157,54 +206,57 @@ func TestStoreSuite(t *testing.T) { // Benchmarks -func BenchmarkGet(b *testing.B) { +func BenchmarkCreate(b *testing.B) { store, _ := New(testDbPath) - key := []byte("hello") - value := []byte("world") - _ = store.Set(key, value) + uid := []byte("Kb8X") + url := []byte("https://google.com/") for n := 0; n < b.N; n++ { - _, _ = store.Get(key) + store.Create(append(uid, string(n)...), url) } - _ = store.Close() - _ = os.RemoveAll(testDbPath) + store.Close() + os.RemoveAll(testDbPath) } -func BenchmarkSet(b *testing.B) { +func BenchmarkFindByUID(b *testing.B) { store, _ := New(testDbPath) - key := []byte("hello") - value := []byte("world") + uid := []byte("Kb8X") + url := []byte("https://google.com/") + store.Create(uid, url) for n := 0; n < b.N; n++ { - _ = store.Set(append(key, string(n)...), value) + store.FindByUID(uid) } - _ = store.Close() - _ = os.RemoveAll(testDbPath) + store.Close() + os.RemoveAll(testDbPath) +} + +func BenchmarkFindByURL(b *testing.B) { + store, _ := New(testDbPath) + + uid := []byte("Kb8X") + url := []byte("https://google.com/") + store.Create(uid, url) + + for n := 0; n < b.N; n++ { + store.FindByURL(url) + } + + store.Close() + os.RemoveAll(testDbPath) } func BenchmarkNextSequence(b *testing.B) { store, _ := New(testDbPath) for n := 0; n < b.N; n++ { - _, _ = store.NextSequence() + store.NextSequence() } - _ = store.Close() - _ = os.RemoveAll(testDbPath) -} - -func BenchmarkIncr(b *testing.B) { - store, _ := New(testDbPath) - - key := []byte("incr-benchmark-counter") - for n := 0; n < b.N; n++ { - _, _ = store.Incr(key) - } - - _ = store.Close() - _ = os.RemoveAll(testDbPath) + store.Close() + os.RemoveAll(testDbPath) } diff --git a/storage/inmemorystore/store.go b/storage/inmemorystore/store.go index 9f8f3ad..e44f52e 100644 --- a/storage/inmemorystore/store.go +++ b/storage/inmemorystore/store.go @@ -1,17 +1,16 @@ package inmemorystore import ( - "errors" "sync" -) -// ErrNotFound is returned when Get() tries to fetch a non-existent key. -var ErrNotFound = errors.New("not found") + "github.com/jimeh/ozu.io/storage" +) // New creates a new Store using given path to persist data. func New() (*Store, error) { store := &Store{ - Data: map[string][]byte{}, + UIDMap: map[string][]byte{}, + URLMap: map[string][]byte{}, Sequence: 0, } return store, nil @@ -20,43 +19,79 @@ func New() (*Store, error) { // Store allows storing data into a in-memory map. type Store struct { sync.RWMutex - Data map[string][]byte + UIDMap map[string][]byte + URLMap map[string][]byte Sequence int - Closed bool } // Close database. func (s *Store) Close() error { - s.Data = make(map[string][]byte) + s.Lock() + s.UIDMap = make(map[string][]byte) + s.URLMap = make(map[string][]byte) s.Sequence = 0 + s.Unlock() return nil } -// Get a given key's value. -func (s *Store) Get(key []byte) ([]byte, error) { +// Create a given Record +func (s *Store) Create(uid []byte, url []byte) (*storage.Record, error) { + s.Lock() + s.UIDMap[string(uid)] = url + s.URLMap[string(url)] = uid + s.Unlock() + return &storage.Record{UID: uid, URL: url}, nil +} + +// FindByUID looks up records based on their UID. +func (s *Store) FindByUID(uid []byte) (*storage.Record, error) { s.RLock() - value := s.Data[string(key)] + value := s.UIDMap[string(uid)] s.RUnlock() if value == nil { - return nil, ErrNotFound + return &storage.Record{}, storage.ErrNotFound } - return value, nil + return &storage.Record{UID: uid, URL: value}, nil } -// Set a given key's to the specified value. -func (s *Store) Set(key []byte, value []byte) error { - s.Lock() - s.Data[string(key)] = value - s.Unlock() - return nil +// FindByURL looks up records based on their URL. +func (s *Store) FindByURL(url []byte) (*storage.Record, error) { + s.RLock() + value := s.URLMap[string(url)] + s.RUnlock() + if value == nil { + return &storage.Record{}, storage.ErrNotFound + } + return &storage.Record{UID: value, URL: url}, nil } -// Delete a given key. -func (s *Store) Delete(key []byte) error { +// DeleteByUID deletes records based on their UID. +func (s *Store) DeleteByUID(uid []byte) (*storage.Record, error) { + record, err := s.FindByUID(uid) + if err != nil { + return &storage.Record{}, err + } + + s.delete(record) + return record, nil +} + +// DeleteByURL deletes records based on their URL. +func (s *Store) DeleteByURL(url []byte) (*storage.Record, error) { + record, err := s.FindByURL(url) + if err != nil { + return &storage.Record{}, err + } + + s.delete(record) + return record, nil +} + +func (s *Store) delete(r *storage.Record) { s.Lock() - delete(s.Data, string(key)) + delete(s.UIDMap, string(r.UID)) + delete(s.URLMap, string(r.URL)) s.Unlock() - return nil } // NextSequence returns a auto-incrementing int. diff --git a/storage/inmemorystore/store_test.go b/storage/inmemorystore/store_test.go index 424d18f..0bc116f 100644 --- a/storage/inmemorystore/store_test.go +++ b/storage/inmemorystore/store_test.go @@ -9,13 +9,10 @@ import ( // Setup Suite -var examples = []struct { - key []byte - value []byte -}{ - {key: []byte("hello"), value: []byte("world")}, - {key: []byte("foo"), value: []byte("bar")}, - {key: []byte("wtf"), value: []byte("dude")}, +var examples = []storage.Record{ + storage.Record{UID: []byte("Kb8X"), URL: []byte("https://google.com/")}, + storage.Record{UID: []byte("h3mz"), URL: []byte("https://github.com/")}, + storage.Record{UID: []byte("3qxs"), URL: []byte("https://twitter.com/")}, } type StoreSuite struct { @@ -35,7 +32,8 @@ func (s *StoreSuite) TearDownTest() { func (s *StoreSuite) Seed() { for _, e := range examples { - s.store.Data[string(e.key)] = e.value + s.store.UIDMap[string(e.UID)] = e.URL + s.store.URLMap[string(e.URL)] = e.UID } } @@ -45,60 +43,112 @@ func (s *StoreSuite) TestStoreInterface() { s.Implements(new(storage.Store), new(Store)) } -func (s *StoreSuite) TestSet() { +func (s *StoreSuite) TestCreate() { for _, e := range examples { - err := s.store.Set(e.key, e.value) + record, err := s.store.Create(e.UID, e.URL) s.NoError(err) + s.Equal(e.UID, record.UID) + s.Equal(e.URL, record.URL) } for _, e := range examples { - result, _ := s.store.Data[string(e.key)] - s.Equal(e.value, result) + recordURL, _ := s.store.UIDMap[string(e.UID)] + s.Equal(e.URL, recordURL) + recordUID, _ := s.store.URLMap[string(e.URL)] + s.Equal(e.UID, recordUID) } } -func (s *StoreSuite) TestGetExisting() { +func (s *StoreSuite) TestFindExistingByUID() { s.Seed() for _, e := range examples { - result, err := s.store.Get(e.key) + record, err := s.store.FindByUID(e.UID) s.NoError(err) - s.Equal(e.value, result) + s.Equal(e.UID, record.UID) + s.Equal(e.URL, record.URL) } } -func (s *StoreSuite) TestGetNonExistant() { - result, err := s.store.Get([]byte("does-not-exist")) - s.Nil(result) +func (s *StoreSuite) TestFindNonExistantByUID() { + record, err := s.store.FindByUID([]byte("does-not-exist")) + s.Nil(record.UID) + s.Nil(record.URL) s.EqualError(err, "not found") } -func (s *StoreSuite) TestDeleteExisting() { +func (s *StoreSuite) TestFindExistingByURL() { s.Seed() for _, e := range examples { - value := s.store.Data[string(e.key)] - s.Require().Equal(e.value, value) - - value, err := s.store.Get(e.key) - s.Require().NoError(err) - s.Require().Equal(value, e.value) - - err = s.store.Delete(e.key) + record, err := s.store.FindByURL(e.URL) s.NoError(err) - - value, err = s.store.Get(e.key) - s.Nil(value) - s.EqualError(err, "not found") - - _, has := s.store.Data[string(e.key)] - s.Equal(false, has) + s.Equal(e.UID, record.UID) + s.Equal(e.URL, record.URL) } } -func (s *StoreSuite) TestDeleteNonExistant() { - err := s.store.Delete([]byte("does-not-exist")) - s.NoError(err) +func (s *StoreSuite) TestFindNonExistantByURL() { + record, err := s.store.FindByURL([]byte("http://nope.com/")) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") +} + +func (s *StoreSuite) TestDeleteExistingByUID() { + s.Seed() + + for _, e := range examples { + record, err := s.store.DeleteByUID(e.UID) + s.NoError(err) + s.Equal(record.UID, e.UID) + s.Equal(record.URL, e.URL) + + record, err = s.store.FindByUID(e.UID) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") + + record, err = s.store.FindByURL(e.URL) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") + } +} + +func (s *StoreSuite) TestDeleteNonExistantByUID() { + record, err := s.store.DeleteByUID([]byte("nope")) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") +} + +func (s *StoreSuite) TestDeleteExistingByURL() { + s.Seed() + + for _, e := range examples { + record, err := s.store.DeleteByURL(e.URL) + s.NoError(err) + s.Equal(record.UID, e.UID) + s.Equal(record.URL, e.URL) + + record, err = s.store.FindByUID(e.UID) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") + + record, err = s.store.FindByURL(e.URL) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") + } +} + +func (s *StoreSuite) TestDeleteNonExistantByURL() { + record, err := s.store.DeleteByURL([]byte("http://nope/")) + s.Nil(record.UID) + s.Nil(record.URL) + s.EqualError(err, "not found") } func (s *StoreSuite) TestNextSequenceExisting() { @@ -126,39 +176,53 @@ func TestStoreSuite(t *testing.T) { // Benchmarks -func BenchmarkGet(b *testing.B) { +func BenchmarkCreate(b *testing.B) { store, _ := New() - key := []byte("hello") - value := []byte("world") - _ = store.Set(key, value) + uid := []byte("Kb8X") + url := []byte("https://google.com/") for n := 0; n < b.N; n++ { - _, _ = store.Get(key) + store.Create(append(uid, string(n)...), url) } - _ = store.Close() + store.Close() } -func BenchmarkSet(b *testing.B) { +func BenchmarkFindByUID(b *testing.B) { store, _ := New() - key := []byte("hello") - value := []byte("world") + uid := []byte("Kb8X") + url := []byte("https://google.com/") + store.Create(uid, url) for n := 0; n < b.N; n++ { - _ = store.Set(append(key, string(n)...), value) + store.FindByUID(uid) } - _ = store.Close() + store.Close() +} + +func BenchmarkFindByURL(b *testing.B) { + store, _ := New() + + uid := []byte("Kb8X") + url := []byte("https://google.com/") + store.Create(uid, url) + + for n := 0; n < b.N; n++ { + store.FindByURL(url) + } + + store.Close() } func BenchmarkNextSequence(b *testing.B) { store, _ := New() for n := 0; n < b.N; n++ { - _, _ = store.NextSequence() + store.NextSequence() } - _ = store.Close() + store.Close() } diff --git a/storage/store.go b/storage/store.go index c79431d..6514e1b 100644 --- a/storage/store.go +++ b/storage/store.go @@ -1,10 +1,23 @@ package storage -// Store defines a standard interface for storage. +import "errors" + +// ErrNotFound is the default error message when data is not found. +var ErrNotFound = errors.New("not found") + +// Store defines a standard interface for storage type Store interface { Close() error - Get([]byte) ([]byte, error) - Set([]byte, []byte) error - Delete([]byte) error + Create(UID []byte, URL []byte) (*Record, error) + FindByUID(UID []byte) (*Record, error) + FindByURL(URL []byte) (*Record, error) + DeleteByUID(UID []byte) (*Record, error) + DeleteByURL(URL []byte) (*Record, error) NextSequence() (int, error) } + +// Record provides a standard way to refer to a shortened URL. +type Record struct { + UID []byte + URL []byte +} From d86f3d82183d9dfe310dd552a0e4d0750071f1ea Mon Sep 17 00:00:00 2001 From: Jim Myhrberg Date: Sun, 4 Dec 2016 23:00:03 +0000 Subject: [PATCH 2/3] Restrict Docker image builds to master branch and tags only --- .travis.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 557e78b..919d9f7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,7 @@ services: language: go go: - - 1.7.3 + - 1.7.4 before_install: - go get github.com/kardianos/govendor @@ -16,10 +16,10 @@ script: - make test after_success: - - if [ "$TRAVIS_PULL_REQUEST" == "false" ]; then + - if ([ "$TRAVIS_BRANCH" == "master" ] || [ -n "$TRAVIS_TAG" ]) && [ "$TRAVIS_PULL_REQUEST" == "false" ]; then docker login -u $DOCKERHUB_USER -p $DOCKERHUB_PASS; export DOCKER_REPO=jimeh/ozu.io; - export TAG="$(if [ "$TRAVIS_BRANCH" == "master" ]; then echo "latest"; else echo $TRAVIS_BRANCH ; fi)"; + export TAG="$(if [ "$TRAVIS_BRANCH" == "master" ]; then echo "latest"; else echo $TRAVIS_TAG ; fi)"; make bin/ozuio_linux_amd64; docker build -f Dockerfile -t $DOCKER_REPO:$TAG .; docker push $DOCKER_REPO:$TAG; From de2ec75696cb057f7fda7642a62eb5ebaa8d2ad4 Mon Sep 17 00:00:00 2001 From: Jim Myhrberg Date: Sun, 4 Dec 2016 23:01:51 +0000 Subject: [PATCH 3/3] Refactor shortner and web packages for new Store interface --- shortener/base58_shortener.go | 58 ++++-------- shortener/base58_shortener_test.go | 99 ++++++++++++--------- shortener/mocks/Store.go | 136 +++++++++++++++++++++-------- shortener/shortener.go | 6 +- web/api_handler.go | 8 +- web/handler.go | 10 +-- web/handler_helpers.go | 9 +- 7 files changed, 193 insertions(+), 133 deletions(-) diff --git a/shortener/base58_shortener.go b/shortener/base58_shortener.go index 4179486..257b538 100644 --- a/shortener/base58_shortener.go +++ b/shortener/base58_shortener.go @@ -1,9 +1,7 @@ package shortener import ( - "crypto/sha1" "errors" - "fmt" "github.com/jimeh/go-base58" "github.com/jimeh/ozu.io/storage" @@ -24,55 +22,40 @@ type Base58Shortener struct { } // Shorten a given URL. -func (s *Base58Shortener) Shorten(rawURL []byte) (uid []byte, url []byte, err error) { - url, err = NormalizeURL(rawURL) +func (s *Base58Shortener) Shorten(rawURL []byte) (*storage.Record, error) { + url, err := NormalizeURL(rawURL) if err != nil { - return nil, nil, err + return &storage.Record{}, err } - urlKey := s.makeURLKey(url) - uid, err = s.Store.Get(urlKey) - - if uid != nil && err == nil { - return uid, url, nil - } else if err != nil && err.Error() != "not found" { - return nil, nil, err + record, err := s.Store.FindByURL(url) + if err == nil { + return record, nil + } else if err != storage.ErrNotFound { + return &storage.Record{}, err } - uid, err = s.newUID() + uid, err := s.newUID() if err != nil { - return nil, nil, err + return &storage.Record{}, err } - err = s.Store.Set(urlKey, uid) + record, err = s.Store.Create(uid, url) if err != nil { - return nil, nil, err + return &storage.Record{}, err } - uidKey := s.makeUIDKey(uid) - err = s.Store.Set(uidKey, url) - if err != nil { - return nil, nil, err - } - - return uid, url, nil + return record, nil } // Lookup the URL of a given UID. -func (s *Base58Shortener) Lookup(uid []byte) ([]byte, error) { +func (s *Base58Shortener) Lookup(uid []byte) (*storage.Record, error) { _, err := base58.Decode(uid) if err != nil { - return nil, errInvalidUID + return &storage.Record{}, errInvalidUID } - uidKey := s.makeUIDKey(uid) - - url, err := s.Store.Get(uidKey) - if err != nil { - return nil, err - } - - return url, nil + return s.Store.FindByUID(uid) } func (s *Base58Shortener) newUID() ([]byte, error) { @@ -83,12 +66,3 @@ func (s *Base58Shortener) newUID() ([]byte, error) { return base58.Encode(index), nil } - -func (s *Base58Shortener) makeUIDKey(uid []byte) []byte { - return append(uidKeyPrefix, uid...) -} - -func (s *Base58Shortener) makeURLKey(rawURL []byte) []byte { - urlSHA := fmt.Sprintf("%x", sha1.Sum(rawURL)) - return append(urlKeyPrefix, urlSHA...) -} diff --git a/shortener/base58_shortener_test.go b/shortener/base58_shortener_test.go index e537012..2e1e46a 100644 --- a/shortener/base58_shortener_test.go +++ b/shortener/base58_shortener_test.go @@ -1,13 +1,12 @@ package shortener import ( - "crypto/sha1" "errors" - "fmt" "strings" "testing" "github.com/jimeh/ozu.io/shortener/mocks" + "github.com/jimeh/ozu.io/storage" "github.com/stretchr/testify/suite" ) @@ -19,52 +18,70 @@ import ( type Base58ShortenerSuite struct { suite.Suite - store *mocks.Store - shortener *Base58Shortener - errNotFound error + store *mocks.Store + shortener *Base58Shortener } func (s *Base58ShortenerSuite) SetupTest() { s.store = new(mocks.Store) s.shortener = NewBase58(s.store) - s.errNotFound = errors.New("not found") } // Tests func (s *Base58ShortenerSuite) TestShortenExisting() { - rawURL := []byte("http://google.com/") uid := []byte("ig") - urlSHA := fmt.Sprintf("%x", sha1.Sum(rawURL)) + url := []byte("https://google.com/") + record := storage.Record{UID: uid, URL: url} - s.store.On("Get", append([]byte("url:"), urlSHA...)).Return(uid, nil) + s.store.On("FindByURL", url).Return(&record, nil) - resultUID, resultURL, err := s.shortener.Shorten(rawURL) + result, err := s.shortener.Shorten(url) s.NoError(err) - s.Equal(uid, resultUID) - s.Equal(rawURL, resultURL) + s.Equal(uid, result.UID) + s.Equal(url, result.URL) s.store.AssertExpectations(s.T()) } func (s *Base58ShortenerSuite) TestShortenNew() { - rawURL := []byte("https://google.com") - url := []byte("https://google.com/") uid := []byte("ig") - urlKey := append([]byte("url:"), fmt.Sprintf("%x", sha1.Sum(url))...) + url := []byte("https://google.com/") + record := storage.Record{UID: uid, URL: url} - s.store.On("Get", urlKey).Return(nil, s.errNotFound) + s.store.On("FindByURL", url).Return(nil, storage.ErrNotFound) s.store.On("NextSequence").Return(1001, nil) - s.store.On("Set", urlKey, uid).Return(nil) - s.store.On("Set", append([]byte("uid:"), uid...), url).Return(nil) + s.store.On("Create", uid, url).Return(&record, nil) - rUID, rURL, err := s.shortener.Shorten(rawURL) + result, err := s.shortener.Shorten(url) s.NoError(err) - s.Equal(uid, rUID) - s.Equal(url, rURL) + s.Equal(uid, result.UID) + s.Equal(url, result.URL) s.store.AssertExpectations(s.T()) } +func (s *Base58ShortenerSuite) TestShortenAndNormalizeURL() { + examples := []struct { + url []byte + normalized []byte + }{ + {[]byte("google.com"), []byte("http://google.com/")}, + {[]byte("google.com/"), []byte("http://google.com/")}, + {[]byte("http://google.com"), []byte("http://google.com/")}, + } + + for _, e := range examples { + record := storage.Record{UID: []byte("ig"), URL: e.normalized} + s.store.On("FindByURL", record.URL).Return(&record, nil) + + result, err := s.shortener.Shorten(e.url) + s.NoError(err) + s.Equal(record.UID, result.UID) + s.Equal(record.URL, result.URL) + s.store.AssertExpectations(s.T()) + } +} + func (s *Base58ShortenerSuite) TestShortenInvalidURL() { examples := []struct { url string @@ -93,9 +110,9 @@ func (s *Base58ShortenerSuite) TestShortenInvalidURL() { } for _, e := range examples { - rUID, rURL, err := s.shortener.Shorten([]byte(e.url)) - s.Nil(rUID) - s.Nil(rURL) + record, err := s.shortener.Shorten([]byte(e.url)) + s.Nil(record.UID) + s.Nil(record.URL) s.EqualError(err, e.error) } } @@ -103,48 +120,50 @@ func (s *Base58ShortenerSuite) TestShortenInvalidURL() { func (s *Base58ShortenerSuite) TestShortenStoreError() { url := []byte("https://google.com/") storeErr := errors.New("leveldb: something wrong") - urlKey := append([]byte("url:"), fmt.Sprintf("%x", sha1.Sum(url))...) - s.store.On("Get", urlKey).Return(nil, storeErr) + s.store.On("FindByURL", url).Return(nil, storeErr) - rUID, rURL, err := s.shortener.Shorten(url) - s.Nil(rUID) - s.Nil(rURL) + result, err := s.shortener.Shorten(url) + s.Nil(result.UID) + s.Nil(result.URL) s.EqualError(err, storeErr.Error()) + s.store.AssertExpectations(s.T()) } func (s *Base58ShortenerSuite) TestLookupExisting() { - url := []byte("https://google.com/") uid := []byte("ig") + url := []byte("https://google.com/") + record := storage.Record{UID: uid, URL: url} - s.store.On("Get", append([]byte("uid:"), uid...)).Return(url, nil) - - rURL, err := s.shortener.Lookup(uid) + s.store.On("FindByUID", uid).Return(&record, nil) + result, err := s.shortener.Lookup(uid) s.NoError(err) - s.Equal(url, rURL) + s.Equal(uid, result.UID) + s.Equal(url, result.URL) s.store.AssertExpectations(s.T()) } func (s *Base58ShortenerSuite) TestLookupNonExistant() { uid := []byte("ig") - s.store.On("Get", append([]byte("uid:"), uid...)).Return(nil, s.errNotFound) - - rURL, err := s.shortener.Lookup(uid) + s.store.On("FindByUID", uid).Return(&storage.Record{}, storage.ErrNotFound) + result, err := s.shortener.Lookup(uid) s.EqualError(err, "not found") - s.Nil(rURL) + s.Nil(result.UID) + s.Nil(result.URL) s.store.AssertExpectations(s.T()) } func (s *Base58ShortenerSuite) TestLookupInvalid() { uid := []byte("ig\"; drop table haha") - rURL, err := s.shortener.Lookup(uid) + result, err := s.shortener.Lookup(uid) s.EqualError(err, "invalid UID") - s.Nil(rURL) + s.Nil(result.UID) + s.Nil(result.URL) s.store.AssertExpectations(s.T()) } diff --git a/shortener/mocks/Store.go b/shortener/mocks/Store.go index feb6ab6..4504e42 100644 --- a/shortener/mocks/Store.go +++ b/shortener/mocks/Store.go @@ -22,36 +22,114 @@ func (_m *Store) Close() error { return r0 } -// Delete provides a mock function with given fields: _a0 -func (_m *Store) Delete(_a0 []byte) error { - ret := _m.Called(_a0) +// Create provides a mock function with given fields: UID, URL +func (_m *Store) Create(UID []byte, URL []byte) (*storage.Record, error) { + ret := _m.Called(UID, URL) - var r0 error - if rf, ok := ret.Get(0).(func([]byte) error); ok { - r0 = rf(_a0) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// Get provides a mock function with given fields: _a0 -func (_m *Store) Get(_a0 []byte) ([]byte, error) { - ret := _m.Called(_a0) - - var r0 []byte - if rf, ok := ret.Get(0).(func([]byte) []byte); ok { - r0 = rf(_a0) + var r0 *storage.Record + if rf, ok := ret.Get(0).(func([]byte, []byte) *storage.Record); ok { + r0 = rf(UID, URL) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).([]byte) + r0 = ret.Get(0).(*storage.Record) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func([]byte, []byte) error); ok { + r1 = rf(UID, URL) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// DeleteByUID provides a mock function with given fields: UID +func (_m *Store) DeleteByUID(UID []byte) (*storage.Record, error) { + ret := _m.Called(UID) + + var r0 *storage.Record + if rf, ok := ret.Get(0).(func([]byte) *storage.Record); ok { + r0 = rf(UID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*storage.Record) } } var r1 error if rf, ok := ret.Get(1).(func([]byte) error); ok { - r1 = rf(_a0) + r1 = rf(UID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// DeleteByURL provides a mock function with given fields: URL +func (_m *Store) DeleteByURL(URL []byte) (*storage.Record, error) { + ret := _m.Called(URL) + + var r0 *storage.Record + if rf, ok := ret.Get(0).(func([]byte) *storage.Record); ok { + r0 = rf(URL) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*storage.Record) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func([]byte) error); ok { + r1 = rf(URL) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// FindByUID provides a mock function with given fields: UID +func (_m *Store) FindByUID(UID []byte) (*storage.Record, error) { + ret := _m.Called(UID) + + var r0 *storage.Record + if rf, ok := ret.Get(0).(func([]byte) *storage.Record); ok { + r0 = rf(UID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*storage.Record) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func([]byte) error); ok { + r1 = rf(UID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// FindByURL provides a mock function with given fields: URL +func (_m *Store) FindByURL(URL []byte) (*storage.Record, error) { + ret := _m.Called(URL) + + var r0 *storage.Record + if rf, ok := ret.Get(0).(func([]byte) *storage.Record); ok { + r0 = rf(URL) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*storage.Record) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func([]byte) error); ok { + r1 = rf(URL) } else { r1 = ret.Error(1) } @@ -80,18 +158,4 @@ func (_m *Store) NextSequence() (int, error) { return r0, r1 } -// Set provides a mock function with given fields: _a0, _a1 -func (_m *Store) Set(_a0 []byte, _a1 []byte) error { - ret := _m.Called(_a0, _a1) - - var r0 error - if rf, ok := ret.Get(0).(func([]byte, []byte) error); ok { - r0 = rf(_a0, _a1) - } else { - r0 = ret.Error(0) - } - - return r0 -} - var _ storage.Store = (*Store)(nil) diff --git a/shortener/shortener.go b/shortener/shortener.go index 4679e54..96484c0 100644 --- a/shortener/shortener.go +++ b/shortener/shortener.go @@ -1,7 +1,9 @@ package shortener +import "github.com/jimeh/ozu.io/storage" + // Shortener defines a shortener interface for shortening URLs. type Shortener interface { - Shorten([]byte) ([]byte, []byte, error) - Lookup([]byte) ([]byte, error) + Shorten([]byte) (*storage.Record, error) + Lookup([]byte) (*storage.Record, error) } diff --git a/web/api_handler.go b/web/api_handler.go index 3b6940a..9326a7b 100644 --- a/web/api_handler.go +++ b/web/api_handler.go @@ -20,24 +20,24 @@ type APIHandler struct { // Shorten shortens given URL. func (h *APIHandler) Shorten(c *routing.Context) error { - uid, url, err := h.shortener.Shorten(c.FormValue("url")) + record, err := h.shortener.Shorten(c.FormValue("url")) if err != nil { return h.respondWithError(c, err) } - r := makeResponse(c, uid, url) + r := makeResponse(c, record) return h.respond(c, &r) } // Lookup shortened UID. func (h *APIHandler) Lookup(c *routing.Context) error { uid := c.FormValue("uid") - url, err := h.shortener.Lookup(uid) + record, err := h.shortener.Lookup(uid) if err != nil { return h.respondWithError(c, err) } - r := makeResponse(c, uid, url) + r := makeResponse(c, record) return h.respond(c, &r) } diff --git a/web/handler.go b/web/handler.go index 0480f4b..95aaebd 100644 --- a/web/handler.go +++ b/web/handler.go @@ -71,12 +71,12 @@ func (h *Handler) Index(c *routing.Context) error { rawURL := c.FormValue("url") if len(rawURL) > 0 { - uid, url, err := h.shortener.Shorten(rawURL) + record, err := h.shortener.Shorten(rawURL) if err != nil { return h.respond(c, template, makeErrResponse(err)) } - r := makeResponse(c, uid, url) + r := makeResponse(c, record) return h.respond(c, template, r) } @@ -106,19 +106,19 @@ func (h *Handler) Static(c *routing.Context) error { func (h *Handler) LookupAndRedirect(c *routing.Context) error { uid := []byte(c.Param("uid")) - url, err := h.shortener.Lookup(uid) + record, err := h.shortener.Lookup(uid) if err != nil { return h.NotFound(c) } - r := makeResponse(c, uid, url) + r := makeResponse(c, record) c.Response.Header.Set("Pragma", "no-cache") c.Response.Header.Set("Expires", "Mon, 01 Jan 1990 00:00:00 GMT") c.Response.Header.Set("X-XSS-Protection", "1; mode=block") c.Response.Header.Set("Cache-Control", "no-cache, no-store, max-age=0, must-revalidate") - c.Redirect(string(url), fasthttp.StatusMovedPermanently) + c.Redirect(string(record.URL), fasthttp.StatusMovedPermanently) c.Response.Header.Set("Connection", "close") c.Response.Header.Set("X-Content-Type-Options", "nosniff") c.Response.Header.Set("Accept-Ranges", "none") diff --git a/web/handler_helpers.go b/web/handler_helpers.go index e1a7198..3155b7f 100644 --- a/web/handler_helpers.go +++ b/web/handler_helpers.go @@ -3,14 +3,15 @@ package web import ( "net/url" + "github.com/jimeh/ozu.io/storage" "github.com/qiangxue/fasthttp-routing" ) -func makeResponse(c *routing.Context, uid []byte, url []byte) Response { +func makeResponse(c *routing.Context, r *storage.Record) Response { return Response{ - UID: string(uid), - URL: makeShortURL(c, uid), - Target: string(url), + UID: string(r.UID), + URL: makeShortURL(c, r.UID), + Target: string(r.URL), } }