cleanup unused code, call storage even for empty data

Author: peterargue

module/executiondatasync/optimistic_sync/core.go

@@ -334,7 +334,7 @@ func (c *CoreImpl) Persist() error {
 	persisterStores := []stores.PersisterStore{
 		stores.NewEventsStore(indexerData.Events, c.workingData.persistentEvents, c.executionResult.BlockID),
 		stores.NewResultsStore(indexerData.Results, c.workingData.persistentResults, c.executionResult.BlockID),
-		stores.NewCollectionsStore(indexerData.Collections, c.workingData.persistentCollections, c.workingData.lockManager),
+		stores.NewCollectionsStore(indexerData.Collections, c.workingData.persistentCollections),
 		stores.NewTxResultErrMsgStore(c.workingData.txResultErrMsgsData, c.workingData.persistentTxResultErrMsgs, c.executionResult.BlockID),
 		stores.NewLatestSealedResultStore(c.workingData.latestPersistedSealedResult, c.executionResult.ID(), c.block.Height),
 	}

module/executiondatasync/optimistic_sync/persisters/block_test.go

@@ -67,7 +67,11 @@ func (p *PersisterSuite) TestPersister_HappyPath() {
 }
 
 func (p *PersisterSuite) TestPersister_EmptyData() {
-	p.indexerData = &indexer.IndexerData{}
+	p.indexerData = &indexer.IndexerData{
+		// this is needed because the events storage caches an empty slice when no events are passed.
+		// without it, assert.Equals will fail because nil != empty slice
+		Events: []flow.Event{},
+	}
 	p.txErrMsgs = nil
 	p.testWithDatabase()
 }
@@ -106,7 +110,7 @@ func (p *PersisterSuite) testWithDatabase() {
 			[]stores.PersisterStore{
 				stores.NewEventsStore(p.indexerData.Events, events, p.executionResult.BlockID),
 				stores.NewResultsStore(p.indexerData.Results, results, p.executionResult.BlockID),
-				stores.NewCollectionsStore(p.indexerData.Collections, collections, lockManager),
+				stores.NewCollectionsStore(p.indexerData.Collections, collections),
 				stores.NewTxResultErrMsgStore(p.txErrMsgs, txResultErrMsg, p.executionResult.BlockID),
 				stores.NewLatestSealedResultStore(latestPersistedSealedResult, p.executionResult.ID(), p.header.Height),
 			},
@@ -186,7 +190,7 @@ func (p *PersisterSuite) TestPersister_ErrorHandling() {
 			lockManager,
 			p.executionResult,
 			[]stores.PersisterStore{
-				stores.NewCollectionsStore(p.indexerData.Collections, collections, lockManager),
+				stores.NewCollectionsStore(p.indexerData.Collections, collections),
 				stores.NewEventsStore(p.indexerData.Events, events, p.executionResult.BlockID),
 			},
 		)
@@ -208,7 +212,7 @@ func (p *PersisterSuite) TestPersister_ErrorHandling() {
 			lockManager,
 			p.executionResult,
 			[]stores.PersisterStore{
-				stores.NewCollectionsStore(p.indexerData.Collections, collections, lockManager),
+				stores.NewCollectionsStore(p.indexerData.Collections, collections),
 				stores.NewEventsStore(p.indexerData.Events, events, p.executionResult.BlockID),
 			},
 		)

module/executiondatasync/optimistic_sync/persisters/stores/events.go

@@ -34,10 +34,8 @@ func NewEventsStore(
 //
 // No error returns are expected during normal operations
 func (e *EventsStore) Persist(_ lockctx.Proof, batch storage.ReaderBatchWriter) error {
-	if len(e.data) > 0 {
-		if err := e.persistedEvents.BatchStore(e.blockID, []flow.EventsList{e.data}, batch); err != nil {
-			return fmt.Errorf("could not add events to batch: %w", err)
-		}
+	if err := e.persistedEvents.BatchStore(e.blockID, []flow.EventsList{e.data}, batch); err != nil {
+		return fmt.Errorf("could not add events to batch: %w", err)
 	}
 	return nil
 }

module/executiondatasync/optimistic_sync/persisters/stores/light_collections.go

@@ -20,7 +20,6 @@ type LightCollectionsStore struct {
 func NewCollectionsStore(
 	data []*flow.Collection,
 	persistedCollections storage.Collections,
-	lockManager storage.LockManager,
 ) *LightCollectionsStore {
 	return &LightCollectionsStore{
 		data:                 data,

module/executiondatasync/optimistic_sync/persisters/stores/persister_store.go

@@ -9,6 +9,7 @@ import (
 // PersisterStore is the interface to handle persisting of a data type to persisted storage using batch operation.
 type PersisterStore interface {
 	// Persist adds data to the batch for later commitment.
-	// No errors are expected during normal operations
+	//
+	// No error returns are expected during normal operations
 	Persist(lctx lockctx.Proof, batch storage.ReaderBatchWriter) error
 }

module/executiondatasync/optimistic_sync/persisters/stores/results.go

@@ -34,11 +34,8 @@ func NewResultsStore(
 //
 // No error returns are expected during normal operations
 func (r *ResultsStore) Persist(_ lockctx.Proof, batch storage.ReaderBatchWriter) error {
-	if len(r.data) > 0 {
-		if err := r.persistedResults.BatchStore(r.blockID, r.data, batch); err != nil {
-			return fmt.Errorf("could not add transaction results to batch: %w", err)
-		}
+	if err := r.persistedResults.BatchStore(r.blockID, r.data, batch); err != nil {
+		return fmt.Errorf("could not add transaction results to batch: %w", err)
 	}
-
 	return nil
 }

module/executiondatasync/optimistic_sync/persisters/stores/transaction_result_error_messages.go

@@ -34,11 +34,8 @@ func NewTxResultErrMsgStore(
 //
 // No error returns are expected during normal operations
 func (t *TxResultErrMsgStore) Persist(_ lockctx.Proof, batch storage.ReaderBatchWriter) error {
-	if len(t.data) > 0 {
-		if err := t.persistedTxResultErrMsg.BatchStore(t.blockID, t.data, batch); err != nil {
-			return fmt.Errorf("could not add transaction result error messages to batch: %w", err)
-		}
+	if err := t.persistedTxResultErrMsg.BatchStore(t.blockID, t.data, batch); err != nil {
+		return fmt.Errorf("could not add transaction result error messages to batch: %w", err)
 	}
-
 	return nil
 }