From 0e837e2d03e022681a565e6380c03fa164896396 Mon Sep 17 00:00:00 2001 From: Matt K <1036969+mkrump@users.noreply.github.com> Date: Wed, 13 Dec 2017 10:51:11 -0600 Subject: [PATCH] Refactoring (#101) * Make naming consistent for watched_contracts * Update FindContract and FindBlockByNumber to return errors rather than nil --- Gododir/main.go | 2 +- .../main.go | 0 pkg/contract_summary/summary.go | 15 ++---- pkg/history/populate_blocks_test.go | 19 ++++--- pkg/observers/blockchain_db_observer_test.go | 4 +- pkg/repositories/in_memory.go | 27 +++++----- pkg/repositories/postgres.go | 50 +++++++++++-------- pkg/repositories/postgres_test.go | 18 ++++--- pkg/repositories/repository.go | 4 +- pkg/repositories/testing/helpers.go | 43 ++++++++-------- 10 files changed, 98 insertions(+), 84 deletions(-) rename cmd/{subscribe_contract => watch_contract}/main.go (100%) diff --git a/Gododir/main.go b/Gododir/main.go index 44456216..745bb16d 100644 --- a/Gododir/main.go +++ b/Gododir/main.go @@ -64,7 +64,7 @@ func tasks(p *do.Project) { "environment": environment, "contractHash": contractHash, "abiFilepath": abiFilepath, - "$in": "cmd/subscribe_contract", + "$in": "cmd/watch_contract", }) }) diff --git a/cmd/subscribe_contract/main.go b/cmd/watch_contract/main.go similarity index 100% rename from cmd/subscribe_contract/main.go rename to cmd/watch_contract/main.go diff --git a/pkg/contract_summary/summary.go b/pkg/contract_summary/summary.go index fea42f9f..98e26ee5 100644 --- a/pkg/contract_summary/summary.go +++ b/pkg/contract_summary/summary.go @@ -1,9 +1,6 @@ package contract_summary import ( - "errors" - "fmt" - "math/big" "github.com/8thlight/vulcanizedb/pkg/core" @@ -20,16 +17,12 @@ type ContractSummary struct { blockChain core.Blockchain } -var ErrContractDoesNotExist = func(contractHash string) error { - return errors.New(fmt.Sprintf("Contract %v does not exist", contractHash)) -} - func NewSummary(blockchain core.Blockchain, repository repositories.Repository, contractHash string, blockNumber *big.Int) (ContractSummary, error) { - contract := repository.FindContract(contractHash) - if contract != nil { - return newContractSummary(blockchain, *contract, blockNumber), nil + contract, err := repository.FindContract(contractHash) + if err != nil { + return ContractSummary{}, err } else { - return ContractSummary{}, ErrContractDoesNotExist(contractHash) + return newContractSummary(blockchain, contract, blockNumber), nil } } diff --git a/pkg/history/populate_blocks_test.go b/pkg/history/populate_blocks_test.go index 1b3c772b..0dcc7aee 100644 --- a/pkg/history/populate_blocks_test.go +++ b/pkg/history/populate_blocks_test.go @@ -19,8 +19,8 @@ var _ = Describe("Populating blocks", func() { history.PopulateBlocks(blockchain, repository, 1) - block := repository.FindBlockByNumber(1) - Expect(block).NotTo(BeNil()) + block, err := repository.FindBlockByNumber(1) + Expect(err).ToNot(HaveOccurred()) Expect(block.Hash).To(Equal("x012343")) }) @@ -45,11 +45,16 @@ var _ = Describe("Populating blocks", func() { history.PopulateBlocks(blockchain, repository, 5) Expect(repository.BlockCount()).To(Equal(11)) - Expect(repository.FindBlockByNumber(4)).To(BeNil()) - Expect(repository.FindBlockByNumber(5)).NotTo(BeNil()) - Expect(repository.FindBlockByNumber(8)).NotTo(BeNil()) - Expect(repository.FindBlockByNumber(10)).NotTo(BeNil()) - Expect(repository.FindBlockByNumber(13)).To(BeNil()) + _, err := repository.FindBlockByNumber(4) + Expect(err).To(HaveOccurred()) + _, err = repository.FindBlockByNumber(5) + Expect(err).ToNot(HaveOccurred()) + _, err = repository.FindBlockByNumber(8) + Expect(err).ToNot(HaveOccurred()) + _, err = repository.FindBlockByNumber(10) + Expect(err).ToNot(HaveOccurred()) + _, err = repository.FindBlockByNumber(13) + Expect(err).To(HaveOccurred()) }) It("returns the number of blocks created", func() { diff --git a/pkg/observers/blockchain_db_observer_test.go b/pkg/observers/blockchain_db_observer_test.go index 0f510095..38a254ae 100644 --- a/pkg/observers/blockchain_db_observer_test.go +++ b/pkg/observers/blockchain_db_observer_test.go @@ -30,8 +30,8 @@ var _ = Describe("Saving blocks to the database", func() { observer := observers.NewBlockchainDbObserver(repository) observer.NotifyBlockAdded(block) - savedBlock := repository.FindBlockByNumber(123) - Expect(savedBlock).NotTo(BeNil()) + savedBlock, err := repository.FindBlockByNumber(123) + Expect(err).ToNot(HaveOccurred()) Expect(len(savedBlock.Transactions)).To(Equal(1)) }) diff --git a/pkg/repositories/in_memory.go b/pkg/repositories/in_memory.go index 9e0bac9c..b2a0de6c 100644 --- a/pkg/repositories/in_memory.go +++ b/pkg/repositories/in_memory.go @@ -7,8 +7,8 @@ import ( ) type InMemory struct { - blocks map[int64]*core.Block - contracts map[string]*core.Contract + blocks map[int64]core.Block + contracts map[string]core.Contract logs map[string][]core.Log } @@ -34,7 +34,7 @@ func (repository *InMemory) FindLogs(address string, blockNumber int64) []core.L } func (repository *InMemory) CreateContract(contract core.Contract) error { - repository.contracts[contract.Hash] = &contract + repository.contracts[contract.Hash] = contract return nil } @@ -43,10 +43,10 @@ func (repository *InMemory) ContractExists(contractHash string) bool { return present } -func (repository *InMemory) FindContract(contractHash string) *core.Contract { +func (repository *InMemory) FindContract(contractHash string) (core.Contract, error) { contract, ok := repository.contracts[contractHash] if !ok { - return nil + return core.Contract{}, ErrContractDoesNotExist(contractHash) } for _, block := range repository.blocks { for _, transaction := range block.Transactions { @@ -55,13 +55,13 @@ func (repository *InMemory) FindContract(contractHash string) *core.Contract { } } } - return contract + return contract, nil } func (repository *InMemory) MissingBlockNumbers(startingBlockNumber int64, endingBlockNumber int64) []int64 { missingNumbers := []int64{} for blockNumber := int64(startingBlockNumber); blockNumber <= endingBlockNumber; blockNumber++ { - if repository.blocks[blockNumber] == nil { + if _, ok := repository.blocks[blockNumber]; !ok { missingNumbers = append(missingNumbers, blockNumber) } } @@ -70,14 +70,14 @@ func (repository *InMemory) MissingBlockNumbers(startingBlockNumber int64, endin func NewInMemory() *InMemory { return &InMemory{ - blocks: make(map[int64]*core.Block), - contracts: make(map[string]*core.Contract), + blocks: make(map[int64]core.Block), + contracts: make(map[string]core.Contract), logs: make(map[string][]core.Log), } } func (repository *InMemory) CreateBlock(block core.Block) error { - repository.blocks[block.Number] = &block + repository.blocks[block.Number] = block return nil } @@ -85,8 +85,11 @@ func (repository *InMemory) BlockCount() int { return len(repository.blocks) } -func (repository *InMemory) FindBlockByNumber(blockNumber int64) *core.Block { - return repository.blocks[blockNumber] +func (repository *InMemory) FindBlockByNumber(blockNumber int64) (core.Block, error) { + if block, ok := repository.blocks[blockNumber]; ok { + return block, nil + } + return core.Block{}, ErrBlockDoesNotExist(blockNumber) } func (repository *InMemory) MaxBlockNumber() int64 { diff --git a/pkg/repositories/postgres.go b/pkg/repositories/postgres.go index 0d59b033..1f8cdf4d 100644 --- a/pkg/repositories/postgres.go +++ b/pkg/repositories/postgres.go @@ -7,6 +7,8 @@ import ( "errors" + "fmt" + "github.com/8thlight/vulcanizedb/pkg/config" "github.com/8thlight/vulcanizedb/pkg/core" "github.com/jmoiron/sqlx" @@ -25,6 +27,28 @@ var ( ErrUnableToSetNode = errors.New("postgres: unable to set node") ) +var ErrContractDoesNotExist = func(contractHash string) error { + return errors.New(fmt.Sprintf("Contract %v does not exist", contractHash)) +} + +var ErrBlockDoesNotExist = func(blockNumber int64) error { + return errors.New(fmt.Sprintf("Block number %d does not exist", blockNumber)) +} + +func NewPostgres(databaseConfig config.Database, node core.Node) (Postgres, error) { + connectString := config.DbConnectionString(databaseConfig) + db, err := sqlx.Connect("postgres", connectString) + if err != nil { + return Postgres{}, ErrDBConnectionFailed + } + pg := Postgres{Db: db, node: node} + err = pg.CreateNode(&node) + if err != nil { + return Postgres{}, ErrUnableToSetNode + } + return pg, nil +} + func (repository Postgres) CreateLogs(logs []core.Log) error { tx, _ := repository.Db.BeginTx(context.Background(), nil) for _, tlog := range logs { @@ -71,20 +95,6 @@ func (repository Postgres) FindLogs(address string, blockNumber int64) []core.Lo return repository.loadLogs(logRows) } -func NewPostgres(databaseConfig config.Database, node core.Node) (Postgres, error) { - connectString := config.DbConnectionString(databaseConfig) - db, err := sqlx.Connect("postgres", connectString) - if err != nil { - return Postgres{}, ErrDBConnectionFailed - } - pg := Postgres{Db: db, node: node} - err = pg.CreateNode(&node) - if err != nil { - return Postgres{}, ErrUnableToSetNode - } - return pg, nil -} - func (repository *Postgres) CreateNode(node *core.Node) error { var nodeId int64 err := repository.Db.QueryRow( @@ -129,17 +139,17 @@ func (repository Postgres) ContractExists(contractHash string) bool { return exists } -func (repository Postgres) FindContract(contractHash string) *core.Contract { +func (repository Postgres) FindContract(contractHash string) (core.Contract, error) { var hash string var abi string contract := repository.Db.QueryRow( `SELECT contract_hash, contract_abi FROM watched_contracts WHERE contract_hash=$1`, contractHash) err := contract.Scan(&hash, &abi) if err == sql.ErrNoRows { - return nil + return core.Contract{}, ErrContractDoesNotExist(contractHash) } savedContract := repository.addTransactions(core.Contract{Hash: hash, Abi: abi}) - return &savedContract + return savedContract, nil } func (repository Postgres) MaxBlockNumber() int64 { @@ -162,7 +172,7 @@ func (repository Postgres) MissingBlockNumbers(startingBlockNumber int64, highes return numbers } -func (repository Postgres) FindBlockByNumber(blockNumber int64) *core.Block { +func (repository Postgres) FindBlockByNumber(blockNumber int64) (core.Block, error) { blockRows, _ := repository.Db.Query( `SELECT id, block_number, @@ -183,9 +193,9 @@ func (repository Postgres) FindBlockByNumber(blockNumber int64) *core.Block { savedBlocks = append(savedBlocks, savedBlock) } if len(savedBlocks) > 0 { - return &savedBlocks[0] + return savedBlocks[0], nil } else { - return nil + return core.Block{}, ErrBlockDoesNotExist(blockNumber) } } diff --git a/pkg/repositories/postgres_test.go b/pkg/repositories/postgres_test.go index dca3f37f..eaef2d8a 100644 --- a/pkg/repositories/postgres_test.go +++ b/pkg/repositories/postgres_test.go @@ -43,11 +43,12 @@ var _ = Describe("Postgres repository", func() { node := core.Node{GenesisBlock: "GENESIS", NetworkId: 1} repository, _ := repositories.NewPostgres(cfg.Database, node) - err := repository.CreateBlock(badBlock) - savedBlock := repository.FindBlockByNumber(123) + err1 := repository.CreateBlock(badBlock) + savedBlock, err2 := repository.FindBlockByNumber(123) - Expect(err).ToNot(BeNil()) - Expect(savedBlock).To(BeNil()) + Expect(err1).To(HaveOccurred()) + Expect(err2).To(HaveOccurred()) + Expect(savedBlock).To(BeZero()) }) It("throws error when can't connect to the database", func() { @@ -96,11 +97,12 @@ var _ = Describe("Postgres repository", func() { node := core.Node{GenesisBlock: "GENESIS", NetworkId: 1} repository, _ := repositories.NewPostgres(cfg.Database, node) - err := repository.CreateBlock(block) - savedBlock := repository.FindBlockByNumber(123) + err1 := repository.CreateBlock(block) + savedBlock, err2 := repository.FindBlockByNumber(123) - Expect(err).ToNot(BeNil()) - Expect(savedBlock).To(BeNil()) + Expect(err1).To(HaveOccurred()) + Expect(err2).To(HaveOccurred()) + Expect(savedBlock).To(BeZero()) }) }) diff --git a/pkg/repositories/repository.go b/pkg/repositories/repository.go index 51e0d81d..fe062989 100644 --- a/pkg/repositories/repository.go +++ b/pkg/repositories/repository.go @@ -5,12 +5,12 @@ import "github.com/8thlight/vulcanizedb/pkg/core" type Repository interface { CreateBlock(block core.Block) error BlockCount() int - FindBlockByNumber(blockNumber int64) *core.Block + FindBlockByNumber(blockNumber int64) (core.Block, error) MaxBlockNumber() int64 MissingBlockNumbers(startingBlockNumber int64, endingBlockNumber int64) []int64 CreateContract(contract core.Contract) error ContractExists(contractHash string) bool - FindContract(contractHash string) *core.Contract + FindContract(contractHash string) (core.Contract, error) CreateLogs(log []core.Log) error FindLogs(address string, blockNumber int64) []core.Log } diff --git a/pkg/repositories/testing/helpers.go b/pkg/repositories/testing/helpers.go index c482be68..d13bc29f 100644 --- a/pkg/repositories/testing/helpers.go +++ b/pkg/repositories/testing/helpers.go @@ -49,8 +49,8 @@ func AssertRepositoryBehavior(buildRepository func(node core.Node) repositories. } repositoryTwo := buildRepository(nodeTwo) - foundBlock := repositoryTwo.FindBlockByNumber(123) - Expect(foundBlock).To(BeNil()) + _, err := repositoryTwo.FindBlockByNumber(123) + Expect(err).To(HaveOccurred()) }) It("saves the attributes of the block", func() { @@ -79,7 +79,8 @@ func AssertRepositoryBehavior(buildRepository func(node core.Node) repositories. repository.CreateBlock(block) - savedBlock := repository.FindBlockByNumber(blockNumber) + savedBlock, err := repository.FindBlockByNumber(blockNumber) + Expect(err).NotTo(HaveOccurred()) Expect(savedBlock.Difficulty).To(Equal(difficulty)) Expect(savedBlock.GasLimit).To(Equal(gasLimit)) Expect(savedBlock.GasUsed).To(Equal(gasUsed)) @@ -93,9 +94,9 @@ func AssertRepositoryBehavior(buildRepository func(node core.Node) repositories. }) It("does not find a block when searching for a number that does not exist", func() { - savedBlock := repository.FindBlockByNumber(111) + _, err := repository.FindBlockByNumber(111) - Expect(savedBlock).To(BeNil()) + Expect(err).To(HaveOccurred()) }) It("saves one transaction associated to the block", func() { @@ -106,7 +107,7 @@ func AssertRepositoryBehavior(buildRepository func(node core.Node) repositories. repository.CreateBlock(block) - savedBlock := repository.FindBlockByNumber(123) + savedBlock, _ := repository.FindBlockByNumber(123) Expect(len(savedBlock.Transactions)).To(Equal(1)) }) @@ -118,7 +119,7 @@ func AssertRepositoryBehavior(buildRepository func(node core.Node) repositories. repository.CreateBlock(block) - savedBlock := repository.FindBlockByNumber(123) + savedBlock, _ := repository.FindBlockByNumber(123) Expect(len(savedBlock.Transactions)).To(Equal(2)) }) @@ -145,7 +146,7 @@ func AssertRepositoryBehavior(buildRepository func(node core.Node) repositories. repository.CreateBlock(block) - savedBlock := repository.FindBlockByNumber(123) + savedBlock, _ := repository.FindBlockByNumber(123) Expect(len(savedBlock.Transactions)).To(Equal(1)) savedTransaction := savedBlock.Transactions[0] Expect(savedTransaction.Hash).To(Equal(transaction.Hash)) @@ -231,23 +232,23 @@ func AssertRepositoryBehavior(buildRepository func(node core.Node) repositories. It("returns the contract when it exists", func() { repository.CreateContract(core.Contract{Hash: "x123"}) - contract := repository.FindContract("x123") - Expect(contract).NotTo(BeNil()) + contract, err := repository.FindContract("x123") + Expect(err).NotTo(HaveOccurred()) Expect(contract.Hash).To(Equal("x123")) Expect(repository.ContractExists("x123")).To(BeTrue()) Expect(repository.ContractExists("x456")).To(BeFalse()) }) - It("returns nil if contract does not exist", func() { - contract := repository.FindContract("x123") - Expect(contract).To(BeNil()) + It("returns err if contract does not exist", func() { + _, err := repository.FindContract("x123") + Expect(err).To(HaveOccurred()) }) It("returns empty array when no transactions 'To' a contract", func() { repository.CreateContract(core.Contract{Hash: "x123"}) - contract := repository.FindContract("x123") - Expect(contract).ToNot(BeNil()) + contract, err := repository.FindContract("x123") + Expect(err).ToNot(HaveOccurred()) Expect(contract.Transactions).To(BeEmpty()) }) @@ -263,8 +264,8 @@ func AssertRepositoryBehavior(buildRepository func(node core.Node) repositories. repository.CreateBlock(block) repository.CreateContract(core.Contract{Hash: "x123"}) - contract := repository.FindContract("x123") - Expect(contract).ToNot(BeNil()) + contract, err := repository.FindContract("x123") + Expect(err).ToNot(HaveOccurred()) Expect(contract.Transactions).To( Equal([]core.Transaction{ {Hash: "TRANSACTION1", To: "x123"}, @@ -277,8 +278,8 @@ func AssertRepositoryBehavior(buildRepository func(node core.Node) repositories. Abi: "{\"some\": \"json\"}", Hash: "x123", }) - contract := repository.FindContract("x123") - Expect(contract).ToNot(BeNil()) + contract, err := repository.FindContract("x123") + Expect(err).ToNot(HaveOccurred()) Expect(contract.Abi).To(Equal("{\"some\": \"json\"}")) }) @@ -291,8 +292,8 @@ func AssertRepositoryBehavior(buildRepository func(node core.Node) repositories. Abi: "{\"some\": \"different json\"}", Hash: "x123", }) - contract := repository.FindContract("x123") - Expect(contract).ToNot(BeNil()) + contract, err := repository.FindContract("x123") + Expect(err).ToNot(HaveOccurred()) Expect(contract.Abi).To(Equal("{\"some\": \"different json\"}")) }) })