From 7359754b7a22a42b7cf3a4fa02bfce8482a224fa Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Sun, 10 Nov 2024 22:34:23 +0200 Subject: [PATCH 01/35] Added basic factory and data providers for blocks --- .../rest/common/convert/block_status.go | 22 ++ .../rest/common/convert/block_status_test.go | 39 +++ .../{http/request => common/convert}/id.go | 2 +- .../request => common/convert}/id_test.go | 2 +- engine/access/rest/http/request/get_block.go | 3 +- engine/access/rest/http/request/get_events.go | 3 +- .../rest/http/request/get_execution_result.go | 3 +- engine/access/rest/http/request/get_script.go | 3 +- .../rest/http/request/get_transaction.go | 5 +- engine/access/rest/http/request/helpers.go | 3 +- .../access/rest/http/request/transaction.go | 3 +- engine/access/rest/server.go | 4 + .../data_providers/base_provider.go | 99 ++++++ .../data_providers/blocks_data_provider.go | 282 ++++++++++++++++++ .../data_providers/data_provider.go | 7 + .../rest/websockets/data_providers/factory.go | 70 +++++ .../legacy/request/subscribe_events.go | 3 +- .../rest/websockets/models/block_models.go | 10 + engine/access/rest_api_test.go | 14 +- 19 files changed, 559 insertions(+), 18 deletions(-) create mode 100644 engine/access/rest/common/convert/block_status.go create mode 100644 engine/access/rest/common/convert/block_status_test.go rename engine/access/rest/{http/request => common/convert}/id.go (98%) rename engine/access/rest/{http/request => common/convert}/id_test.go (98%) create mode 100644 engine/access/rest/websockets/data_providers/base_provider.go create mode 100644 engine/access/rest/websockets/data_providers/blocks_data_provider.go create mode 100644 engine/access/rest/websockets/data_providers/data_provider.go create mode 100644 engine/access/rest/websockets/data_providers/factory.go create mode 100644 engine/access/rest/websockets/models/block_models.go diff --git a/engine/access/rest/common/convert/block_status.go b/engine/access/rest/common/convert/block_status.go new file mode 100644 index 00000000000..762508797fc --- /dev/null +++ b/engine/access/rest/common/convert/block_status.go @@ -0,0 +1,22 @@ +package convert + +import ( + "fmt" + + "github.com/onflow/flow-go/model/flow" +) + +const ( + Finalized = "finalized" + Sealed = "sealed" +) + +func ParseBlockStatus(blockStatus string) (flow.BlockStatus, error) { + switch blockStatus { + case Finalized: + return flow.BlockStatusFinalized, nil + case Sealed: + return flow.BlockStatusSealed, nil + } + return flow.BlockStatusUnknown, fmt.Errorf("invalid 'block_status', must be '%s' or '%s'", Finalized, Sealed) +} diff --git a/engine/access/rest/common/convert/block_status_test.go b/engine/access/rest/common/convert/block_status_test.go new file mode 100644 index 00000000000..3313bbc788c --- /dev/null +++ b/engine/access/rest/common/convert/block_status_test.go @@ -0,0 +1,39 @@ +package convert + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/onflow/flow-go/model/flow" +) + +// TestParseBlockStatus_Invalid tests the ParseBlockStatus function with invalid inputs. +// It verifies that for each invalid block status string, the function returns an error +// matching the expected error message format. +func TestParseBlockStatus_Invalid(t *testing.T) { + tests := []string{"unknown", "pending", ""} + expectedErr := fmt.Sprintf("invalid 'block_status', must be '%s' or '%s'", Finalized, Sealed) + + for _, input := range tests { + _, err := ParseBlockStatus(input) + assert.EqualError(t, err, expectedErr) + } +} + +// TestParseBlockStatus_Valid tests the ParseBlockStatus function with valid inputs. +// It ensures that the function returns the correct flow.BlockStatus for valid status +// strings "finalized" and "sealed" without errors. +func TestParseBlockStatus_Valid(t *testing.T) { + tests := map[string]flow.BlockStatus{ + Finalized: flow.BlockStatusFinalized, + Sealed: flow.BlockStatusSealed, + } + + for input, expectedStatus := range tests { + status, err := ParseBlockStatus(input) + assert.NoError(t, err) + assert.Equal(t, expectedStatus, status) + } +} diff --git a/engine/access/rest/http/request/id.go b/engine/access/rest/common/convert/id.go similarity index 98% rename from engine/access/rest/http/request/id.go rename to engine/access/rest/common/convert/id.go index ba3c1200527..b0d7c2bbbf9 100644 --- a/engine/access/rest/http/request/id.go +++ b/engine/access/rest/common/convert/id.go @@ -1,4 +1,4 @@ -package request +package convert import ( "errors" diff --git a/engine/access/rest/http/request/id_test.go b/engine/access/rest/common/convert/id_test.go similarity index 98% rename from engine/access/rest/http/request/id_test.go rename to engine/access/rest/common/convert/id_test.go index 1096fdbe696..70621ddcbb5 100644 --- a/engine/access/rest/http/request/id_test.go +++ b/engine/access/rest/common/convert/id_test.go @@ -1,4 +1,4 @@ -package request +package convert import ( "testing" diff --git a/engine/access/rest/http/request/get_block.go b/engine/access/rest/http/request/get_block.go index fd74b0e4be0..903bfd6a02c 100644 --- a/engine/access/rest/http/request/get_block.go +++ b/engine/access/rest/http/request/get_block.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/onflow/flow-go/engine/access/rest/common" + "github.com/onflow/flow-go/engine/access/rest/common/convert" "github.com/onflow/flow-go/model/flow" ) @@ -122,7 +123,7 @@ func (g *GetBlockByIDs) Build(r *common.Request) error { } func (g *GetBlockByIDs) Parse(rawIds []string) error { - var ids IDs + var ids convert.IDs err := ids.Parse(rawIds) if err != nil { return err diff --git a/engine/access/rest/http/request/get_events.go b/engine/access/rest/http/request/get_events.go index 39f2ba9faef..f5aadf31369 100644 --- a/engine/access/rest/http/request/get_events.go +++ b/engine/access/rest/http/request/get_events.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/onflow/flow-go/engine/access/rest/common" + "github.com/onflow/flow-go/engine/access/rest/common/convert" "github.com/onflow/flow-go/model/flow" ) @@ -50,7 +51,7 @@ func (g *GetEvents) Parse(rawType string, rawStart string, rawEnd string, rawBlo } g.EndHeight = height.Flow() - var blockIDs IDs + var blockIDs convert.IDs err = blockIDs.Parse(rawBlockIDs) if err != nil { return err diff --git a/engine/access/rest/http/request/get_execution_result.go b/engine/access/rest/http/request/get_execution_result.go index cdf216766c1..8feb7aac51f 100644 --- a/engine/access/rest/http/request/get_execution_result.go +++ b/engine/access/rest/http/request/get_execution_result.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/onflow/flow-go/engine/access/rest/common" + "github.com/onflow/flow-go/engine/access/rest/common/convert" "github.com/onflow/flow-go/model/flow" ) @@ -30,7 +31,7 @@ func (g *GetExecutionResultByBlockIDs) Build(r *common.Request) error { } func (g *GetExecutionResultByBlockIDs) Parse(rawIDs []string) error { - var ids IDs + var ids convert.IDs err := ids.Parse(rawIDs) if err != nil { return err diff --git a/engine/access/rest/http/request/get_script.go b/engine/access/rest/http/request/get_script.go index de8da72cac1..01fbf996601 100644 --- a/engine/access/rest/http/request/get_script.go +++ b/engine/access/rest/http/request/get_script.go @@ -5,6 +5,7 @@ import ( "io" "github.com/onflow/flow-go/engine/access/rest/common" + "github.com/onflow/flow-go/engine/access/rest/common/convert" "github.com/onflow/flow-go/model/flow" ) @@ -42,7 +43,7 @@ func (g *GetScript) Parse(rawHeight string, rawID string, rawScript io.Reader) e } g.BlockHeight = height.Flow() - var id ID + var id convert.ID err = id.Parse(rawID) if err != nil { return err diff --git a/engine/access/rest/http/request/get_transaction.go b/engine/access/rest/http/request/get_transaction.go index 359570cd71d..ba80fad0105 100644 --- a/engine/access/rest/http/request/get_transaction.go +++ b/engine/access/rest/http/request/get_transaction.go @@ -2,6 +2,7 @@ package request import ( "github.com/onflow/flow-go/engine/access/rest/common" + "github.com/onflow/flow-go/engine/access/rest/common/convert" "github.com/onflow/flow-go/model/flow" ) @@ -15,14 +16,14 @@ type TransactionOptionals struct { } func (t *TransactionOptionals) Parse(r *common.Request) error { - var blockId ID + var blockId convert.ID err := blockId.Parse(r.GetQueryParam(blockIDQueryParam)) if err != nil { return err } t.BlockID = blockId.Flow() - var collectionId ID + var collectionId convert.ID err = collectionId.Parse(r.GetQueryParam(collectionIDQueryParam)) if err != nil { return err diff --git a/engine/access/rest/http/request/helpers.go b/engine/access/rest/http/request/helpers.go index 5591cc6df9b..8ad33bb1d81 100644 --- a/engine/access/rest/http/request/helpers.go +++ b/engine/access/rest/http/request/helpers.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/onflow/flow-go/engine/access/rest/common" + "github.com/onflow/flow-go/engine/access/rest/common/convert" "github.com/onflow/flow-go/model/flow" ) @@ -60,7 +61,7 @@ func (g *GetByIDRequest) Build(r *common.Request) error { } func (g *GetByIDRequest) Parse(rawID string) error { - var id ID + var id convert.ID err := id.Parse(rawID) if err != nil { return err diff --git a/engine/access/rest/http/request/transaction.go b/engine/access/rest/http/request/transaction.go index 614d78f1e07..1ebe595da81 100644 --- a/engine/access/rest/http/request/transaction.go +++ b/engine/access/rest/http/request/transaction.go @@ -4,6 +4,7 @@ import ( "fmt" "io" + convert2 "github.com/onflow/flow-go/engine/access/rest/common/convert" "github.com/onflow/flow-go/engine/access/rest/http/models" "github.com/onflow/flow-go/engine/access/rest/util" "github.com/onflow/flow-go/engine/common/rpc/convert" @@ -89,7 +90,7 @@ func (t *Transaction) Parse(raw io.Reader, chain flow.Chain) error { return fmt.Errorf("invalid transaction script encoding") } - var blockID ID + var blockID convert2.ID err = blockID.Parse(tx.ReferenceBlockId) if err != nil { return fmt.Errorf("invalid reference block ID: %w", err) diff --git a/engine/access/rest/server.go b/engine/access/rest/server.go index d25044a60a5..f3c0a79194a 100644 --- a/engine/access/rest/server.go +++ b/engine/access/rest/server.go @@ -9,6 +9,7 @@ import ( "github.com/onflow/flow-go/access" "github.com/onflow/flow-go/engine/access/rest/router" + "github.com/onflow/flow-go/engine/access/rest/websockets/data_providers" "github.com/onflow/flow-go/engine/access/state_stream" "github.com/onflow/flow-go/engine/access/state_stream/backend" "github.com/onflow/flow-go/model/flow" @@ -48,6 +49,9 @@ func NewServer(serverAPI access.API, builder.AddWsLegacyRoutes(stateStreamApi, chain, stateStreamConfig, config.MaxRequestSize) } + // TODO: add new websocket routes + _ = data_providers.NewDataProviderFactory(logger, stateStreamConfig.EventFilterConfig, stateStreamApi, serverAPI) + c := cors.New(cors.Options{ AllowedOrigins: []string{"*"}, AllowedHeaders: []string{"*"}, diff --git a/engine/access/rest/websockets/data_providers/base_provider.go b/engine/access/rest/websockets/data_providers/base_provider.go new file mode 100644 index 00000000000..a30cf9a6887 --- /dev/null +++ b/engine/access/rest/websockets/data_providers/base_provider.go @@ -0,0 +1,99 @@ +package data_providers + +import ( + "context" + "fmt" + + "github.com/onflow/flow-go/access" + "github.com/onflow/flow-go/engine/access/subscription" +) + +// BaseDataProvider defines the basic interface for a data provider. It provides methods +// for retrieving the provider's unique ID, topic, and a method to close the provider. +type BaseDataProvider interface { + // ID returns the unique identifier of subscription in the data provider. + ID() string + // Topic returns the topic associated with the data provider. + Topic() string + // Close terminates the data provider. + Close() +} + +var _ BaseDataProvider = (*BaseDataProviderImpl)(nil) + +// BaseDataProviderImpl is the concrete implementation of the BaseDataProvider interface. +// It holds common objects for the provider. +type BaseDataProviderImpl struct { + topic string + + cancel context.CancelFunc + ctx context.Context + + api access.API + send chan<- interface{} + subscription subscription.Subscription +} + +// NewBaseDataProviderImpl creates a new instance of BaseDataProviderImpl. +func NewBaseDataProviderImpl( + ctx context.Context, + cancel context.CancelFunc, + api access.API, + topic string, + send chan<- interface{}, + subscription subscription.Subscription, +) *BaseDataProviderImpl { + return &BaseDataProviderImpl{ + topic: topic, + + ctx: ctx, + cancel: cancel, + + api: api, + send: send, + subscription: subscription, + } +} + +// ID returns the unique identifier of the data provider's subscription. +func (b *BaseDataProviderImpl) ID() string { + return b.subscription.ID() +} + +// Topic returns the topic associated with the data provider. +func (b *BaseDataProviderImpl) Topic() string { + return b.topic +} + +// Close terminates the data provider. +func (b *BaseDataProviderImpl) Close() { + b.cancel() +} + +// TODO: refactor rpc version of HandleSubscription and use it +func HandleSubscription[T any](ctx context.Context, sub subscription.Subscription, handleResponse func(resp T) error) error { + for { + select { + case v, ok := <-sub.Channel(): + if !ok { + if sub.Err() != nil { + return fmt.Errorf("stream encountered an error: %w", sub.Err()) + } + return nil + } + + resp, ok := v.(T) + if !ok { + return fmt.Errorf("unexpected subscription response type: %T", v) + } + + err := handleResponse(resp) + if err != nil { + return err + } + case <-ctx.Done(): + // context closed, subscription closed + return nil + } + } +} diff --git a/engine/access/rest/websockets/data_providers/blocks_data_provider.go b/engine/access/rest/websockets/data_providers/blocks_data_provider.go new file mode 100644 index 00000000000..f5a24139a87 --- /dev/null +++ b/engine/access/rest/websockets/data_providers/blocks_data_provider.go @@ -0,0 +1,282 @@ +package data_providers + +import ( + "context" + "fmt" + + "github.com/rs/zerolog" + + "github.com/onflow/flow-go/access" + "github.com/onflow/flow-go/engine/access/rest/common/convert" + "github.com/onflow/flow-go/engine/access/rest/util" + "github.com/onflow/flow-go/engine/access/rest/websockets/models" + "github.com/onflow/flow-go/model/flow" +) + +// BlocksFromStartBlockIDArgs contains the arguments required for subscribing to blocks +// starting from a specific block ID. +type BlocksFromStartBlockIDArgs struct { + StartBlockID flow.Identifier + BlockStatus flow.BlockStatus +} + +// BlocksFromStartBlockIDProvider is responsible for providing blocks starting +// from a specific block ID. +type BlocksFromStartBlockIDProvider struct { + *BaseDataProviderImpl + + logger zerolog.Logger + args BlocksFromStartBlockIDArgs +} + +var _ DataProvider = (*BlocksFromStartBlockIDProvider)(nil) + +// NewBlocksFromStartBlockIDProvider creates a new instance of BlocksFromStartBlockIDProvider. +func NewBlocksFromStartBlockIDProvider( + ctx context.Context, + logger zerolog.Logger, + api access.API, + topic string, + arguments map[string]string, + send chan<- interface{}, +) (*BlocksFromStartBlockIDProvider, error) { + ctx, cancel := context.WithCancel(ctx) + + p := &BlocksFromStartBlockIDProvider{ + logger: logger.With().Str("component", "block-from-start-block-id-provider").Logger(), + } + + // Validate arguments passed to the provider. + err := p.validateArguments(arguments) + if err != nil { + return nil, fmt.Errorf("invalid arguments: %w", err) + } + + // Subscribe to blocks from the start block ID with the specified block status. + subscription := p.api.SubscribeBlocksFromStartBlockID(ctx, p.args.StartBlockID, p.args.BlockStatus) + p.BaseDataProviderImpl = NewBaseDataProviderImpl( + ctx, + cancel, + api, + topic, + send, + subscription, + ) + + return p, nil +} + +// Run starts processing the subscription for blocks from the start block ID and handles responses. +// +// No errors are expected during normal operations. +func (p *BlocksFromStartBlockIDProvider) Run() error { + return HandleSubscription(p.ctx, p.subscription, handleResponse(p.send, p.args.BlockStatus)) +} + +// validateArguments checks and validates the arguments passed to the provider. +// +// No errors are expected during normal operations. +func (p *BlocksFromStartBlockIDProvider) validateArguments(arguments map[string]string) error { + // Check for block_status argument and validate + if blockStatusIn, ok := arguments["block_status"]; ok { + blockStatus, err := convert.ParseBlockStatus(blockStatusIn) + if err != nil { + return err + } + p.args.BlockStatus = blockStatus + } else { + return fmt.Errorf("'block_status' must be provided") + } + + // Check for start_block_id argument and validate + if startBlockIDIn, ok := arguments["start_block_id"]; ok { + var startBlockID convert.ID + err := startBlockID.Parse(startBlockIDIn) + if err != nil { + return err + } + p.args.StartBlockID = startBlockID.Flow() + } else { + return fmt.Errorf("'start_block_id' must be provided") + } + + return nil +} + +// BlocksFromBlockHeightArgs contains the arguments required for subscribing to blocks +// starting from a specific block height. +type BlocksFromBlockHeightArgs struct { + StartBlockHeight uint64 + BlockStatus flow.BlockStatus +} + +// BlocksFromStartBlockHeightProvider is responsible for providing blocks starting +// from a specific block height. +type BlocksFromStartBlockHeightProvider struct { + *BaseDataProviderImpl + + logger zerolog.Logger + args BlocksFromBlockHeightArgs +} + +var _ DataProvider = (*BlocksFromStartBlockHeightProvider)(nil) + +// NewBlocksFromStartBlockHeightProvider creates a new instance of BlocksFromStartBlockHeightProvider. +func NewBlocksFromStartBlockHeightProvider( + ctx context.Context, + logger zerolog.Logger, + api access.API, + topic string, + arguments map[string]string, + send chan<- interface{}, +) (*BlocksFromStartBlockHeightProvider, error) { + ctx, cancel := context.WithCancel(ctx) + + p := &BlocksFromStartBlockHeightProvider{ + logger: logger.With().Str("component", "block-from-start-block-height-provider").Logger(), + } + + // Validate arguments passed to the provider. + err := p.validateArguments(arguments) + if err != nil { + return nil, fmt.Errorf("invalid arguments: %w", err) + } + + // Subscribe to blocks from the start block height with the specified block status. + subscription := p.api.SubscribeBlocksFromStartHeight(ctx, p.args.StartBlockHeight, p.args.BlockStatus) + p.BaseDataProviderImpl = NewBaseDataProviderImpl( + ctx, + cancel, + api, + topic, + send, + subscription, + ) + + return p, nil +} + +// Run starts processing the subscription for blocks from the start block height and handles responses. +// +// No errors are expected during normal operations. +func (p *BlocksFromStartBlockHeightProvider) Run() error { + return HandleSubscription(p.ctx, p.subscription, handleResponse(p.send, p.args.BlockStatus)) +} + +// validateArguments checks and validates the arguments passed to the provider. +// +// No errors are expected during normal operations. +func (p *BlocksFromStartBlockHeightProvider) validateArguments(arguments map[string]string) error { + // Check for block_status argument and validate + if blockStatusIn, ok := arguments["block_status"]; ok { + blockStatus, err := convert.ParseBlockStatus(blockStatusIn) + if err != nil { + return err + } + p.args.BlockStatus = blockStatus + } else { + return fmt.Errorf("'block_status' must be provided") + } + + if startBlockHeightIn, ok := arguments["start_block_height"]; ok { + var err error + p.args.StartBlockHeight, err = util.ToUint64(startBlockHeightIn) + if err != nil { + return fmt.Errorf("invalid start height: %w", err) + } + } else { + return fmt.Errorf("'start_block_height' must be provided") + } + + return nil +} + +// BlocksFromLatestArgs contains the arguments required for subscribing to blocks +// starting from latest block. +type BlocksFromLatestArgs struct { + BlockStatus flow.BlockStatus +} + +// BlocksFromLatestProvider is responsible for providing blocks starting from latest block. +type BlocksFromLatestProvider struct { + *BaseDataProviderImpl + + logger zerolog.Logger + args BlocksFromLatestArgs +} + +var _ DataProvider = (*BlocksFromLatestProvider)(nil) + +// NewBlocksFromLatestProvider creates a new BlocksFromLatestProvider. +func NewBlocksFromLatestProvider( + ctx context.Context, + logger zerolog.Logger, + api access.API, + topic string, + arguments map[string]string, + send chan<- interface{}, +) (*BlocksFromLatestProvider, error) { + ctx, cancel := context.WithCancel(ctx) + + p := &BlocksFromLatestProvider{ + logger: logger.With().Str("component", "block-from-latest-provider").Logger(), + } + + // Validate arguments passed to the provider. + err := p.validateArguments(arguments) + if err != nil { + return nil, fmt.Errorf("invalid arguments: %w", err) + } + + // Subscribe to blocks from the latest block height with the specified block status. + subscription := p.api.SubscribeBlocksFromLatest(ctx, p.args.BlockStatus) + p.BaseDataProviderImpl = NewBaseDataProviderImpl( + ctx, + cancel, + api, + topic, + send, + subscription, + ) + + return p, nil +} + +// Run starts processing the subscription for blocks from the latest block and handles responses. +// +// No errors are expected during normal operations. +func (p *BlocksFromLatestProvider) Run() error { + return HandleSubscription(p.ctx, p.subscription, handleResponse(p.send, p.args.BlockStatus)) +} + +// validateArguments checks and validates the arguments passed to the provider. +// +// No errors are expected during normal operations. +func (p *BlocksFromLatestProvider) validateArguments(arguments map[string]string) error { + // Check for block_status argument and validate + if blockStatusIn, ok := arguments["block_status"]; ok { + blockStatus, err := convert.ParseBlockStatus(blockStatusIn) + if err != nil { + return err + } + p.args.BlockStatus = blockStatus + } else { + return fmt.Errorf("'block_status' must be provided") + } + + return nil +} + +// handleResponse processes a block and sends the formatted response. +// +// No errors are expected during normal operations. +func handleResponse(send chan<- interface{}, blockStatus flow.BlockStatus) func(*flow.Block) error { + return func(block *flow.Block) error { + send <- &models.BlockMessageResponse{ + Block: block, + BlockStatus: blockStatus, + } + + return nil + } +} diff --git a/engine/access/rest/websockets/data_providers/data_provider.go b/engine/access/rest/websockets/data_providers/data_provider.go new file mode 100644 index 00000000000..ba7dd815c9b --- /dev/null +++ b/engine/access/rest/websockets/data_providers/data_provider.go @@ -0,0 +1,7 @@ +package data_providers + +type DataProvider interface { + BaseDataProvider + + Run() error +} diff --git a/engine/access/rest/websockets/data_providers/factory.go b/engine/access/rest/websockets/data_providers/factory.go new file mode 100644 index 00000000000..5b9d0bad1ba --- /dev/null +++ b/engine/access/rest/websockets/data_providers/factory.go @@ -0,0 +1,70 @@ +package data_providers + +import ( + "context" + "fmt" + + "github.com/rs/zerolog" + + "github.com/onflow/flow-go/access" + "github.com/onflow/flow-go/engine/access/state_stream" +) + +const ( + EventsTopic = "events" + AccountStatusesTopic = "account_statuses" + BlockHeadersTopic = "block_headers" + BlockDigestsTopic = "block_digests" + TransactionStatusesTopic = "transaction_statuses" + + BlocksFromStartBlockIDTopic = "blocks_from_start_block_id" + BlocksFromStartBlockHeightTopic = "blocks_from_start_block_height" + BlocksFromLatestTopic = "blocks_from_latest" +) + +type DataProviderFactory struct { + logger zerolog.Logger + eventFilterConfig state_stream.EventFilterConfig + + stateStreamApi state_stream.API + accessApi access.API +} + +func NewDataProviderFactory( + logger zerolog.Logger, + eventFilterConfig state_stream.EventFilterConfig, + stateStreamApi state_stream.API, + accessApi access.API, +) *DataProviderFactory { + return &DataProviderFactory{ + logger: logger, + eventFilterConfig: eventFilterConfig, + stateStreamApi: stateStreamApi, + accessApi: accessApi, + } +} + +func (s *DataProviderFactory) NewDataProvider( + ctx context.Context, + topic string, + arguments map[string]string, + ch chan<- interface{}, +) (DataProvider, error) { + switch topic { + case BlocksFromStartBlockIDTopic: + return NewBlocksFromStartBlockIDProvider(ctx, s.logger, s.accessApi, topic, arguments, ch) + case BlocksFromStartBlockHeightTopic: + return NewBlocksFromStartBlockHeightProvider(ctx, s.logger, s.accessApi, topic, arguments, ch) + case BlocksFromLatestTopic: + return NewBlocksFromLatestProvider(ctx, s.logger, s.accessApi, topic, arguments, ch) + // TODO: Implemented handlers for each topic should be added in respective case + case EventsTopic, + AccountStatusesTopic, + BlockHeadersTopic, + BlockDigestsTopic, + TransactionStatusesTopic: + return nil, fmt.Errorf("topic \"%s\" not implemented yet", topic) + default: + return nil, fmt.Errorf("unsupported topic \"%s\"", topic) + } +} diff --git a/engine/access/rest/websockets/legacy/request/subscribe_events.go b/engine/access/rest/websockets/legacy/request/subscribe_events.go index 5b2574ccc82..7f02ad1c10e 100644 --- a/engine/access/rest/websockets/legacy/request/subscribe_events.go +++ b/engine/access/rest/websockets/legacy/request/subscribe_events.go @@ -5,6 +5,7 @@ import ( "strconv" "github.com/onflow/flow-go/engine/access/rest/common" + "github.com/onflow/flow-go/engine/access/rest/common/convert" "github.com/onflow/flow-go/engine/access/rest/http/request" "github.com/onflow/flow-go/model/flow" ) @@ -56,7 +57,7 @@ func (g *SubscribeEvents) Parse( rawContracts []string, rawHeartbeatInterval string, ) error { - var startBlockID request.ID + var startBlockID convert.ID err := startBlockID.Parse(rawStartBlockID) if err != nil { return err diff --git a/engine/access/rest/websockets/models/block_models.go b/engine/access/rest/websockets/models/block_models.go new file mode 100644 index 00000000000..f363038808b --- /dev/null +++ b/engine/access/rest/websockets/models/block_models.go @@ -0,0 +1,10 @@ +package models + +import ( + "github.com/onflow/flow-go/model/flow" +) + +type BlockMessageResponse struct { + Block *flow.Block `json:"block"` + BlockStatus flow.BlockStatus `json:"block_status"` +} diff --git a/engine/access/rest_api_test.go b/engine/access/rest_api_test.go index 64dab073c1d..9c8eaa20881 100644 --- a/engine/access/rest_api_test.go +++ b/engine/access/rest_api_test.go @@ -22,7 +22,7 @@ import ( accessmock "github.com/onflow/flow-go/engine/access/mock" "github.com/onflow/flow-go/engine/access/rest" "github.com/onflow/flow-go/engine/access/rest/common" - "github.com/onflow/flow-go/engine/access/rest/http/request" + "github.com/onflow/flow-go/engine/access/rest/common/convert" "github.com/onflow/flow-go/engine/access/rest/router" "github.com/onflow/flow-go/engine/access/rpc" "github.com/onflow/flow-go/engine/access/rpc/backend" @@ -230,8 +230,8 @@ func TestRestAPI(t *testing.T) { func (suite *RestAPITestSuite) TestGetBlock() { - testBlockIDs := make([]string, request.MaxIDsLength) - testBlocks := make([]*flow.Block, request.MaxIDsLength) + testBlockIDs := make([]string, convert.MaxIDsLength) + testBlocks := make([]*flow.Block, convert.MaxIDsLength) for i := range testBlockIDs { collections := unittest.CollectionListFixture(1) block := unittest.BlockWithGuaranteesFixture( @@ -281,7 +281,7 @@ func (suite *RestAPITestSuite) TestGetBlock() { actualBlocks, resp, err := client.BlocksApi.BlocksIdGet(ctx, blockIDSlice, optionsForBlockByID()) require.NoError(suite.T(), err) assert.Equal(suite.T(), http.StatusOK, resp.StatusCode) - assert.Len(suite.T(), actualBlocks, request.MaxIDsLength) + assert.Len(suite.T(), actualBlocks, convert.MaxIDsLength) for i, b := range testBlocks { assert.Equal(suite.T(), b.ID().String(), actualBlocks[i].Header.Id) } @@ -379,13 +379,13 @@ func (suite *RestAPITestSuite) TestGetBlock() { ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) defer cancel() - blockIDs := make([]string, request.MaxIDsLength+1) + blockIDs := make([]string, convert.MaxIDsLength+1) copy(blockIDs, testBlockIDs) - blockIDs[request.MaxIDsLength] = unittest.IdentifierFixture().String() + blockIDs[convert.MaxIDsLength] = unittest.IdentifierFixture().String() blockIDSlice := []string{strings.Join(blockIDs, ",")} _, resp, err := client.BlocksApi.BlocksIdGet(ctx, blockIDSlice, optionsForBlockByID()) - assertError(suite.T(), resp, err, http.StatusBadRequest, fmt.Sprintf("at most %d IDs can be requested at a time", request.MaxIDsLength)) + assertError(suite.T(), resp, err, http.StatusBadRequest, fmt.Sprintf("at most %d IDs can be requested at a time", convert.MaxIDsLength)) }) suite.Run("GetBlockByID with one non-existing block ID", func() { From 9f9154015655a31a2726755c566a7b20e6c3f7fd Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Mon, 11 Nov 2024 10:40:34 +0200 Subject: [PATCH 02/35] Added godoc for DataProvider interface --- .../websockets/data_providers/blocks_data_provider_test.go | 0 engine/access/rest/websockets/data_providers/data_provider.go | 4 ++++ 2 files changed, 4 insertions(+) create mode 100644 engine/access/rest/websockets/data_providers/blocks_data_provider_test.go diff --git a/engine/access/rest/websockets/data_providers/blocks_data_provider_test.go b/engine/access/rest/websockets/data_providers/blocks_data_provider_test.go new file mode 100644 index 00000000000..e69de29bb2d diff --git a/engine/access/rest/websockets/data_providers/data_provider.go b/engine/access/rest/websockets/data_providers/data_provider.go index ba7dd815c9b..11a8bc16c38 100644 --- a/engine/access/rest/websockets/data_providers/data_provider.go +++ b/engine/access/rest/websockets/data_providers/data_provider.go @@ -1,7 +1,11 @@ package data_providers +// The DataProvider is the interface abstracts of the actual subscriptions used by the WebSocketCollector. type DataProvider interface { BaseDataProvider + // Run starts processing the subscription and handles responses. + // + // No errors are expected during normal operations. Run() error } From 7502a798ab9f74b3a5f482408259428716f94a2b Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Mon, 11 Nov 2024 11:30:02 +0200 Subject: [PATCH 03/35] Added godoc for factory --- .../rest/websockets/data_providers/factory.go | 22 +++++++++++++++++++ .../websockets/data_providers/factory_test.go | 0 2 files changed, 22 insertions(+) create mode 100644 engine/access/rest/websockets/data_providers/factory_test.go diff --git a/engine/access/rest/websockets/data_providers/factory.go b/engine/access/rest/websockets/data_providers/factory.go index 5b9d0bad1ba..d8285743d5e 100644 --- a/engine/access/rest/websockets/data_providers/factory.go +++ b/engine/access/rest/websockets/data_providers/factory.go @@ -10,6 +10,8 @@ import ( "github.com/onflow/flow-go/engine/access/state_stream" ) +// Constants defining various topic names used to specify different types of +// data providers. const ( EventsTopic = "events" AccountStatusesTopic = "account_statuses" @@ -22,6 +24,9 @@ const ( BlocksFromLatestTopic = "blocks_from_latest" ) +// DataProviderFactory is responsible for creating data providers based on the +// requested topic. It manages access to logging, state stream configuration, +// and relevant APIs needed to retrieve data. type DataProviderFactory struct { logger zerolog.Logger eventFilterConfig state_stream.EventFilterConfig @@ -30,6 +35,13 @@ type DataProviderFactory struct { accessApi access.API } +// NewDataProviderFactory creates a new DataProviderFactory +// +// Parameters: +// - logger: Used for logging within the data providers. +// - eventFilterConfig: Configuration for filtering events from state streams. +// - stateStreamApi: API for accessing data from the Flow state stream API. +// - accessApi: API for accessing data from the Flow Access API. func NewDataProviderFactory( logger zerolog.Logger, eventFilterConfig state_stream.EventFilterConfig, @@ -44,6 +56,16 @@ func NewDataProviderFactory( } } +// NewDataProvider creates a new data provider based on the specified topic +// and configuration parameters. +// +// Parameters: +// - ctx: Context for managing request lifetime and cancellation. +// - topic: The topic for which a data provider is to be created. +// - arguments: Configuration arguments for the data provider. +// - ch: Channel to which the data provider sends data. +// +// No errors are expected during normal operations. func (s *DataProviderFactory) NewDataProvider( ctx context.Context, topic string, diff --git a/engine/access/rest/websockets/data_providers/factory_test.go b/engine/access/rest/websockets/data_providers/factory_test.go new file mode 100644 index 00000000000..e69de29bb2d From c30b1a00adfa3a1131a5041e89eeba12c933a0a6 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 12 Nov 2024 10:45:02 +0200 Subject: [PATCH 04/35] Refacored BlockProvider, added part of unit tests to provider factory and block provider --- .../{convert => parser}/block_status.go | 2 +- .../{convert => parser}/block_status_test.go | 2 +- .../rest/common/{convert => parser}/id.go | 2 +- .../common/{convert => parser}/id_test.go | 2 +- engine/access/rest/http/request/get_block.go | 4 +- engine/access/rest/http/request/get_events.go | 4 +- .../rest/http/request/get_execution_result.go | 4 +- engine/access/rest/http/request/get_script.go | 4 +- .../rest/http/request/get_transaction.go | 6 +- engine/access/rest/http/request/helpers.go | 4 +- .../access/rest/http/request/transaction.go | 2 +- engine/access/rest/http/routes/events.go | 1 - .../data_providers/blocks_data_provider.go | 282 ------------------ .../blocks_data_provider_test.go | 0 .../data_providers/blocks_provider.go | 145 +++++++++ .../data_providers/blocks_provider_test.go | 147 +++++++++ 16 files changed, 310 insertions(+), 301 deletions(-) rename engine/access/rest/common/{convert => parser}/block_status.go (96%) rename engine/access/rest/common/{convert => parser}/block_status_test.go (98%) rename engine/access/rest/common/{convert => parser}/id.go (98%) rename engine/access/rest/common/{convert => parser}/id_test.go (98%) delete mode 100644 engine/access/rest/websockets/data_providers/blocks_data_provider.go delete mode 100644 engine/access/rest/websockets/data_providers/blocks_data_provider_test.go create mode 100644 engine/access/rest/websockets/data_providers/blocks_provider.go create mode 100644 engine/access/rest/websockets/data_providers/blocks_provider_test.go diff --git a/engine/access/rest/common/convert/block_status.go b/engine/access/rest/common/parser/block_status.go similarity index 96% rename from engine/access/rest/common/convert/block_status.go rename to engine/access/rest/common/parser/block_status.go index 762508797fc..a1b6e8a7b46 100644 --- a/engine/access/rest/common/convert/block_status.go +++ b/engine/access/rest/common/parser/block_status.go @@ -1,4 +1,4 @@ -package convert +package parser import ( "fmt" diff --git a/engine/access/rest/common/convert/block_status_test.go b/engine/access/rest/common/parser/block_status_test.go similarity index 98% rename from engine/access/rest/common/convert/block_status_test.go rename to engine/access/rest/common/parser/block_status_test.go index 3313bbc788c..0bbaa30c56b 100644 --- a/engine/access/rest/common/convert/block_status_test.go +++ b/engine/access/rest/common/parser/block_status_test.go @@ -1,4 +1,4 @@ -package convert +package parser import ( "fmt" diff --git a/engine/access/rest/common/convert/id.go b/engine/access/rest/common/parser/id.go similarity index 98% rename from engine/access/rest/common/convert/id.go rename to engine/access/rest/common/parser/id.go index b0d7c2bbbf9..7b1436b4761 100644 --- a/engine/access/rest/common/convert/id.go +++ b/engine/access/rest/common/parser/id.go @@ -1,4 +1,4 @@ -package convert +package parser import ( "errors" diff --git a/engine/access/rest/common/convert/id_test.go b/engine/access/rest/common/parser/id_test.go similarity index 98% rename from engine/access/rest/common/convert/id_test.go rename to engine/access/rest/common/parser/id_test.go index 70621ddcbb5..a663c915e7a 100644 --- a/engine/access/rest/common/convert/id_test.go +++ b/engine/access/rest/common/parser/id_test.go @@ -1,4 +1,4 @@ -package convert +package parser import ( "testing" diff --git a/engine/access/rest/http/request/get_block.go b/engine/access/rest/http/request/get_block.go index 903bfd6a02c..972cd2ee97b 100644 --- a/engine/access/rest/http/request/get_block.go +++ b/engine/access/rest/http/request/get_block.go @@ -4,7 +4,7 @@ import ( "fmt" "github.com/onflow/flow-go/engine/access/rest/common" - "github.com/onflow/flow-go/engine/access/rest/common/convert" + "github.com/onflow/flow-go/engine/access/rest/common/parser" "github.com/onflow/flow-go/model/flow" ) @@ -123,7 +123,7 @@ func (g *GetBlockByIDs) Build(r *common.Request) error { } func (g *GetBlockByIDs) Parse(rawIds []string) error { - var ids convert.IDs + var ids parser.IDs err := ids.Parse(rawIds) if err != nil { return err diff --git a/engine/access/rest/http/request/get_events.go b/engine/access/rest/http/request/get_events.go index f5aadf31369..c864cf24a47 100644 --- a/engine/access/rest/http/request/get_events.go +++ b/engine/access/rest/http/request/get_events.go @@ -4,7 +4,7 @@ import ( "fmt" "github.com/onflow/flow-go/engine/access/rest/common" - "github.com/onflow/flow-go/engine/access/rest/common/convert" + "github.com/onflow/flow-go/engine/access/rest/common/parser" "github.com/onflow/flow-go/model/flow" ) @@ -51,7 +51,7 @@ func (g *GetEvents) Parse(rawType string, rawStart string, rawEnd string, rawBlo } g.EndHeight = height.Flow() - var blockIDs convert.IDs + var blockIDs parser.IDs err = blockIDs.Parse(rawBlockIDs) if err != nil { return err diff --git a/engine/access/rest/http/request/get_execution_result.go b/engine/access/rest/http/request/get_execution_result.go index 8feb7aac51f..4947cd8f07f 100644 --- a/engine/access/rest/http/request/get_execution_result.go +++ b/engine/access/rest/http/request/get_execution_result.go @@ -4,7 +4,7 @@ import ( "fmt" "github.com/onflow/flow-go/engine/access/rest/common" - "github.com/onflow/flow-go/engine/access/rest/common/convert" + "github.com/onflow/flow-go/engine/access/rest/common/parser" "github.com/onflow/flow-go/model/flow" ) @@ -31,7 +31,7 @@ func (g *GetExecutionResultByBlockIDs) Build(r *common.Request) error { } func (g *GetExecutionResultByBlockIDs) Parse(rawIDs []string) error { - var ids convert.IDs + var ids parser.IDs err := ids.Parse(rawIDs) if err != nil { return err diff --git a/engine/access/rest/http/request/get_script.go b/engine/access/rest/http/request/get_script.go index 01fbf996601..a01a025465a 100644 --- a/engine/access/rest/http/request/get_script.go +++ b/engine/access/rest/http/request/get_script.go @@ -5,7 +5,7 @@ import ( "io" "github.com/onflow/flow-go/engine/access/rest/common" - "github.com/onflow/flow-go/engine/access/rest/common/convert" + "github.com/onflow/flow-go/engine/access/rest/common/parser" "github.com/onflow/flow-go/model/flow" ) @@ -43,7 +43,7 @@ func (g *GetScript) Parse(rawHeight string, rawID string, rawScript io.Reader) e } g.BlockHeight = height.Flow() - var id convert.ID + var id parser.ID err = id.Parse(rawID) if err != nil { return err diff --git a/engine/access/rest/http/request/get_transaction.go b/engine/access/rest/http/request/get_transaction.go index ba80fad0105..0d5df1e541e 100644 --- a/engine/access/rest/http/request/get_transaction.go +++ b/engine/access/rest/http/request/get_transaction.go @@ -2,7 +2,7 @@ package request import ( "github.com/onflow/flow-go/engine/access/rest/common" - "github.com/onflow/flow-go/engine/access/rest/common/convert" + "github.com/onflow/flow-go/engine/access/rest/common/parser" "github.com/onflow/flow-go/model/flow" ) @@ -16,14 +16,14 @@ type TransactionOptionals struct { } func (t *TransactionOptionals) Parse(r *common.Request) error { - var blockId convert.ID + var blockId parser.ID err := blockId.Parse(r.GetQueryParam(blockIDQueryParam)) if err != nil { return err } t.BlockID = blockId.Flow() - var collectionId convert.ID + var collectionId parser.ID err = collectionId.Parse(r.GetQueryParam(collectionIDQueryParam)) if err != nil { return err diff --git a/engine/access/rest/http/request/helpers.go b/engine/access/rest/http/request/helpers.go index 8ad33bb1d81..38a669d0ad1 100644 --- a/engine/access/rest/http/request/helpers.go +++ b/engine/access/rest/http/request/helpers.go @@ -8,7 +8,7 @@ import ( "strings" "github.com/onflow/flow-go/engine/access/rest/common" - "github.com/onflow/flow-go/engine/access/rest/common/convert" + "github.com/onflow/flow-go/engine/access/rest/common/parser" "github.com/onflow/flow-go/model/flow" ) @@ -61,7 +61,7 @@ func (g *GetByIDRequest) Build(r *common.Request) error { } func (g *GetByIDRequest) Parse(rawID string) error { - var id convert.ID + var id parser.ID err := id.Parse(rawID) if err != nil { return err diff --git a/engine/access/rest/http/request/transaction.go b/engine/access/rest/http/request/transaction.go index 1ebe595da81..a26f929ca8e 100644 --- a/engine/access/rest/http/request/transaction.go +++ b/engine/access/rest/http/request/transaction.go @@ -4,7 +4,7 @@ import ( "fmt" "io" - convert2 "github.com/onflow/flow-go/engine/access/rest/common/convert" + convert2 "github.com/onflow/flow-go/engine/access/rest/common/parser" "github.com/onflow/flow-go/engine/access/rest/http/models" "github.com/onflow/flow-go/engine/access/rest/util" "github.com/onflow/flow-go/engine/common/rpc/convert" diff --git a/engine/access/rest/http/routes/events.go b/engine/access/rest/http/routes/events.go index 038a4a98aeb..fed682555d0 100644 --- a/engine/access/rest/http/routes/events.go +++ b/engine/access/rest/http/routes/events.go @@ -7,7 +7,6 @@ import ( "github.com/onflow/flow-go/access" "github.com/onflow/flow-go/engine/access/rest/common" - "github.com/onflow/flow-go/engine/access/rest/http/models" "github.com/onflow/flow-go/engine/access/rest/http/request" ) diff --git a/engine/access/rest/websockets/data_providers/blocks_data_provider.go b/engine/access/rest/websockets/data_providers/blocks_data_provider.go deleted file mode 100644 index f5a24139a87..00000000000 --- a/engine/access/rest/websockets/data_providers/blocks_data_provider.go +++ /dev/null @@ -1,282 +0,0 @@ -package data_providers - -import ( - "context" - "fmt" - - "github.com/rs/zerolog" - - "github.com/onflow/flow-go/access" - "github.com/onflow/flow-go/engine/access/rest/common/convert" - "github.com/onflow/flow-go/engine/access/rest/util" - "github.com/onflow/flow-go/engine/access/rest/websockets/models" - "github.com/onflow/flow-go/model/flow" -) - -// BlocksFromStartBlockIDArgs contains the arguments required for subscribing to blocks -// starting from a specific block ID. -type BlocksFromStartBlockIDArgs struct { - StartBlockID flow.Identifier - BlockStatus flow.BlockStatus -} - -// BlocksFromStartBlockIDProvider is responsible for providing blocks starting -// from a specific block ID. -type BlocksFromStartBlockIDProvider struct { - *BaseDataProviderImpl - - logger zerolog.Logger - args BlocksFromStartBlockIDArgs -} - -var _ DataProvider = (*BlocksFromStartBlockIDProvider)(nil) - -// NewBlocksFromStartBlockIDProvider creates a new instance of BlocksFromStartBlockIDProvider. -func NewBlocksFromStartBlockIDProvider( - ctx context.Context, - logger zerolog.Logger, - api access.API, - topic string, - arguments map[string]string, - send chan<- interface{}, -) (*BlocksFromStartBlockIDProvider, error) { - ctx, cancel := context.WithCancel(ctx) - - p := &BlocksFromStartBlockIDProvider{ - logger: logger.With().Str("component", "block-from-start-block-id-provider").Logger(), - } - - // Validate arguments passed to the provider. - err := p.validateArguments(arguments) - if err != nil { - return nil, fmt.Errorf("invalid arguments: %w", err) - } - - // Subscribe to blocks from the start block ID with the specified block status. - subscription := p.api.SubscribeBlocksFromStartBlockID(ctx, p.args.StartBlockID, p.args.BlockStatus) - p.BaseDataProviderImpl = NewBaseDataProviderImpl( - ctx, - cancel, - api, - topic, - send, - subscription, - ) - - return p, nil -} - -// Run starts processing the subscription for blocks from the start block ID and handles responses. -// -// No errors are expected during normal operations. -func (p *BlocksFromStartBlockIDProvider) Run() error { - return HandleSubscription(p.ctx, p.subscription, handleResponse(p.send, p.args.BlockStatus)) -} - -// validateArguments checks and validates the arguments passed to the provider. -// -// No errors are expected during normal operations. -func (p *BlocksFromStartBlockIDProvider) validateArguments(arguments map[string]string) error { - // Check for block_status argument and validate - if blockStatusIn, ok := arguments["block_status"]; ok { - blockStatus, err := convert.ParseBlockStatus(blockStatusIn) - if err != nil { - return err - } - p.args.BlockStatus = blockStatus - } else { - return fmt.Errorf("'block_status' must be provided") - } - - // Check for start_block_id argument and validate - if startBlockIDIn, ok := arguments["start_block_id"]; ok { - var startBlockID convert.ID - err := startBlockID.Parse(startBlockIDIn) - if err != nil { - return err - } - p.args.StartBlockID = startBlockID.Flow() - } else { - return fmt.Errorf("'start_block_id' must be provided") - } - - return nil -} - -// BlocksFromBlockHeightArgs contains the arguments required for subscribing to blocks -// starting from a specific block height. -type BlocksFromBlockHeightArgs struct { - StartBlockHeight uint64 - BlockStatus flow.BlockStatus -} - -// BlocksFromStartBlockHeightProvider is responsible for providing blocks starting -// from a specific block height. -type BlocksFromStartBlockHeightProvider struct { - *BaseDataProviderImpl - - logger zerolog.Logger - args BlocksFromBlockHeightArgs -} - -var _ DataProvider = (*BlocksFromStartBlockHeightProvider)(nil) - -// NewBlocksFromStartBlockHeightProvider creates a new instance of BlocksFromStartBlockHeightProvider. -func NewBlocksFromStartBlockHeightProvider( - ctx context.Context, - logger zerolog.Logger, - api access.API, - topic string, - arguments map[string]string, - send chan<- interface{}, -) (*BlocksFromStartBlockHeightProvider, error) { - ctx, cancel := context.WithCancel(ctx) - - p := &BlocksFromStartBlockHeightProvider{ - logger: logger.With().Str("component", "block-from-start-block-height-provider").Logger(), - } - - // Validate arguments passed to the provider. - err := p.validateArguments(arguments) - if err != nil { - return nil, fmt.Errorf("invalid arguments: %w", err) - } - - // Subscribe to blocks from the start block height with the specified block status. - subscription := p.api.SubscribeBlocksFromStartHeight(ctx, p.args.StartBlockHeight, p.args.BlockStatus) - p.BaseDataProviderImpl = NewBaseDataProviderImpl( - ctx, - cancel, - api, - topic, - send, - subscription, - ) - - return p, nil -} - -// Run starts processing the subscription for blocks from the start block height and handles responses. -// -// No errors are expected during normal operations. -func (p *BlocksFromStartBlockHeightProvider) Run() error { - return HandleSubscription(p.ctx, p.subscription, handleResponse(p.send, p.args.BlockStatus)) -} - -// validateArguments checks and validates the arguments passed to the provider. -// -// No errors are expected during normal operations. -func (p *BlocksFromStartBlockHeightProvider) validateArguments(arguments map[string]string) error { - // Check for block_status argument and validate - if blockStatusIn, ok := arguments["block_status"]; ok { - blockStatus, err := convert.ParseBlockStatus(blockStatusIn) - if err != nil { - return err - } - p.args.BlockStatus = blockStatus - } else { - return fmt.Errorf("'block_status' must be provided") - } - - if startBlockHeightIn, ok := arguments["start_block_height"]; ok { - var err error - p.args.StartBlockHeight, err = util.ToUint64(startBlockHeightIn) - if err != nil { - return fmt.Errorf("invalid start height: %w", err) - } - } else { - return fmt.Errorf("'start_block_height' must be provided") - } - - return nil -} - -// BlocksFromLatestArgs contains the arguments required for subscribing to blocks -// starting from latest block. -type BlocksFromLatestArgs struct { - BlockStatus flow.BlockStatus -} - -// BlocksFromLatestProvider is responsible for providing blocks starting from latest block. -type BlocksFromLatestProvider struct { - *BaseDataProviderImpl - - logger zerolog.Logger - args BlocksFromLatestArgs -} - -var _ DataProvider = (*BlocksFromLatestProvider)(nil) - -// NewBlocksFromLatestProvider creates a new BlocksFromLatestProvider. -func NewBlocksFromLatestProvider( - ctx context.Context, - logger zerolog.Logger, - api access.API, - topic string, - arguments map[string]string, - send chan<- interface{}, -) (*BlocksFromLatestProvider, error) { - ctx, cancel := context.WithCancel(ctx) - - p := &BlocksFromLatestProvider{ - logger: logger.With().Str("component", "block-from-latest-provider").Logger(), - } - - // Validate arguments passed to the provider. - err := p.validateArguments(arguments) - if err != nil { - return nil, fmt.Errorf("invalid arguments: %w", err) - } - - // Subscribe to blocks from the latest block height with the specified block status. - subscription := p.api.SubscribeBlocksFromLatest(ctx, p.args.BlockStatus) - p.BaseDataProviderImpl = NewBaseDataProviderImpl( - ctx, - cancel, - api, - topic, - send, - subscription, - ) - - return p, nil -} - -// Run starts processing the subscription for blocks from the latest block and handles responses. -// -// No errors are expected during normal operations. -func (p *BlocksFromLatestProvider) Run() error { - return HandleSubscription(p.ctx, p.subscription, handleResponse(p.send, p.args.BlockStatus)) -} - -// validateArguments checks and validates the arguments passed to the provider. -// -// No errors are expected during normal operations. -func (p *BlocksFromLatestProvider) validateArguments(arguments map[string]string) error { - // Check for block_status argument and validate - if blockStatusIn, ok := arguments["block_status"]; ok { - blockStatus, err := convert.ParseBlockStatus(blockStatusIn) - if err != nil { - return err - } - p.args.BlockStatus = blockStatus - } else { - return fmt.Errorf("'block_status' must be provided") - } - - return nil -} - -// handleResponse processes a block and sends the formatted response. -// -// No errors are expected during normal operations. -func handleResponse(send chan<- interface{}, blockStatus flow.BlockStatus) func(*flow.Block) error { - return func(block *flow.Block) error { - send <- &models.BlockMessageResponse{ - Block: block, - BlockStatus: blockStatus, - } - - return nil - } -} diff --git a/engine/access/rest/websockets/data_providers/blocks_data_provider_test.go b/engine/access/rest/websockets/data_providers/blocks_data_provider_test.go deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go new file mode 100644 index 00000000000..679724c0346 --- /dev/null +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -0,0 +1,145 @@ +package data_providers + +import ( + "context" + "fmt" + + "github.com/rs/zerolog" + + "github.com/onflow/flow-go/access" + "github.com/onflow/flow-go/engine/access/rest/common/parser" + "github.com/onflow/flow-go/engine/access/rest/http/request" + "github.com/onflow/flow-go/engine/access/rest/util" + "github.com/onflow/flow-go/engine/access/rest/websockets/models" + "github.com/onflow/flow-go/engine/access/subscription" + "github.com/onflow/flow-go/model/flow" +) + +// BlocksArguments contains the arguments required for subscribing to blocks +type BlocksArguments struct { + StartBlockID flow.Identifier + StartBlockHeight uint64 + BlockStatus flow.BlockStatus +} + +// BlocksDataProvider is responsible for providing blocks +type BlocksDataProvider struct { + *BaseDataProviderImpl + + logger zerolog.Logger + args BlocksArguments + api access.API +} + +var _ DataProvider = (*BlocksDataProvider)(nil) + +// NewBlocksDataProvider creates a new instance of BlocksDataProvider. +func NewBlocksDataProvider( + ctx context.Context, + logger zerolog.Logger, + api access.API, + topic string, + arguments map[string]string, + send chan<- interface{}, +) (*BlocksDataProvider, error) { + ctx, cancel := context.WithCancel(ctx) + + p := &BlocksDataProvider{ + logger: logger.With().Str("component", "block-data-provider").Logger(), + api: api, + } + + // Validate arguments passed to the provider. + err := p.validateArguments(arguments) + if err != nil { + return nil, fmt.Errorf("invalid arguments: %w", err) + } + + // Subscribe to blocks from the start block ID with the specified block status. + subscription := p.createSubscription(ctx) + p.BaseDataProviderImpl = NewBaseDataProviderImpl( + ctx, + cancel, + topic, + send, + subscription, + ) + + return p, nil +} + +// Run starts processing the subscription for blocks and handles responses. +// +// No errors are expected during normal operations. +func (p *BlocksDataProvider) Run() error { + return HandleSubscription(p.ctx, p.subscription, handleResponse(p.send, p.args.BlockStatus)) +} + +// validateArguments checks and validates the arguments passed to the provider. +// +// No errors are expected during normal operations. +func (p *BlocksDataProvider) validateArguments(arguments map[string]string) error { + // Check for block_status argument and validate + if blockStatusIn, ok := arguments["block_status"]; ok { + blockStatus, err := parser.ParseBlockStatus(blockStatusIn) + if err != nil { + return err + } + p.args.BlockStatus = blockStatus + } else { + return fmt.Errorf("'block_status' must be provided") + } + + if startBlockIDIn, ok := arguments["start_block_id"]; ok { + var startBlockID parser.ID + err := startBlockID.Parse(startBlockIDIn) + if err != nil { + return err + } + p.args.StartBlockID = startBlockID.Flow() + } + + if startBlockHeightIn, ok := arguments["start_block_height"]; ok { + var err error + p.args.StartBlockHeight, err = util.ToUint64(startBlockHeightIn) + if err != nil { + return fmt.Errorf("invalid 'start_block_height': %w", err) + } + } else { + p.args.StartBlockHeight = request.EmptyHeight + } + + // if both start_block_id and start_height are provided + if p.args.StartBlockID != flow.ZeroID && p.args.StartBlockHeight != request.EmptyHeight { + return fmt.Errorf("can only provide either 'start_block_id' or 'start_block_height'") + } + + return nil +} + +// createSubscription +func (p *BlocksDataProvider) createSubscription(ctx context.Context) subscription.Subscription { + if p.args.StartBlockID != flow.ZeroID { + return p.api.SubscribeBlocksFromStartBlockID(ctx, p.args.StartBlockID, p.args.BlockStatus) + } + + if p.args.StartBlockHeight > 0 { + return p.api.SubscribeBlocksFromStartHeight(ctx, p.args.StartBlockHeight, p.args.BlockStatus) + } + + return p.api.SubscribeBlocksFromLatest(ctx, p.args.BlockStatus) +} + +// handleResponse processes a block and sends the formatted response. +// +// No errors are expected during normal operations. +func handleResponse(send chan<- interface{}, blockStatus flow.BlockStatus) func(*flow.Block) error { + return func(block *flow.Block) error { + send <- &models.BlockMessageResponse{ + Block: block, + BlockStatus: blockStatus, + } + + return nil + } +} diff --git a/engine/access/rest/websockets/data_providers/blocks_provider_test.go b/engine/access/rest/websockets/data_providers/blocks_provider_test.go new file mode 100644 index 00000000000..3b8a7fe2198 --- /dev/null +++ b/engine/access/rest/websockets/data_providers/blocks_provider_test.go @@ -0,0 +1,147 @@ +package data_providers + +import ( + "context" + "fmt" + "testing" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" + + accessmock "github.com/onflow/flow-go/access/mock" + "github.com/onflow/flow-go/engine/access/rest/common/parser" + mockstatestream "github.com/onflow/flow-go/engine/access/state_stream/mock" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" +) + +const unknownBlockStatus = "unknown_block_status" + +type testErrType struct { + name string + arguments map[string]string + expectedErrorMsg string +} + +// BlocksProviderSuite is a test suite for testing the block providers functionality. +type BlocksProviderSuite struct { + suite.Suite + + log zerolog.Logger + api *accessmock.API + + blockMap map[uint64]*flow.Block + rootBlock flow.Block + finalizedBlock *flow.Header +} + +func TestBlocksProviderSuite(t *testing.T) { + suite.Run(t, new(BlocksProviderSuite)) +} + +func (s *BlocksProviderSuite) SetupTest() { + s.log = unittest.Logger() + s.api = accessmock.NewAPI(s.T()) + + blockCount := 5 + s.blockMap = make(map[uint64]*flow.Block, blockCount) + s.rootBlock = unittest.BlockFixture() + s.rootBlock.Header.Height = 0 + parent := s.rootBlock.Header + + for i := 0; i < blockCount; i++ { + block := unittest.BlockWithParentFixture(parent) + // update for next iteration + parent = block.Header + s.blockMap[block.Header.Height] = block + } + s.finalizedBlock = parent +} + +// TestBlocksDataProvider_InvalidArguments verifies that +func (s *BlocksProviderSuite) TestBlocksDataProvider_InvalidArguments() { + ctx := context.Background() + send := make(chan interface{}) + + testCases := []testErrType{ + { + name: "missing 'block_status' argument", + arguments: map[string]string{ + "start_block_id": s.rootBlock.ID().String(), + }, + expectedErrorMsg: "'block_status' must be provided", + }, + { + name: "unknown 'block_status' argument", + arguments: map[string]string{ + "block_status": unknownBlockStatus, + }, + expectedErrorMsg: fmt.Sprintf("invalid 'block_status', must be '%s' or '%s'", parser.Finalized, parser.Sealed), + }, + { + name: "provide both 'start_block_id' and 'start_block_height' arguments", + arguments: map[string]string{ + "block_status": parser.Finalized, + "start_block_id": s.rootBlock.ID().String(), + "start_block_height": fmt.Sprintf("%d", s.rootBlock.Header.Height), + }, + expectedErrorMsg: "can only provide either 'start_block_id' or 'start_block_height'", + }, + } + + topic := BlocksTopic + + for _, test := range testCases { + s.Run(test.name, func() { + provider, err := NewBlocksDataProvider(ctx, s.log, s.api, topic, test.arguments, send) + s.Require().Nil(provider) + s.Require().Error(err) + s.Require().Contains(err.Error(), test.expectedErrorMsg) + }) + } +} + +// TestBlocksDataProvider_ValidArguments tests +func (s *BlocksProviderSuite) TestBlocksDataProvider_ValidArguments() { + ctx := context.Background() + + topic := BlocksTopic + send := make(chan interface{}) + + s.Run("subscribe blocks from start block id", func() { + subscription := mockstatestream.NewSubscription(s.T()) + startBlockId := s.rootBlock.Header.ID() + + s.api.On("SubscribeBlocksFromStartBlockID", mock.Anything, startBlockId, flow.BlockStatusFinalized).Return(subscription).Once() + + arguments := map[string]string{ + "start_block_id": startBlockId.String(), + "block_status": parser.Finalized, + } + + provider, err := NewBlocksDataProvider(ctx, s.log, s.api, topic, arguments, send) + s.Require().NoError(err) + s.Require().NotNil(provider) + s.Require().Equal(flow.BlockStatusFinalized, provider.args.BlockStatus) + + // Create a channel to receive mock Blocks objects + ch := make(chan interface{}) + var chReadOnly <-chan interface{} + // Simulate sending a mock Blocks + go func() { + for _, block := range s.blockMap { + // Send the mock Blocks through the channel + ch <- block + } + }() + + chReadOnly = ch + subscription.Mock.On("Channel").Return(chReadOnly) + + err = provider.Run() + s.Require().NoError(err) + + provider.Close() + }) +} From 67a184a5ccd93a1b4eb001c7dc6606fe023b6096 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 12 Nov 2024 11:07:45 +0200 Subject: [PATCH 05/35] Updated last commit --- .../access/rest/http/request/transaction.go | 4 +- .../data_providers/base_provider.go | 33 ---------- .../data_providers/blocks_provider.go | 16 +++-- .../data_providers/blocks_provider_test.go | 46 ------------- .../rest/websockets/data_providers/factory.go | 12 +--- .../websockets/data_providers/factory_test.go | 66 +++++++++++++++++++ .../legacy/request/subscribe_events.go | 4 +- engine/access/rest_api_test.go | 14 ++-- 8 files changed, 89 insertions(+), 106 deletions(-) diff --git a/engine/access/rest/http/request/transaction.go b/engine/access/rest/http/request/transaction.go index a26f929ca8e..68bad0009f2 100644 --- a/engine/access/rest/http/request/transaction.go +++ b/engine/access/rest/http/request/transaction.go @@ -4,7 +4,7 @@ import ( "fmt" "io" - convert2 "github.com/onflow/flow-go/engine/access/rest/common/parser" + "github.com/onflow/flow-go/engine/access/rest/common/parser" "github.com/onflow/flow-go/engine/access/rest/http/models" "github.com/onflow/flow-go/engine/access/rest/util" "github.com/onflow/flow-go/engine/common/rpc/convert" @@ -90,7 +90,7 @@ func (t *Transaction) Parse(raw io.Reader, chain flow.Chain) error { return fmt.Errorf("invalid transaction script encoding") } - var blockID convert2.ID + var blockID parser.ID err = blockID.Parse(tx.ReferenceBlockId) if err != nil { return fmt.Errorf("invalid reference block ID: %w", err) diff --git a/engine/access/rest/websockets/data_providers/base_provider.go b/engine/access/rest/websockets/data_providers/base_provider.go index a30cf9a6887..567b7647eff 100644 --- a/engine/access/rest/websockets/data_providers/base_provider.go +++ b/engine/access/rest/websockets/data_providers/base_provider.go @@ -2,9 +2,7 @@ package data_providers import ( "context" - "fmt" - "github.com/onflow/flow-go/access" "github.com/onflow/flow-go/engine/access/subscription" ) @@ -29,7 +27,6 @@ type BaseDataProviderImpl struct { cancel context.CancelFunc ctx context.Context - api access.API send chan<- interface{} subscription subscription.Subscription } @@ -38,7 +35,6 @@ type BaseDataProviderImpl struct { func NewBaseDataProviderImpl( ctx context.Context, cancel context.CancelFunc, - api access.API, topic string, send chan<- interface{}, subscription subscription.Subscription, @@ -49,7 +45,6 @@ func NewBaseDataProviderImpl( ctx: ctx, cancel: cancel, - api: api, send: send, subscription: subscription, } @@ -69,31 +64,3 @@ func (b *BaseDataProviderImpl) Topic() string { func (b *BaseDataProviderImpl) Close() { b.cancel() } - -// TODO: refactor rpc version of HandleSubscription and use it -func HandleSubscription[T any](ctx context.Context, sub subscription.Subscription, handleResponse func(resp T) error) error { - for { - select { - case v, ok := <-sub.Channel(): - if !ok { - if sub.Err() != nil { - return fmt.Errorf("stream encountered an error: %w", sub.Err()) - } - return nil - } - - resp, ok := v.(T) - if !ok { - return fmt.Errorf("unexpected subscription response type: %T", v) - } - - err := handleResponse(resp) - if err != nil { - return err - } - case <-ctx.Done(): - // context closed, subscription closed - return nil - } - } -} diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index 679724c0346..2aba13337c2 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -17,9 +17,9 @@ import ( // BlocksArguments contains the arguments required for subscribing to blocks type BlocksArguments struct { - StartBlockID flow.Identifier - StartBlockHeight uint64 - BlockStatus flow.BlockStatus + StartBlockID flow.Identifier // ID of the block to start subscription from + StartBlockHeight uint64 // Height of the block to start subscription from + BlockStatus flow.BlockStatus // Status of blocks to subscribe to } // BlocksDataProvider is responsible for providing blocks @@ -55,7 +55,7 @@ func NewBlocksDataProvider( return nil, fmt.Errorf("invalid arguments: %w", err) } - // Subscribe to blocks from the start block ID with the specified block status. + // Set up a subscription to blocks based on the provided start block ID and block status. subscription := p.createSubscription(ctx) p.BaseDataProviderImpl = NewBaseDataProviderImpl( ctx, @@ -79,7 +79,7 @@ func (p *BlocksDataProvider) Run() error { // // No errors are expected during normal operations. func (p *BlocksDataProvider) validateArguments(arguments map[string]string) error { - // Check for block_status argument and validate + // Parse 'block_status' if blockStatusIn, ok := arguments["block_status"]; ok { blockStatus, err := parser.ParseBlockStatus(blockStatusIn) if err != nil { @@ -90,6 +90,7 @@ func (p *BlocksDataProvider) validateArguments(arguments map[string]string) erro return fmt.Errorf("'block_status' must be provided") } + // Parse 'start_block_id' if provided if startBlockIDIn, ok := arguments["start_block_id"]; ok { var startBlockID parser.ID err := startBlockID.Parse(startBlockIDIn) @@ -99,6 +100,7 @@ func (p *BlocksDataProvider) validateArguments(arguments map[string]string) erro p.args.StartBlockID = startBlockID.Flow() } + // Parse 'start_block_height' if provided if startBlockHeightIn, ok := arguments["start_block_height"]; ok { var err error p.args.StartBlockHeight, err = util.ToUint64(startBlockHeightIn) @@ -117,13 +119,13 @@ func (p *BlocksDataProvider) validateArguments(arguments map[string]string) erro return nil } -// createSubscription +// createSubscription creates a new subscription based on the specified start block ID or height. func (p *BlocksDataProvider) createSubscription(ctx context.Context) subscription.Subscription { if p.args.StartBlockID != flow.ZeroID { return p.api.SubscribeBlocksFromStartBlockID(ctx, p.args.StartBlockID, p.args.BlockStatus) } - if p.args.StartBlockHeight > 0 { + if p.args.StartBlockHeight != request.EmptyHeight { return p.api.SubscribeBlocksFromStartHeight(ctx, p.args.StartBlockHeight, p.args.BlockStatus) } diff --git a/engine/access/rest/websockets/data_providers/blocks_provider_test.go b/engine/access/rest/websockets/data_providers/blocks_provider_test.go index 3b8a7fe2198..db460d4f879 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider_test.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider_test.go @@ -6,12 +6,10 @@ import ( "testing" "github.com/rs/zerolog" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" accessmock "github.com/onflow/flow-go/access/mock" "github.com/onflow/flow-go/engine/access/rest/common/parser" - mockstatestream "github.com/onflow/flow-go/engine/access/state_stream/mock" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/utils/unittest" ) @@ -101,47 +99,3 @@ func (s *BlocksProviderSuite) TestBlocksDataProvider_InvalidArguments() { }) } } - -// TestBlocksDataProvider_ValidArguments tests -func (s *BlocksProviderSuite) TestBlocksDataProvider_ValidArguments() { - ctx := context.Background() - - topic := BlocksTopic - send := make(chan interface{}) - - s.Run("subscribe blocks from start block id", func() { - subscription := mockstatestream.NewSubscription(s.T()) - startBlockId := s.rootBlock.Header.ID() - - s.api.On("SubscribeBlocksFromStartBlockID", mock.Anything, startBlockId, flow.BlockStatusFinalized).Return(subscription).Once() - - arguments := map[string]string{ - "start_block_id": startBlockId.String(), - "block_status": parser.Finalized, - } - - provider, err := NewBlocksDataProvider(ctx, s.log, s.api, topic, arguments, send) - s.Require().NoError(err) - s.Require().NotNil(provider) - s.Require().Equal(flow.BlockStatusFinalized, provider.args.BlockStatus) - - // Create a channel to receive mock Blocks objects - ch := make(chan interface{}) - var chReadOnly <-chan interface{} - // Simulate sending a mock Blocks - go func() { - for _, block := range s.blockMap { - // Send the mock Blocks through the channel - ch <- block - } - }() - - chReadOnly = ch - subscription.Mock.On("Channel").Return(chReadOnly) - - err = provider.Run() - s.Require().NoError(err) - - provider.Close() - }) -} diff --git a/engine/access/rest/websockets/data_providers/factory.go b/engine/access/rest/websockets/data_providers/factory.go index d8285743d5e..7061215ac20 100644 --- a/engine/access/rest/websockets/data_providers/factory.go +++ b/engine/access/rest/websockets/data_providers/factory.go @@ -19,9 +19,7 @@ const ( BlockDigestsTopic = "block_digests" TransactionStatusesTopic = "transaction_statuses" - BlocksFromStartBlockIDTopic = "blocks_from_start_block_id" - BlocksFromStartBlockHeightTopic = "blocks_from_start_block_height" - BlocksFromLatestTopic = "blocks_from_latest" + BlocksTopic = "blocks" ) // DataProviderFactory is responsible for creating data providers based on the @@ -73,12 +71,8 @@ func (s *DataProviderFactory) NewDataProvider( ch chan<- interface{}, ) (DataProvider, error) { switch topic { - case BlocksFromStartBlockIDTopic: - return NewBlocksFromStartBlockIDProvider(ctx, s.logger, s.accessApi, topic, arguments, ch) - case BlocksFromStartBlockHeightTopic: - return NewBlocksFromStartBlockHeightProvider(ctx, s.logger, s.accessApi, topic, arguments, ch) - case BlocksFromLatestTopic: - return NewBlocksFromLatestProvider(ctx, s.logger, s.accessApi, topic, arguments, ch) + case BlocksTopic: + return NewBlocksDataProvider(ctx, s.logger, s.accessApi, topic, arguments, ch) // TODO: Implemented handlers for each topic should be added in respective case case EventsTopic, AccountStatusesTopic, diff --git a/engine/access/rest/websockets/data_providers/factory_test.go b/engine/access/rest/websockets/data_providers/factory_test.go index e69de29bb2d..45b8aebbf67 100644 --- a/engine/access/rest/websockets/data_providers/factory_test.go +++ b/engine/access/rest/websockets/data_providers/factory_test.go @@ -0,0 +1,66 @@ +package data_providers + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/suite" + + accessmock "github.com/onflow/flow-go/access/mock" + "github.com/onflow/flow-go/engine/access/state_stream" + statestreammock "github.com/onflow/flow-go/engine/access/state_stream/mock" + "github.com/onflow/flow-go/utils/unittest" +) + +// DataProviderFactorySuite is a test suite for testing the DataProviderFactory functionality. +type DataProviderFactorySuite struct { + suite.Suite + + ctx context.Context + ch chan interface{} + + factory *DataProviderFactory +} + +func TestDataProviderFactorySuite(t *testing.T) { + suite.Run(t, new(DataProviderFactorySuite)) +} + +// SetupTest sets up the initial context and dependencies for each test case. +// It initializes the factory with mock instances and validates that it is created successfully. +func (s *DataProviderFactorySuite) SetupTest() { + log := unittest.Logger() + eventFilterConfig := state_stream.EventFilterConfig{} + stateStreamApi := statestreammock.NewAPI(s.T()) + accessApi := accessmock.NewAPI(s.T()) + + s.ctx = context.Background() + s.ch = make(chan interface{}) + + s.factory = NewDataProviderFactory(log, eventFilterConfig, stateStreamApi, accessApi) + s.Require().NotNil(s.factory) +} + +// TestSupportedTopics verifies that supported topics return a valid provider and no errors. +// Each test case includes a topic and arguments for which a data provider should be created. +func (s *DataProviderFactorySuite) TestSupportedTopics() { + +} + +// TestUnsupportedTopics verifies that unsupported topics do not return a provider +// and instead return an error indicating the topic is unsupported. +func (s *DataProviderFactorySuite) TestUnsupportedTopics() { + // Define unsupported topics + unsupportedTopics := []string{ + "unknown_topic", + "", + } + + for _, topic := range unsupportedTopics { + provider, err := s.factory.NewDataProvider(s.ctx, topic, nil, s.ch) + s.Require().Nil(provider, "Expected no provider for unsupported topic %s", topic) + s.Require().Error(err, "Expected error for unsupported topic %s", topic) + s.Require().EqualError(err, fmt.Sprintf("unsupported topic \"%s\"", topic)) + } +} diff --git a/engine/access/rest/websockets/legacy/request/subscribe_events.go b/engine/access/rest/websockets/legacy/request/subscribe_events.go index 7f02ad1c10e..1110d3582d4 100644 --- a/engine/access/rest/websockets/legacy/request/subscribe_events.go +++ b/engine/access/rest/websockets/legacy/request/subscribe_events.go @@ -5,7 +5,7 @@ import ( "strconv" "github.com/onflow/flow-go/engine/access/rest/common" - "github.com/onflow/flow-go/engine/access/rest/common/convert" + "github.com/onflow/flow-go/engine/access/rest/common/parser" "github.com/onflow/flow-go/engine/access/rest/http/request" "github.com/onflow/flow-go/model/flow" ) @@ -57,7 +57,7 @@ func (g *SubscribeEvents) Parse( rawContracts []string, rawHeartbeatInterval string, ) error { - var startBlockID convert.ID + var startBlockID parser.ID err := startBlockID.Parse(rawStartBlockID) if err != nil { return err diff --git a/engine/access/rest_api_test.go b/engine/access/rest_api_test.go index 9c8eaa20881..c1f6eeb0c21 100644 --- a/engine/access/rest_api_test.go +++ b/engine/access/rest_api_test.go @@ -22,7 +22,7 @@ import ( accessmock "github.com/onflow/flow-go/engine/access/mock" "github.com/onflow/flow-go/engine/access/rest" "github.com/onflow/flow-go/engine/access/rest/common" - "github.com/onflow/flow-go/engine/access/rest/common/convert" + "github.com/onflow/flow-go/engine/access/rest/common/parser" "github.com/onflow/flow-go/engine/access/rest/router" "github.com/onflow/flow-go/engine/access/rpc" "github.com/onflow/flow-go/engine/access/rpc/backend" @@ -230,8 +230,8 @@ func TestRestAPI(t *testing.T) { func (suite *RestAPITestSuite) TestGetBlock() { - testBlockIDs := make([]string, convert.MaxIDsLength) - testBlocks := make([]*flow.Block, convert.MaxIDsLength) + testBlockIDs := make([]string, parser.MaxIDsLength) + testBlocks := make([]*flow.Block, parser.MaxIDsLength) for i := range testBlockIDs { collections := unittest.CollectionListFixture(1) block := unittest.BlockWithGuaranteesFixture( @@ -281,7 +281,7 @@ func (suite *RestAPITestSuite) TestGetBlock() { actualBlocks, resp, err := client.BlocksApi.BlocksIdGet(ctx, blockIDSlice, optionsForBlockByID()) require.NoError(suite.T(), err) assert.Equal(suite.T(), http.StatusOK, resp.StatusCode) - assert.Len(suite.T(), actualBlocks, convert.MaxIDsLength) + assert.Len(suite.T(), actualBlocks, parser.MaxIDsLength) for i, b := range testBlocks { assert.Equal(suite.T(), b.ID().String(), actualBlocks[i].Header.Id) } @@ -379,13 +379,13 @@ func (suite *RestAPITestSuite) TestGetBlock() { ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) defer cancel() - blockIDs := make([]string, convert.MaxIDsLength+1) + blockIDs := make([]string, parser.MaxIDsLength+1) copy(blockIDs, testBlockIDs) - blockIDs[convert.MaxIDsLength] = unittest.IdentifierFixture().String() + blockIDs[parser.MaxIDsLength] = unittest.IdentifierFixture().String() blockIDSlice := []string{strings.Join(blockIDs, ",")} _, resp, err := client.BlocksApi.BlocksIdGet(ctx, blockIDSlice, optionsForBlockByID()) - assertError(suite.T(), resp, err, http.StatusBadRequest, fmt.Sprintf("at most %d IDs can be requested at a time", convert.MaxIDsLength)) + assertError(suite.T(), resp, err, http.StatusBadRequest, fmt.Sprintf("at most %d IDs can be requested at a time", parser.MaxIDsLength)) }) suite.Run("GetBlockByID with one non-existing block ID", func() { From e9c6ac8c149b7483414a10672929941cd5a4ca9c Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 12 Nov 2024 11:08:44 +0200 Subject: [PATCH 06/35] Redactored HandleSubscription to use it in the rest subscription data providers --- access/handler.go | 20 +++---- engine/access/state_stream/backend/handler.go | 22 +++---- engine/access/subscription/util.go | 57 +++++++++++++------ 3 files changed, 61 insertions(+), 38 deletions(-) diff --git a/access/handler.go b/access/handler.go index 25316e7f3dd..b974e7034fc 100644 --- a/access/handler.go +++ b/access/handler.go @@ -1066,7 +1066,7 @@ func (h *Handler) SubscribeBlocksFromStartBlockID(request *access.SubscribeBlock } sub := h.api.SubscribeBlocksFromStartBlockID(stream.Context(), startBlockID, blockStatus) - return subscription.HandleSubscription(sub, h.handleBlocksResponse(stream.Send, request.GetFullBlockResponse(), blockStatus)) + return subscription.HandleRPCSubscription(sub, h.handleBlocksResponse(stream.Send, request.GetFullBlockResponse(), blockStatus)) } // SubscribeBlocksFromStartHeight handles subscription requests for blocks started from block height. @@ -1093,7 +1093,7 @@ func (h *Handler) SubscribeBlocksFromStartHeight(request *access.SubscribeBlocks } sub := h.api.SubscribeBlocksFromStartHeight(stream.Context(), request.GetStartBlockHeight(), blockStatus) - return subscription.HandleSubscription(sub, h.handleBlocksResponse(stream.Send, request.GetFullBlockResponse(), blockStatus)) + return subscription.HandleRPCSubscription(sub, h.handleBlocksResponse(stream.Send, request.GetFullBlockResponse(), blockStatus)) } // SubscribeBlocksFromLatest handles subscription requests for blocks started from latest sealed block. @@ -1120,7 +1120,7 @@ func (h *Handler) SubscribeBlocksFromLatest(request *access.SubscribeBlocksFromL } sub := h.api.SubscribeBlocksFromLatest(stream.Context(), blockStatus) - return subscription.HandleSubscription(sub, h.handleBlocksResponse(stream.Send, request.GetFullBlockResponse(), blockStatus)) + return subscription.HandleRPCSubscription(sub, h.handleBlocksResponse(stream.Send, request.GetFullBlockResponse(), blockStatus)) } // handleBlocksResponse handles the subscription to block updates and sends @@ -1179,7 +1179,7 @@ func (h *Handler) SubscribeBlockHeadersFromStartBlockID(request *access.Subscrib } sub := h.api.SubscribeBlockHeadersFromStartBlockID(stream.Context(), startBlockID, blockStatus) - return subscription.HandleSubscription(sub, h.handleBlockHeadersResponse(stream.Send)) + return subscription.HandleRPCSubscription(sub, h.handleBlockHeadersResponse(stream.Send)) } // SubscribeBlockHeadersFromStartHeight handles subscription requests for block headers started from block height. @@ -1206,7 +1206,7 @@ func (h *Handler) SubscribeBlockHeadersFromStartHeight(request *access.Subscribe } sub := h.api.SubscribeBlockHeadersFromStartHeight(stream.Context(), request.GetStartBlockHeight(), blockStatus) - return subscription.HandleSubscription(sub, h.handleBlockHeadersResponse(stream.Send)) + return subscription.HandleRPCSubscription(sub, h.handleBlockHeadersResponse(stream.Send)) } // SubscribeBlockHeadersFromLatest handles subscription requests for block headers started from latest sealed block. @@ -1233,7 +1233,7 @@ func (h *Handler) SubscribeBlockHeadersFromLatest(request *access.SubscribeBlock } sub := h.api.SubscribeBlockHeadersFromLatest(stream.Context(), blockStatus) - return subscription.HandleSubscription(sub, h.handleBlockHeadersResponse(stream.Send)) + return subscription.HandleRPCSubscription(sub, h.handleBlockHeadersResponse(stream.Send)) } // handleBlockHeadersResponse handles the subscription to block updates and sends @@ -1293,7 +1293,7 @@ func (h *Handler) SubscribeBlockDigestsFromStartBlockID(request *access.Subscrib } sub := h.api.SubscribeBlockDigestsFromStartBlockID(stream.Context(), startBlockID, blockStatus) - return subscription.HandleSubscription(sub, h.handleBlockDigestsResponse(stream.Send)) + return subscription.HandleRPCSubscription(sub, h.handleBlockDigestsResponse(stream.Send)) } // SubscribeBlockDigestsFromStartHeight handles subscription requests for lightweight blocks started from block height. @@ -1320,7 +1320,7 @@ func (h *Handler) SubscribeBlockDigestsFromStartHeight(request *access.Subscribe } sub := h.api.SubscribeBlockDigestsFromStartHeight(stream.Context(), request.GetStartBlockHeight(), blockStatus) - return subscription.HandleSubscription(sub, h.handleBlockDigestsResponse(stream.Send)) + return subscription.HandleRPCSubscription(sub, h.handleBlockDigestsResponse(stream.Send)) } // SubscribeBlockDigestsFromLatest handles subscription requests for lightweight block started from latest sealed block. @@ -1347,7 +1347,7 @@ func (h *Handler) SubscribeBlockDigestsFromLatest(request *access.SubscribeBlock } sub := h.api.SubscribeBlockDigestsFromLatest(stream.Context(), blockStatus) - return subscription.HandleSubscription(sub, h.handleBlockDigestsResponse(stream.Send)) + return subscription.HandleRPCSubscription(sub, h.handleBlockDigestsResponse(stream.Send)) } // handleBlockDigestsResponse handles the subscription to block updates and sends @@ -1433,7 +1433,7 @@ func (h *Handler) SendAndSubscribeTransactionStatuses( sub := h.api.SubscribeTransactionStatuses(ctx, &tx, request.GetEventEncodingVersion()) messageIndex := counters.NewMonotonousCounter(0) - return subscription.HandleSubscription(sub, func(txResults []*TransactionResult) error { + return subscription.HandleRPCSubscription(sub, func(txResults []*TransactionResult) error { for i := range txResults { index := messageIndex.Value() if ok := messageIndex.Set(index + 1); !ok { diff --git a/engine/access/state_stream/backend/handler.go b/engine/access/state_stream/backend/handler.go index b2066440bb8..3acf1bad6ca 100644 --- a/engine/access/state_stream/backend/handler.go +++ b/engine/access/state_stream/backend/handler.go @@ -102,7 +102,7 @@ func (h *Handler) SubscribeExecutionData(request *executiondata.SubscribeExecuti sub := h.api.SubscribeExecutionData(stream.Context(), startBlockID, request.GetStartBlockHeight()) - return subscription.HandleSubscription(sub, handleSubscribeExecutionData(stream.Send, request.GetEventEncodingVersion())) + return subscription.HandleRPCSubscription(sub, handleSubscribeExecutionData(stream.Send, request.GetEventEncodingVersion())) } // SubscribeExecutionDataFromStartBlockID handles subscription requests for @@ -129,7 +129,7 @@ func (h *Handler) SubscribeExecutionDataFromStartBlockID(request *executiondata. sub := h.api.SubscribeExecutionDataFromStartBlockID(stream.Context(), startBlockID) - return subscription.HandleSubscription(sub, handleSubscribeExecutionData(stream.Send, request.GetEventEncodingVersion())) + return subscription.HandleRPCSubscription(sub, handleSubscribeExecutionData(stream.Send, request.GetEventEncodingVersion())) } // SubscribeExecutionDataFromStartBlockHeight handles subscription requests for @@ -150,7 +150,7 @@ func (h *Handler) SubscribeExecutionDataFromStartBlockHeight(request *executiond sub := h.api.SubscribeExecutionDataFromStartBlockHeight(stream.Context(), request.GetStartBlockHeight()) - return subscription.HandleSubscription(sub, handleSubscribeExecutionData(stream.Send, request.GetEventEncodingVersion())) + return subscription.HandleRPCSubscription(sub, handleSubscribeExecutionData(stream.Send, request.GetEventEncodingVersion())) } // SubscribeExecutionDataFromLatest handles subscription requests for @@ -171,7 +171,7 @@ func (h *Handler) SubscribeExecutionDataFromLatest(request *executiondata.Subscr sub := h.api.SubscribeExecutionDataFromLatest(stream.Context()) - return subscription.HandleSubscription(sub, handleSubscribeExecutionData(stream.Send, request.GetEventEncodingVersion())) + return subscription.HandleRPCSubscription(sub, handleSubscribeExecutionData(stream.Send, request.GetEventEncodingVersion())) } // SubscribeEvents is deprecated and will be removed in a future version. @@ -213,7 +213,7 @@ func (h *Handler) SubscribeEvents(request *executiondata.SubscribeEventsRequest, sub := h.api.SubscribeEvents(stream.Context(), startBlockID, request.GetStartBlockHeight(), filter) - return subscription.HandleSubscription(sub, h.handleEventsResponse(stream.Send, request.HeartbeatInterval, request.GetEventEncodingVersion())) + return subscription.HandleRPCSubscription(sub, h.handleEventsResponse(stream.Send, request.HeartbeatInterval, request.GetEventEncodingVersion())) } // SubscribeEventsFromStartBlockID handles subscription requests for events starting at the specified block ID. @@ -248,7 +248,7 @@ func (h *Handler) SubscribeEventsFromStartBlockID(request *executiondata.Subscri sub := h.api.SubscribeEventsFromStartBlockID(stream.Context(), startBlockID, filter) - return subscription.HandleSubscription(sub, h.handleEventsResponse(stream.Send, request.HeartbeatInterval, request.GetEventEncodingVersion())) + return subscription.HandleRPCSubscription(sub, h.handleEventsResponse(stream.Send, request.HeartbeatInterval, request.GetEventEncodingVersion())) } // SubscribeEventsFromStartHeight handles subscription requests for events starting at the specified block height. @@ -278,7 +278,7 @@ func (h *Handler) SubscribeEventsFromStartHeight(request *executiondata.Subscrib sub := h.api.SubscribeEventsFromStartHeight(stream.Context(), request.GetStartBlockHeight(), filter) - return subscription.HandleSubscription(sub, h.handleEventsResponse(stream.Send, request.HeartbeatInterval, request.GetEventEncodingVersion())) + return subscription.HandleRPCSubscription(sub, h.handleEventsResponse(stream.Send, request.HeartbeatInterval, request.GetEventEncodingVersion())) } // SubscribeEventsFromLatest handles subscription requests for events started from latest sealed block.. @@ -308,7 +308,7 @@ func (h *Handler) SubscribeEventsFromLatest(request *executiondata.SubscribeEven sub := h.api.SubscribeEventsFromLatest(stream.Context(), filter) - return subscription.HandleSubscription(sub, h.handleEventsResponse(stream.Send, request.HeartbeatInterval, request.GetEventEncodingVersion())) + return subscription.HandleRPCSubscription(sub, h.handleEventsResponse(stream.Send, request.HeartbeatInterval, request.GetEventEncodingVersion())) } // handleSubscribeExecutionData handles the subscription to execution data and sends it to the client via the provided stream. @@ -546,7 +546,7 @@ func (h *Handler) SubscribeAccountStatusesFromStartBlockID( sub := h.api.SubscribeAccountStatusesFromStartBlockID(stream.Context(), startBlockID, filter) - return subscription.HandleSubscription(sub, h.handleAccountStatusesResponse(request.HeartbeatInterval, request.GetEventEncodingVersion(), stream.Send)) + return subscription.HandleRPCSubscription(sub, h.handleAccountStatusesResponse(request.HeartbeatInterval, request.GetEventEncodingVersion(), stream.Send)) } // SubscribeAccountStatusesFromStartHeight streams account statuses for all blocks starting at the requested @@ -573,7 +573,7 @@ func (h *Handler) SubscribeAccountStatusesFromStartHeight( sub := h.api.SubscribeAccountStatusesFromStartHeight(stream.Context(), request.GetStartBlockHeight(), filter) - return subscription.HandleSubscription(sub, h.handleAccountStatusesResponse(request.HeartbeatInterval, request.GetEventEncodingVersion(), stream.Send)) + return subscription.HandleRPCSubscription(sub, h.handleAccountStatusesResponse(request.HeartbeatInterval, request.GetEventEncodingVersion(), stream.Send)) } // SubscribeAccountStatusesFromLatestBlock streams account statuses for all blocks starting @@ -600,5 +600,5 @@ func (h *Handler) SubscribeAccountStatusesFromLatestBlock( sub := h.api.SubscribeAccountStatusesFromLatestBlock(stream.Context(), filter) - return subscription.HandleSubscription(sub, h.handleAccountStatusesResponse(request.HeartbeatInterval, request.GetEventEncodingVersion(), stream.Send)) + return subscription.HandleRPCSubscription(sub, h.handleAccountStatusesResponse(request.HeartbeatInterval, request.GetEventEncodingVersion(), stream.Send)) } diff --git a/engine/access/subscription/util.go b/engine/access/subscription/util.go index 593f3d78499..604cbf6e597 100644 --- a/engine/access/subscription/util.go +++ b/engine/access/subscription/util.go @@ -1,8 +1,10 @@ package subscription import ( + "context" + "fmt" + "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" "github.com/onflow/flow-go/engine/common/rpc" ) @@ -11,29 +13,50 @@ import ( // handles the received responses, and sends the processed information to the client via the provided stream using handleResponse. // // Parameters: +// - ctx: Context for the operation. // - sub: The subscription. // - handleResponse: The function responsible for handling the response of the subscribed type. // -// Expected errors during normal operation: -// - codes.Internal: If the subscription encounters an error or gets an unexpected response. -func HandleSubscription[T any](sub Subscription, handleResponse func(resp T) error) error { +// No errors are expected during normal operations. +func HandleSubscription[T any](ctx context.Context, sub Subscription, handleResponse func(resp T) error) error { for { - v, ok := <-sub.Channel() - if !ok { - if sub.Err() != nil { - return rpc.ConvertError(sub.Err(), "stream encountered an error", codes.Internal) + select { + case v, ok := <-sub.Channel(): + if !ok { + if sub.Err() != nil { + return fmt.Errorf("stream encountered an error: %w", sub.Err()) + } + return nil } - return nil - } - resp, ok := v.(T) - if !ok { - return status.Errorf(codes.Internal, "unexpected response type: %T", v) - } + resp, ok := v.(T) + if !ok { + return fmt.Errorf("unexpected response type: %T", v) + } - err := handleResponse(resp) - if err != nil { - return err + err := handleResponse(resp) + if err != nil { + return err + } + case <-ctx.Done(): + return nil } } } + +// HandleRPCSubscription is a generic handler for subscriptions to a specific type for rpc calls. +// +// Parameters: +// - sub: The subscription. +// - handleResponse: The function responsible for handling the response of the subscribed type. +// +// Expected errors during normal operation: +// - codes.Internal: If the subscription encounters an error or gets an unexpected response. +func HandleRPCSubscription[T any](sub Subscription, handleResponse func(resp T) error) error { + err := HandleSubscription(nil, sub, handleResponse) + if err != nil { + return rpc.ConvertError(err, "handle subscription error", codes.Internal) + } + + return nil +} From 5fdece5c296824d7b8651303e8a285101e572516 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 12 Nov 2024 11:10:38 +0200 Subject: [PATCH 07/35] Added missed package name for block provider --- engine/access/rest/websockets/data_providers/blocks_provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index 2aba13337c2..1443becd44a 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -72,7 +72,7 @@ func NewBlocksDataProvider( // // No errors are expected during normal operations. func (p *BlocksDataProvider) Run() error { - return HandleSubscription(p.ctx, p.subscription, handleResponse(p.send, p.args.BlockStatus)) + return subscription.HandleSubscription(p.ctx, p.subscription, handleResponse(p.send, p.args.BlockStatus)) } // validateArguments checks and validates the arguments passed to the provider. From 7c50eabf8e2bccda01c5d66f9b72405832a3269c Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 12 Nov 2024 11:34:24 +0200 Subject: [PATCH 08/35] Added test for suppoted topics for data provoder factory --- .../websockets/data_providers/factory_test.go | 44 +++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/engine/access/rest/websockets/data_providers/factory_test.go b/engine/access/rest/websockets/data_providers/factory_test.go index 45b8aebbf67..cf258733d4f 100644 --- a/engine/access/rest/websockets/data_providers/factory_test.go +++ b/engine/access/rest/websockets/data_providers/factory_test.go @@ -5,11 +5,14 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" accessmock "github.com/onflow/flow-go/access/mock" + "github.com/onflow/flow-go/engine/access/rest/common/parser" "github.com/onflow/flow-go/engine/access/state_stream" statestreammock "github.com/onflow/flow-go/engine/access/state_stream/mock" + "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/utils/unittest" ) @@ -20,6 +23,9 @@ type DataProviderFactorySuite struct { ctx context.Context ch chan interface{} + accessApi *accessmock.API + stateStreamApi *statestreammock.API + factory *DataProviderFactory } @@ -32,20 +38,52 @@ func TestDataProviderFactorySuite(t *testing.T) { func (s *DataProviderFactorySuite) SetupTest() { log := unittest.Logger() eventFilterConfig := state_stream.EventFilterConfig{} - stateStreamApi := statestreammock.NewAPI(s.T()) - accessApi := accessmock.NewAPI(s.T()) + s.stateStreamApi = statestreammock.NewAPI(s.T()) + s.accessApi = accessmock.NewAPI(s.T()) s.ctx = context.Background() s.ch = make(chan interface{}) - s.factory = NewDataProviderFactory(log, eventFilterConfig, stateStreamApi, accessApi) + s.factory = NewDataProviderFactory(log, eventFilterConfig, s.stateStreamApi, s.accessApi) s.Require().NotNil(s.factory) } +// TODO: add others topic to check when they will be implemented // TestSupportedTopics verifies that supported topics return a valid provider and no errors. // Each test case includes a topic and arguments for which a data provider should be created. func (s *DataProviderFactorySuite) TestSupportedTopics() { + // Define supported topics and check if each returns the correct provider without errors + testCases := []struct { + name string + topic string + arguments map[string]string + mockSubscription func() + assertExpectations func() + }{ + { + name: "block topic", + topic: BlocksTopic, + arguments: map[string]string{"block_status": parser.Finalized}, + mockSubscription: func() { + s.accessApi.On("SubscribeBlocksFromLatest", mock.Anything, flow.BlockStatusFinalized).Return(nil).Once() + }, + assertExpectations: func() { + s.accessApi.AssertExpectations(s.T()) + }, + }, + } + + for _, test := range testCases { + s.Run(test.name, func() { + test.mockSubscription() + provider, err := s.factory.NewDataProvider(s.ctx, test.topic, test.arguments, s.ch) + s.Require().NotNil(provider, "Expected provider for topic %s", test.topic) + s.Require().NoError(err, "Expected no error for topic %s", test.topic) + + test.assertExpectations() + }) + } } // TestUnsupportedTopics verifies that unsupported topics do not return a provider From 5390d1a0c25e45c8c743d326c30bd760423cae16 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 12 Nov 2024 13:01:21 +0200 Subject: [PATCH 09/35] Added godoc to test --- .../websockets/data_providers/blocks_provider_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/engine/access/rest/websockets/data_providers/blocks_provider_test.go b/engine/access/rest/websockets/data_providers/blocks_provider_test.go index db460d4f879..7470a2e7bfe 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider_test.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider_test.go @@ -57,7 +57,13 @@ func (s *BlocksProviderSuite) SetupTest() { s.finalizedBlock = parent } -// TestBlocksDataProvider_InvalidArguments verifies that +// TestBlocksDataProvider_InvalidArguments tests the behavior of the block data provider +// when invalid arguments are provided. It verifies that appropriate errors are returned +// for missing or conflicting arguments. +// This test covers the test cases: +// 1. Missing 'block_status' argument. +// 2. Invalid 'block_status' argument. +// 3. Providing both 'start_block_id' and 'start_block_height' simultaneously. func (s *BlocksProviderSuite) TestBlocksDataProvider_InvalidArguments() { ctx := context.Background() send := make(chan interface{}) From 1d63e2d4e137cd0ce62ce118a41d4067332461f8 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 12 Nov 2024 13:43:32 +0200 Subject: [PATCH 10/35] Updated tests to check topic and subscription id, updated godoc --- .../websockets/data_providers/blocks_provider.go | 6 +++--- .../data_providers/blocks_provider_test.go | 2 ++ .../websockets/data_providers/data_provider.go | 2 +- .../websockets/data_providers/factory_test.go | 16 ++++++++++++---- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index 1443becd44a..f3acc76b9d4 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -42,8 +42,6 @@ func NewBlocksDataProvider( arguments map[string]string, send chan<- interface{}, ) (*BlocksDataProvider, error) { - ctx, cancel := context.WithCancel(ctx) - p := &BlocksDataProvider{ logger: logger.With().Str("component", "block-data-provider").Logger(), api: api, @@ -55,10 +53,12 @@ func NewBlocksDataProvider( return nil, fmt.Errorf("invalid arguments: %w", err) } + context, cancel := context.WithCancel(ctx) + // Set up a subscription to blocks based on the provided start block ID and block status. subscription := p.createSubscription(ctx) p.BaseDataProviderImpl = NewBaseDataProviderImpl( - ctx, + context, cancel, topic, send, diff --git a/engine/access/rest/websockets/data_providers/blocks_provider_test.go b/engine/access/rest/websockets/data_providers/blocks_provider_test.go index 7470a2e7bfe..4a619d7bfcf 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider_test.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider_test.go @@ -105,3 +105,5 @@ func (s *BlocksProviderSuite) TestBlocksDataProvider_InvalidArguments() { }) } } + +// TODO: add tests for responses after the WebsocketController is ready diff --git a/engine/access/rest/websockets/data_providers/data_provider.go b/engine/access/rest/websockets/data_providers/data_provider.go index 11a8bc16c38..6a3fcc8991b 100644 --- a/engine/access/rest/websockets/data_providers/data_provider.go +++ b/engine/access/rest/websockets/data_providers/data_provider.go @@ -1,6 +1,6 @@ package data_providers -// The DataProvider is the interface abstracts of the actual subscriptions used by the WebSocketCollector. +// The DataProvider is the interface abstracts of the actual data provider used by the WebSocketCollector. type DataProvider interface { BaseDataProvider diff --git a/engine/access/rest/websockets/data_providers/factory_test.go b/engine/access/rest/websockets/data_providers/factory_test.go index cf258733d4f..2cbbb1276df 100644 --- a/engine/access/rest/websockets/data_providers/factory_test.go +++ b/engine/access/rest/websockets/data_providers/factory_test.go @@ -57,15 +57,20 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { name string topic string arguments map[string]string - mockSubscription func() + mockSubscription func() string // return subscription id assertExpectations func() }{ { name: "block topic", topic: BlocksTopic, arguments: map[string]string{"block_status": parser.Finalized}, - mockSubscription: func() { - s.accessApi.On("SubscribeBlocksFromLatest", mock.Anything, flow.BlockStatusFinalized).Return(nil).Once() + mockSubscription: func() string { + subscription := statestreammock.NewSubscription(s.T()) + subscriptionID := unittest.IdentifierFixture().String() + subscription.On("ID").Return(subscriptionID).Once() + + s.accessApi.On("SubscribeBlocksFromLatest", mock.Anything, flow.BlockStatusFinalized).Return(subscription).Once() + return subscriptionID }, assertExpectations: func() { s.accessApi.AssertExpectations(s.T()) @@ -75,12 +80,15 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { for _, test := range testCases { s.Run(test.name, func() { - test.mockSubscription() + subscriptionID := test.mockSubscription() provider, err := s.factory.NewDataProvider(s.ctx, test.topic, test.arguments, s.ch) s.Require().NotNil(provider, "Expected provider for topic %s", test.topic) s.Require().NoError(err, "Expected no error for topic %s", test.topic) + s.Require().Equal(test.topic, provider.Topic()) + s.Require().Equal(subscriptionID, provider.ID()) + test.assertExpectations() }) } From c33dfa29fbaecf7ee261feb0a48b69680393b433 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 12 Nov 2024 13:51:29 +0200 Subject: [PATCH 11/35] Linted --- engine/access/subscription/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/access/subscription/util.go b/engine/access/subscription/util.go index 604cbf6e597..9f5b5b1bba9 100644 --- a/engine/access/subscription/util.go +++ b/engine/access/subscription/util.go @@ -53,7 +53,7 @@ func HandleSubscription[T any](ctx context.Context, sub Subscription, handleResp // Expected errors during normal operation: // - codes.Internal: If the subscription encounters an error or gets an unexpected response. func HandleRPCSubscription[T any](sub Subscription, handleResponse func(resp T) error) error { - err := HandleSubscription(nil, sub, handleResponse) + err := HandleSubscription(context.TODO(), sub, handleResponse) if err != nil { return rpc.ConvertError(err, "handle subscription error", codes.Internal) } From 7d60c33dfd19681c64f08ee9be5eac7832cec564 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 12 Nov 2024 17:17:52 +0200 Subject: [PATCH 12/35] Updated context for creating subscription for block data provider --- engine/access/rest/websockets/data_providers/blocks_provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index f3acc76b9d4..a1833e171bf 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -56,7 +56,7 @@ func NewBlocksDataProvider( context, cancel := context.WithCancel(ctx) // Set up a subscription to blocks based on the provided start block ID and block status. - subscription := p.createSubscription(ctx) + subscription := p.createSubscription(context) p.BaseDataProviderImpl = NewBaseDataProviderImpl( context, cancel, From 22ad4699dc75b52c6cd2e1682d0efdcabcd9fa0c Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Wed, 13 Nov 2024 11:37:14 +0200 Subject: [PATCH 13/35] Updated BlockMessageResponse, added dogoc for constant values, removed ctx from HandleSubscription --- .../access/rest/common/parser/block_status.go | 2 ++ .../data_providers/base_provider.go | 15 +++----- .../data_providers/blocks_provider.go | 6 ++-- .../rest/websockets/models/block_models.go | 3 +- engine/access/subscription/util.go | 36 ++++++++----------- 5 files changed, 24 insertions(+), 38 deletions(-) diff --git a/engine/access/rest/common/parser/block_status.go b/engine/access/rest/common/parser/block_status.go index a1b6e8a7b46..efb34519894 100644 --- a/engine/access/rest/common/parser/block_status.go +++ b/engine/access/rest/common/parser/block_status.go @@ -6,6 +6,8 @@ import ( "github.com/onflow/flow-go/model/flow" ) +// Finalized and Sealed represents the status of a block. +// It is used in rest arguments to provide block status. const ( Finalized = "finalized" Sealed = "sealed" diff --git a/engine/access/rest/websockets/data_providers/base_provider.go b/engine/access/rest/websockets/data_providers/base_provider.go index 567b7647eff..86bbf9f2c0e 100644 --- a/engine/access/rest/websockets/data_providers/base_provider.go +++ b/engine/access/rest/websockets/data_providers/base_provider.go @@ -22,29 +22,22 @@ var _ BaseDataProvider = (*BaseDataProviderImpl)(nil) // BaseDataProviderImpl is the concrete implementation of the BaseDataProvider interface. // It holds common objects for the provider. type BaseDataProviderImpl struct { - topic string - - cancel context.CancelFunc - ctx context.Context - + topic string + cancel context.CancelFunc send chan<- interface{} subscription subscription.Subscription } // NewBaseDataProviderImpl creates a new instance of BaseDataProviderImpl. func NewBaseDataProviderImpl( - ctx context.Context, cancel context.CancelFunc, topic string, send chan<- interface{}, subscription subscription.Subscription, ) *BaseDataProviderImpl { return &BaseDataProviderImpl{ - topic: topic, - - ctx: ctx, - cancel: cancel, - + topic: topic, + cancel: cancel, send: send, subscription: subscription, } diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index a1833e171bf..62da3a3cbd9 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -58,7 +58,6 @@ func NewBlocksDataProvider( // Set up a subscription to blocks based on the provided start block ID and block status. subscription := p.createSubscription(context) p.BaseDataProviderImpl = NewBaseDataProviderImpl( - context, cancel, topic, send, @@ -72,7 +71,7 @@ func NewBlocksDataProvider( // // No errors are expected during normal operations. func (p *BlocksDataProvider) Run() error { - return subscription.HandleSubscription(p.ctx, p.subscription, handleResponse(p.send, p.args.BlockStatus)) + return subscription.HandleSubscription(p.subscription, handleResponse(p.send, p.args.BlockStatus)) } // validateArguments checks and validates the arguments passed to the provider. @@ -138,8 +137,7 @@ func (p *BlocksDataProvider) createSubscription(ctx context.Context) subscriptio func handleResponse(send chan<- interface{}, blockStatus flow.BlockStatus) func(*flow.Block) error { return func(block *flow.Block) error { send <- &models.BlockMessageResponse{ - Block: block, - BlockStatus: blockStatus, + Block: block, } return nil diff --git a/engine/access/rest/websockets/models/block_models.go b/engine/access/rest/websockets/models/block_models.go index f363038808b..2e3e378d832 100644 --- a/engine/access/rest/websockets/models/block_models.go +++ b/engine/access/rest/websockets/models/block_models.go @@ -5,6 +5,5 @@ import ( ) type BlockMessageResponse struct { - Block *flow.Block `json:"block"` - BlockStatus flow.BlockStatus `json:"block_status"` + Block *flow.Block `json:"block"` } diff --git a/engine/access/subscription/util.go b/engine/access/subscription/util.go index 9f5b5b1bba9..2dadac441cf 100644 --- a/engine/access/subscription/util.go +++ b/engine/access/subscription/util.go @@ -1,7 +1,6 @@ package subscription import ( - "context" "fmt" "google.golang.org/grpc/codes" @@ -13,33 +12,28 @@ import ( // handles the received responses, and sends the processed information to the client via the provided stream using handleResponse. // // Parameters: -// - ctx: Context for the operation. // - sub: The subscription. // - handleResponse: The function responsible for handling the response of the subscribed type. // // No errors are expected during normal operations. -func HandleSubscription[T any](ctx context.Context, sub Subscription, handleResponse func(resp T) error) error { +func HandleSubscription[T any](sub Subscription, handleResponse func(resp T) error) error { for { - select { - case v, ok := <-sub.Channel(): - if !ok { - if sub.Err() != nil { - return fmt.Errorf("stream encountered an error: %w", sub.Err()) - } - return nil + v, ok := <-sub.Channel() + if !ok { + if sub.Err() != nil { + return fmt.Errorf("stream encountered an error: %w", sub.Err()) } + return nil + } - resp, ok := v.(T) - if !ok { - return fmt.Errorf("unexpected response type: %T", v) - } + resp, ok := v.(T) + if !ok { + return fmt.Errorf("unexpected response type: %T", v) + } - err := handleResponse(resp) - if err != nil { - return err - } - case <-ctx.Done(): - return nil + err := handleResponse(resp) + if err != nil { + return err } } } @@ -53,7 +47,7 @@ func HandleSubscription[T any](ctx context.Context, sub Subscription, handleResp // Expected errors during normal operation: // - codes.Internal: If the subscription encounters an error or gets an unexpected response. func HandleRPCSubscription[T any](sub Subscription, handleResponse func(resp T) error) error { - err := HandleSubscription(context.TODO(), sub, handleResponse) + err := HandleSubscription(sub, handleResponse) if err != nil { return rpc.ConvertError(err, "handle subscription error", codes.Internal) } From a9fe159f4c8a3ce82a7a854b8c7c617a4a54bdd7 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Wed, 13 Nov 2024 14:09:55 +0200 Subject: [PATCH 14/35] Updated accoeding to comments --- .../websockets/data_providers/blocks_provider.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index 62da3a3cbd9..605894b67ec 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -47,15 +47,15 @@ func NewBlocksDataProvider( api: api, } - // Validate arguments passed to the provider. - err := p.validateArguments(arguments) + // Initialize arguments passed to the provider. + err := p.initArguments(arguments) if err != nil { return nil, fmt.Errorf("invalid arguments: %w", err) } context, cancel := context.WithCancel(ctx) - // Set up a subscription to blocks based on the provided start block ID and block status. + // Set up a subscription to blocks based on arguments. subscription := p.createSubscription(context) p.BaseDataProviderImpl = NewBaseDataProviderImpl( cancel, @@ -74,10 +74,10 @@ func (p *BlocksDataProvider) Run() error { return subscription.HandleSubscription(p.subscription, handleResponse(p.send, p.args.BlockStatus)) } -// validateArguments checks and validates the arguments passed to the provider. +// initArguments checks and validates the arguments passed to the provider. // // No errors are expected during normal operations. -func (p *BlocksDataProvider) validateArguments(arguments map[string]string) error { +func (p *BlocksDataProvider) initArguments(arguments map[string]string) error { // Parse 'block_status' if blockStatusIn, ok := arguments["block_status"]; ok { blockStatus, err := parser.ParseBlockStatus(blockStatusIn) @@ -118,7 +118,7 @@ func (p *BlocksDataProvider) validateArguments(arguments map[string]string) erro return nil } -// createSubscription creates a new subscription based on the specified start block ID or height. +// createSubscription creates a new subscription using the specified input arguments. func (p *BlocksDataProvider) createSubscription(ctx context.Context) subscription.Subscription { if p.args.StartBlockID != flow.ZeroID { return p.api.SubscribeBlocksFromStartBlockID(ctx, p.args.StartBlockID, p.args.BlockStatus) From 286d3d714cd502d346a77ef87e029d1d6f40dd49 Mon Sep 17 00:00:00 2001 From: Uliana Andrukhiv Date: Wed, 13 Nov 2024 15:18:52 +0200 Subject: [PATCH 15/35] Update topics order Co-authored-by: Andrii Slisarchuk --- engine/access/rest/websockets/data_providers/factory.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/engine/access/rest/websockets/data_providers/factory.go b/engine/access/rest/websockets/data_providers/factory.go index 7061215ac20..05b9aa29e40 100644 --- a/engine/access/rest/websockets/data_providers/factory.go +++ b/engine/access/rest/websockets/data_providers/factory.go @@ -15,11 +15,10 @@ import ( const ( EventsTopic = "events" AccountStatusesTopic = "account_statuses" + BlocksTopic = "blocks" BlockHeadersTopic = "block_headers" BlockDigestsTopic = "block_digests" TransactionStatusesTopic = "transaction_statuses" - - BlocksTopic = "blocks" ) // DataProviderFactory is responsible for creating data providers based on the From 08ec8cfc5e0630b638bc10ef28b94fc06e6b88e6 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 19 Nov 2024 14:01:57 +0200 Subject: [PATCH 16/35] Added implementation of block headers data provider, added unit tests --- .../data_providers/block_headers_provider.go | 93 +++++++++++++++++++ .../block_headers_provider_test.go | 46 +++++++++ .../data_providers/blocks_provider.go | 87 ++++++++--------- .../data_providers/blocks_provider_test.go | 36 ++++--- .../rest/websockets/data_providers/factory.go | 3 +- .../rest/websockets/models/block_models.go | 10 ++ 6 files changed, 218 insertions(+), 57 deletions(-) create mode 100644 engine/access/rest/websockets/data_providers/block_headers_provider.go create mode 100644 engine/access/rest/websockets/data_providers/block_headers_provider_test.go diff --git a/engine/access/rest/websockets/data_providers/block_headers_provider.go b/engine/access/rest/websockets/data_providers/block_headers_provider.go new file mode 100644 index 00000000000..969a7c60244 --- /dev/null +++ b/engine/access/rest/websockets/data_providers/block_headers_provider.go @@ -0,0 +1,93 @@ +package data_providers + +import ( + "context" + "fmt" + + "github.com/rs/zerolog" + + "github.com/onflow/flow-go/access" + "github.com/onflow/flow-go/engine/access/rest/http/request" + "github.com/onflow/flow-go/engine/access/rest/websockets/models" + "github.com/onflow/flow-go/engine/access/subscription" + "github.com/onflow/flow-go/model/flow" +) + +// BlockHeadersDataProvider is responsible for providing block headers +type BlockHeadersDataProvider struct { + *BaseDataProviderImpl + + logger zerolog.Logger + args BlocksArguments + api access.API +} + +var _ DataProvider = (*BlockHeadersDataProvider)(nil) + +// NewBlockHeadersDataProvider creates a new instance of BlockHeadersDataProvider. +func NewBlockHeadersDataProvider( + ctx context.Context, + logger zerolog.Logger, + api access.API, + topic string, + arguments map[string]string, + send chan<- interface{}, +) (*BlockHeadersDataProvider, error) { + p := &BlockHeadersDataProvider{ + logger: logger.With().Str("component", "block-headers-data-provider").Logger(), + api: api, + } + + // Initialize arguments passed to the provider. + var err error + p.args, err = ParseBlocksArguments(arguments) + if err != nil { + return nil, fmt.Errorf("invalid arguments: %w", err) + } + + context, cancel := context.WithCancel(ctx) + + // Set up a subscription to block headers based on arguments. + subscription := p.createSubscription(context) + p.BaseDataProviderImpl = NewBaseDataProviderImpl( + cancel, + topic, + send, + subscription, + ) + + return p, nil +} + +// Run starts processing the subscription for block headers and handles responses. +// +// No errors are expected during normal operations. +func (p *BlockHeadersDataProvider) Run() error { + return subscription.HandleSubscription(p.subscription, p.handleResponse(p.send)) +} + +// createSubscription creates a new subscription using the specified input arguments. +func (p *BlockHeadersDataProvider) createSubscription(ctx context.Context) subscription.Subscription { + if p.args.StartBlockID != flow.ZeroID { + return p.api.SubscribeBlockHeadersFromStartBlockID(ctx, p.args.StartBlockID, p.args.BlockStatus) + } + + if p.args.StartBlockHeight != request.EmptyHeight { + return p.api.SubscribeBlockHeadersFromStartHeight(ctx, p.args.StartBlockHeight, p.args.BlockStatus) + } + + return p.api.SubscribeBlockHeadersFromLatest(ctx, p.args.BlockStatus) +} + +// handleResponse processes a block header and sends the formatted response. +// +// No errors are expected during normal operations. +func (p *BlockHeadersDataProvider) handleResponse(send chan<- interface{}) func(header *flow.Header) error { + return func(header *flow.Header) error { + send <- &models.BlockHeaderMessageResponse{ + Header: header, + } + + return nil + } +} diff --git a/engine/access/rest/websockets/data_providers/block_headers_provider_test.go b/engine/access/rest/websockets/data_providers/block_headers_provider_test.go new file mode 100644 index 00000000000..7d26e80d387 --- /dev/null +++ b/engine/access/rest/websockets/data_providers/block_headers_provider_test.go @@ -0,0 +1,46 @@ +package data_providers + +import ( + "context" + "testing" + + "github.com/stretchr/testify/suite" +) + +type TestBlockHeadersProviderSuite struct { + BlocksProviderSuite +} + +func TestBackendBlockHeadersSuite(t *testing.T) { + suite.Run(t, new(TestBlockHeadersProviderSuite)) +} + +// SetupTest initializes the test suite with required dependencies. +func (s *TestBlockHeadersProviderSuite) SetupTest() { + s.BlocksProviderSuite.SetupTest() +} + +// TestBlockHeadersDataProvider_InvalidArguments tests the behavior of the block headers data provider +// when invalid arguments are provided. It verifies that appropriate errors are returned +// for missing or conflicting arguments. +// This test covers the test cases: +// 1. Missing 'block_status' argument. +// 2. Invalid 'block_status' argument. +// 3. Providing both 'start_block_id' and 'start_block_height' simultaneously. +func (s *TestBlockHeadersProviderSuite) TestBlockHeadersDataProvider_InvalidArguments() { + ctx := context.Background() + send := make(chan interface{}) + + topic := BlockHeadersTopic + + for _, test := range s.invalidArgumentsTestCases() { + s.Run(test.name, func() { + provider, err := NewBlockHeadersDataProvider(ctx, s.log, s.api, topic, test.arguments, send) + s.Require().Nil(provider) + s.Require().Error(err) + s.Require().Contains(err.Error(), test.expectedErrorMsg) + }) + } +} + +// TODO: add tests for responses after the WebsocketController is ready diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index 605894b67ec..01a18b5c4dd 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -43,12 +43,13 @@ func NewBlocksDataProvider( send chan<- interface{}, ) (*BlocksDataProvider, error) { p := &BlocksDataProvider{ - logger: logger.With().Str("component", "block-data-provider").Logger(), + logger: logger.With().Str("component", "blocks-data-provider").Logger(), api: api, } // Initialize arguments passed to the provider. - err := p.initArguments(arguments) + var err error + p.args, err = ParseBlocksArguments(arguments) if err != nil { return nil, fmt.Errorf("invalid arguments: %w", err) } @@ -71,22 +72,48 @@ func NewBlocksDataProvider( // // No errors are expected during normal operations. func (p *BlocksDataProvider) Run() error { - return subscription.HandleSubscription(p.subscription, handleResponse(p.send, p.args.BlockStatus)) + return subscription.HandleSubscription(p.subscription, p.handleResponse(p.send)) } -// initArguments checks and validates the arguments passed to the provider. +// createSubscription creates a new subscription using the specified input arguments. +func (p *BlocksDataProvider) createSubscription(ctx context.Context) subscription.Subscription { + if p.args.StartBlockID != flow.ZeroID { + return p.api.SubscribeBlocksFromStartBlockID(ctx, p.args.StartBlockID, p.args.BlockStatus) + } + + if p.args.StartBlockHeight != request.EmptyHeight { + return p.api.SubscribeBlocksFromStartHeight(ctx, p.args.StartBlockHeight, p.args.BlockStatus) + } + + return p.api.SubscribeBlocksFromLatest(ctx, p.args.BlockStatus) +} + +// handleResponse processes a block and sends the formatted response. // // No errors are expected during normal operations. -func (p *BlocksDataProvider) initArguments(arguments map[string]string) error { +func (p *BlocksDataProvider) handleResponse(send chan<- interface{}) func(*flow.Block) error { + return func(block *flow.Block) error { + send <- &models.BlockMessageResponse{ + Block: block, + } + + return nil + } +} + +// ParseBlocksArguments validates and initializes the blocks arguments. +func ParseBlocksArguments(arguments map[string]string) (BlocksArguments, error) { + var args BlocksArguments + // Parse 'block_status' if blockStatusIn, ok := arguments["block_status"]; ok { blockStatus, err := parser.ParseBlockStatus(blockStatusIn) if err != nil { - return err + return args, err } - p.args.BlockStatus = blockStatus + args.BlockStatus = blockStatus } else { - return fmt.Errorf("'block_status' must be provided") + return args, fmt.Errorf("'block_status' must be provided") } // Parse 'start_block_id' if provided @@ -94,52 +121,26 @@ func (p *BlocksDataProvider) initArguments(arguments map[string]string) error { var startBlockID parser.ID err := startBlockID.Parse(startBlockIDIn) if err != nil { - return err + return args, err } - p.args.StartBlockID = startBlockID.Flow() + args.StartBlockID = startBlockID.Flow() } // Parse 'start_block_height' if provided if startBlockHeightIn, ok := arguments["start_block_height"]; ok { var err error - p.args.StartBlockHeight, err = util.ToUint64(startBlockHeightIn) + args.StartBlockHeight, err = util.ToUint64(startBlockHeightIn) if err != nil { - return fmt.Errorf("invalid 'start_block_height': %w", err) + return args, fmt.Errorf("invalid 'start_block_height': %w", err) } } else { - p.args.StartBlockHeight = request.EmptyHeight + args.StartBlockHeight = request.EmptyHeight } - // if both start_block_id and start_height are provided - if p.args.StartBlockID != flow.ZeroID && p.args.StartBlockHeight != request.EmptyHeight { - return fmt.Errorf("can only provide either 'start_block_id' or 'start_block_height'") + // Ensure only one of start_block_id or start_block_height is provided + if args.StartBlockID != flow.ZeroID && args.StartBlockHeight != request.EmptyHeight { + return args, fmt.Errorf("can only provide either 'start_block_id' or 'start_block_height'") } - return nil -} - -// createSubscription creates a new subscription using the specified input arguments. -func (p *BlocksDataProvider) createSubscription(ctx context.Context) subscription.Subscription { - if p.args.StartBlockID != flow.ZeroID { - return p.api.SubscribeBlocksFromStartBlockID(ctx, p.args.StartBlockID, p.args.BlockStatus) - } - - if p.args.StartBlockHeight != request.EmptyHeight { - return p.api.SubscribeBlocksFromStartHeight(ctx, p.args.StartBlockHeight, p.args.BlockStatus) - } - - return p.api.SubscribeBlocksFromLatest(ctx, p.args.BlockStatus) -} - -// handleResponse processes a block and sends the formatted response. -// -// No errors are expected during normal operations. -func handleResponse(send chan<- interface{}, blockStatus flow.BlockStatus) func(*flow.Block) error { - return func(block *flow.Block) error { - send <- &models.BlockMessageResponse{ - Block: block, - } - - return nil - } + return args, nil } diff --git a/engine/access/rest/websockets/data_providers/blocks_provider_test.go b/engine/access/rest/websockets/data_providers/blocks_provider_test.go index 4a619d7bfcf..9771cb54780 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider_test.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider_test.go @@ -57,18 +57,16 @@ func (s *BlocksProviderSuite) SetupTest() { s.finalizedBlock = parent } -// TestBlocksDataProvider_InvalidArguments tests the behavior of the block data provider -// when invalid arguments are provided. It verifies that appropriate errors are returned -// for missing or conflicting arguments. -// This test covers the test cases: -// 1. Missing 'block_status' argument. -// 2. Invalid 'block_status' argument. -// 3. Providing both 'start_block_id' and 'start_block_height' simultaneously. -func (s *BlocksProviderSuite) TestBlocksDataProvider_InvalidArguments() { - ctx := context.Background() - send := make(chan interface{}) - - testCases := []testErrType{ +// invalidArgumentsTestCases returns a list of test cases with invalid argument combinations +// for testing the behavior of block, block headers, block digests data providers. Each test case includes a name, +// a set of input arguments, and the expected error message that should be returned. +// +// The test cases cover scenarios such as: +// 1. Missing the required 'block_status' argument. +// 2. Providing an unknown or invalid 'block_status' value. +// 3. Supplying both 'start_block_id' and 'start_block_height' simultaneously, which is not allowed. +func (s *BlocksProviderSuite) invalidArgumentsTestCases() []testErrType { + return []testErrType{ { name: "missing 'block_status' argument", arguments: map[string]string{ @@ -93,10 +91,22 @@ func (s *BlocksProviderSuite) TestBlocksDataProvider_InvalidArguments() { expectedErrorMsg: "can only provide either 'start_block_id' or 'start_block_height'", }, } +} + +// TestBlocksDataProvider_InvalidArguments tests the behavior of the block data provider +// when invalid arguments are provided. It verifies that appropriate errors are returned +// for missing or conflicting arguments. +// This test covers the test cases: +// 1. Missing 'block_status' argument. +// 2. Invalid 'block_status' argument. +// 3. Providing both 'start_block_id' and 'start_block_height' simultaneously. +func (s *BlocksProviderSuite) TestBlocksDataProvider_InvalidArguments() { + ctx := context.Background() + send := make(chan interface{}) topic := BlocksTopic - for _, test := range testCases { + for _, test := range s.invalidArgumentsTestCases() { s.Run(test.name, func() { provider, err := NewBlocksDataProvider(ctx, s.log, s.api, topic, test.arguments, send) s.Require().Nil(provider) diff --git a/engine/access/rest/websockets/data_providers/factory.go b/engine/access/rest/websockets/data_providers/factory.go index 05b9aa29e40..c9439266f82 100644 --- a/engine/access/rest/websockets/data_providers/factory.go +++ b/engine/access/rest/websockets/data_providers/factory.go @@ -72,10 +72,11 @@ func (s *DataProviderFactory) NewDataProvider( switch topic { case BlocksTopic: return NewBlocksDataProvider(ctx, s.logger, s.accessApi, topic, arguments, ch) + case BlockHeadersTopic: + return NewBlockHeadersDataProvider(ctx, s.logger, s.accessApi, topic, arguments, ch) // TODO: Implemented handlers for each topic should be added in respective case case EventsTopic, AccountStatusesTopic, - BlockHeadersTopic, BlockDigestsTopic, TransactionStatusesTopic: return nil, fmt.Errorf("topic \"%s\" not implemented yet", topic) diff --git a/engine/access/rest/websockets/models/block_models.go b/engine/access/rest/websockets/models/block_models.go index 2e3e378d832..9eb8c30ee1f 100644 --- a/engine/access/rest/websockets/models/block_models.go +++ b/engine/access/rest/websockets/models/block_models.go @@ -4,6 +4,16 @@ import ( "github.com/onflow/flow-go/model/flow" ) +// BlockMessageResponse is the response message for 'blocks' topic. type BlockMessageResponse struct { + // The sealed or finalized blocks according to the block status + // in the request. Block *flow.Block `json:"block"` } + +// BlockHeaderMessageResponse is the response message for 'block_headers' topic. +type BlockHeaderMessageResponse struct { + // The sealed or finalized block headers according to the block status + // in the request. + Header *flow.Header `json:"header"` +} From 16100da4efcb2eedf6db95978df99d253006cd58 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 19 Nov 2024 14:28:20 +0200 Subject: [PATCH 17/35] Added implementation of block digests data provider, added unit tests --- .../data_providers/block_digests_provider.go | 93 +++++++++++++++++++ .../block_digests_provider_test.go | 46 +++++++++ .../block_headers_provider_test.go | 10 +- .../rest/websockets/data_providers/factory.go | 3 +- .../websockets/data_providers/factory_test.go | 32 +++++++ .../rest/websockets/models/block_models.go | 7 ++ 6 files changed, 185 insertions(+), 6 deletions(-) create mode 100644 engine/access/rest/websockets/data_providers/block_digests_provider.go create mode 100644 engine/access/rest/websockets/data_providers/block_digests_provider_test.go diff --git a/engine/access/rest/websockets/data_providers/block_digests_provider.go b/engine/access/rest/websockets/data_providers/block_digests_provider.go new file mode 100644 index 00000000000..30f7ded3b15 --- /dev/null +++ b/engine/access/rest/websockets/data_providers/block_digests_provider.go @@ -0,0 +1,93 @@ +package data_providers + +import ( + "context" + "fmt" + + "github.com/rs/zerolog" + + "github.com/onflow/flow-go/access" + "github.com/onflow/flow-go/engine/access/rest/http/request" + "github.com/onflow/flow-go/engine/access/rest/websockets/models" + "github.com/onflow/flow-go/engine/access/subscription" + "github.com/onflow/flow-go/model/flow" +) + +// BlockDigestsDataProvider is responsible for providing block digests +type BlockDigestsDataProvider struct { + *BaseDataProviderImpl + + logger zerolog.Logger + args BlocksArguments + api access.API +} + +var _ DataProvider = (*BlockDigestsDataProvider)(nil) + +// NewBlockDigestsDataProvider creates a new instance of BlockDigestsDataProvider. +func NewBlockDigestsDataProvider( + ctx context.Context, + logger zerolog.Logger, + api access.API, + topic string, + arguments map[string]string, + send chan<- interface{}, +) (*BlockDigestsDataProvider, error) { + p := &BlockDigestsDataProvider{ + logger: logger.With().Str("component", "block-digests-data-provider").Logger(), + api: api, + } + + // Initialize arguments passed to the provider. + var err error + p.args, err = ParseBlocksArguments(arguments) + if err != nil { + return nil, fmt.Errorf("invalid arguments: %w", err) + } + + context, cancel := context.WithCancel(ctx) + + // Set up a subscription to block digests based on arguments. + subscription := p.createSubscription(context) + p.BaseDataProviderImpl = NewBaseDataProviderImpl( + cancel, + topic, + send, + subscription, + ) + + return p, nil +} + +// Run starts processing the subscription for block digests and handles responses. +// +// No errors are expected during normal operations. +func (p *BlockDigestsDataProvider) Run() error { + return subscription.HandleSubscription(p.subscription, p.handleResponse(p.send)) +} + +// createSubscription creates a new subscription using the specified input arguments. +func (p *BlockDigestsDataProvider) createSubscription(ctx context.Context) subscription.Subscription { + if p.args.StartBlockID != flow.ZeroID { + return p.api.SubscribeBlockDigestsFromStartBlockID(ctx, p.args.StartBlockID, p.args.BlockStatus) + } + + if p.args.StartBlockHeight != request.EmptyHeight { + return p.api.SubscribeBlockDigestsFromStartHeight(ctx, p.args.StartBlockHeight, p.args.BlockStatus) + } + + return p.api.SubscribeBlockDigestsFromLatest(ctx, p.args.BlockStatus) +} + +// handleResponse processes a block digest and sends the formatted response. +// +// No errors are expected during normal operations. +func (p *BlockDigestsDataProvider) handleResponse(send chan<- interface{}) func(block *flow.BlockDigest) error { + return func(block *flow.BlockDigest) error { + send <- &models.BlockDigestMessageResponse{ + Block: block, + } + + return nil + } +} diff --git a/engine/access/rest/websockets/data_providers/block_digests_provider_test.go b/engine/access/rest/websockets/data_providers/block_digests_provider_test.go new file mode 100644 index 00000000000..57fdabd7994 --- /dev/null +++ b/engine/access/rest/websockets/data_providers/block_digests_provider_test.go @@ -0,0 +1,46 @@ +package data_providers + +import ( + "context" + "testing" + + "github.com/stretchr/testify/suite" +) + +type BlockDigestsProviderSuite struct { + BlocksProviderSuite +} + +func TestBlockDigestsProviderSuite(t *testing.T) { + suite.Run(t, new(BlockDigestsProviderSuite)) +} + +// SetupTest initializes the test suite with required dependencies. +func (s *BlockDigestsProviderSuite) SetupTest() { + s.BlocksProviderSuite.SetupTest() +} + +// TestBlockDigestsDataProvider_InvalidArguments tests the behavior of the block digests data provider +// when invalid arguments are provided. It verifies that appropriate errors are returned +// for missing or conflicting arguments. +// This test covers the test cases: +// 1. Missing 'block_status' argument. +// 2. Invalid 'block_status' argument. +// 3. Providing both 'start_block_id' and 'start_block_height' simultaneously. +func (s *BlockDigestsProviderSuite) TestBlockDigestsDataProvider_InvalidArguments() { + ctx := context.Background() + send := make(chan interface{}) + + topic := BlockDigestsTopic + + for _, test := range s.invalidArgumentsTestCases() { + s.Run(test.name, func() { + provider, err := NewBlockDigestsDataProvider(ctx, s.log, s.api, topic, test.arguments, send) + s.Require().Nil(provider) + s.Require().Error(err) + s.Require().Contains(err.Error(), test.expectedErrorMsg) + }) + } +} + +// TODO: add tests for responses after the WebsocketController is ready diff --git a/engine/access/rest/websockets/data_providers/block_headers_provider_test.go b/engine/access/rest/websockets/data_providers/block_headers_provider_test.go index 7d26e80d387..efd94916e92 100644 --- a/engine/access/rest/websockets/data_providers/block_headers_provider_test.go +++ b/engine/access/rest/websockets/data_providers/block_headers_provider_test.go @@ -7,16 +7,16 @@ import ( "github.com/stretchr/testify/suite" ) -type TestBlockHeadersProviderSuite struct { +type BlockHeadersProviderSuite struct { BlocksProviderSuite } -func TestBackendBlockHeadersSuite(t *testing.T) { - suite.Run(t, new(TestBlockHeadersProviderSuite)) +func TestBlockHeadersProviderSuite(t *testing.T) { + suite.Run(t, new(BlockHeadersProviderSuite)) } // SetupTest initializes the test suite with required dependencies. -func (s *TestBlockHeadersProviderSuite) SetupTest() { +func (s *BlockHeadersProviderSuite) SetupTest() { s.BlocksProviderSuite.SetupTest() } @@ -27,7 +27,7 @@ func (s *TestBlockHeadersProviderSuite) SetupTest() { // 1. Missing 'block_status' argument. // 2. Invalid 'block_status' argument. // 3. Providing both 'start_block_id' and 'start_block_height' simultaneously. -func (s *TestBlockHeadersProviderSuite) TestBlockHeadersDataProvider_InvalidArguments() { +func (s *BlockHeadersProviderSuite) TestBlockHeadersDataProvider_InvalidArguments() { ctx := context.Background() send := make(chan interface{}) diff --git a/engine/access/rest/websockets/data_providers/factory.go b/engine/access/rest/websockets/data_providers/factory.go index c9439266f82..ccff93488a0 100644 --- a/engine/access/rest/websockets/data_providers/factory.go +++ b/engine/access/rest/websockets/data_providers/factory.go @@ -74,10 +74,11 @@ func (s *DataProviderFactory) NewDataProvider( return NewBlocksDataProvider(ctx, s.logger, s.accessApi, topic, arguments, ch) case BlockHeadersTopic: return NewBlockHeadersDataProvider(ctx, s.logger, s.accessApi, topic, arguments, ch) + case BlockDigestsTopic: + return NewBlockDigestsDataProvider(ctx, s.logger, s.accessApi, topic, arguments, ch) // TODO: Implemented handlers for each topic should be added in respective case case EventsTopic, AccountStatusesTopic, - BlockDigestsTopic, TransactionStatusesTopic: return nil, fmt.Errorf("topic \"%s\" not implemented yet", topic) default: diff --git a/engine/access/rest/websockets/data_providers/factory_test.go b/engine/access/rest/websockets/data_providers/factory_test.go index 2cbbb1276df..3d85778f3a9 100644 --- a/engine/access/rest/websockets/data_providers/factory_test.go +++ b/engine/access/rest/websockets/data_providers/factory_test.go @@ -76,6 +76,38 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { s.accessApi.AssertExpectations(s.T()) }, }, + { + name: "block headers topic", + topic: BlockHeadersTopic, + arguments: map[string]string{"block_status": parser.Finalized}, + mockSubscription: func() string { + subscription := statestreammock.NewSubscription(s.T()) + subscriptionID := unittest.IdentifierFixture().String() + subscription.On("ID").Return(subscriptionID).Once() + + s.accessApi.On("SubscribeBlockHeadersFromLatest", mock.Anything, flow.BlockStatusFinalized).Return(subscription).Once() + return subscriptionID + }, + assertExpectations: func() { + s.accessApi.AssertExpectations(s.T()) + }, + }, + { + name: "block digests topic", + topic: BlockDigestsTopic, + arguments: map[string]string{"block_status": parser.Finalized}, + mockSubscription: func() string { + subscription := statestreammock.NewSubscription(s.T()) + subscriptionID := unittest.IdentifierFixture().String() + subscription.On("ID").Return(subscriptionID).Once() + + s.accessApi.On("SubscribeBlockDigestsFromLatest", mock.Anything, flow.BlockStatusFinalized).Return(subscription).Once() + return subscriptionID + }, + assertExpectations: func() { + s.accessApi.AssertExpectations(s.T()) + }, + }, } for _, test := range testCases { diff --git a/engine/access/rest/websockets/models/block_models.go b/engine/access/rest/websockets/models/block_models.go index 9eb8c30ee1f..fa7af987236 100644 --- a/engine/access/rest/websockets/models/block_models.go +++ b/engine/access/rest/websockets/models/block_models.go @@ -17,3 +17,10 @@ type BlockHeaderMessageResponse struct { // in the request. Header *flow.Header `json:"header"` } + +// BlockDigestMessageResponse is the response message for 'block_digests' topic. +type BlockDigestMessageResponse struct { + // The sealed or finalized block digest according to the block status + // in the request. + Block *flow.BlockDigest `json:"block_digest"` +} From 2fa0767fcb527d97440bcb0c85e603e24e05ebcd Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 19 Nov 2024 17:19:17 +0200 Subject: [PATCH 18/35] Updated according to comments --- engine/access/rest/server.go | 2 +- .../data_providers/blocks_provider.go | 2 +- .../rest/websockets/data_providers/factory.go | 11 +++---- .../websockets/data_providers/factory_test.go | 33 ++++++++++--------- 4 files changed, 23 insertions(+), 25 deletions(-) diff --git a/engine/access/rest/server.go b/engine/access/rest/server.go index f3c0a79194a..efaf36bdd44 100644 --- a/engine/access/rest/server.go +++ b/engine/access/rest/server.go @@ -50,7 +50,7 @@ func NewServer(serverAPI access.API, } // TODO: add new websocket routes - _ = data_providers.NewDataProviderFactory(logger, stateStreamConfig.EventFilterConfig, stateStreamApi, serverAPI) + _ = data_providers.NewDataProviderFactory(logger, stateStreamApi, serverAPI) c := cors.New(cors.Options{ AllowedOrigins: []string{"*"}, diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index 01a18b5c4dd..691e165f9e8 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -15,7 +15,7 @@ import ( "github.com/onflow/flow-go/model/flow" ) -// BlocksArguments contains the arguments required for subscribing to blocks +// BlocksArguments contains the arguments required for subscribing to blocks / block headers / block digests type BlocksArguments struct { StartBlockID flow.Identifier // ID of the block to start subscription from StartBlockHeight uint64 // Height of the block to start subscription from diff --git a/engine/access/rest/websockets/data_providers/factory.go b/engine/access/rest/websockets/data_providers/factory.go index ccff93488a0..6ec8dd3185a 100644 --- a/engine/access/rest/websockets/data_providers/factory.go +++ b/engine/access/rest/websockets/data_providers/factory.go @@ -25,8 +25,7 @@ const ( // requested topic. It manages access to logging, state stream configuration, // and relevant APIs needed to retrieve data. type DataProviderFactory struct { - logger zerolog.Logger - eventFilterConfig state_stream.EventFilterConfig + logger zerolog.Logger stateStreamApi state_stream.API accessApi access.API @@ -41,15 +40,13 @@ type DataProviderFactory struct { // - accessApi: API for accessing data from the Flow Access API. func NewDataProviderFactory( logger zerolog.Logger, - eventFilterConfig state_stream.EventFilterConfig, stateStreamApi state_stream.API, accessApi access.API, ) *DataProviderFactory { return &DataProviderFactory{ - logger: logger, - eventFilterConfig: eventFilterConfig, - stateStreamApi: stateStreamApi, - accessApi: accessApi, + logger: logger, + stateStreamApi: stateStreamApi, + accessApi: accessApi, } } diff --git a/engine/access/rest/websockets/data_providers/factory_test.go b/engine/access/rest/websockets/data_providers/factory_test.go index 3d85778f3a9..90c3c8d3b93 100644 --- a/engine/access/rest/websockets/data_providers/factory_test.go +++ b/engine/access/rest/websockets/data_providers/factory_test.go @@ -10,7 +10,6 @@ import ( accessmock "github.com/onflow/flow-go/access/mock" "github.com/onflow/flow-go/engine/access/rest/common/parser" - "github.com/onflow/flow-go/engine/access/state_stream" statestreammock "github.com/onflow/flow-go/engine/access/state_stream/mock" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/utils/unittest" @@ -37,17 +36,26 @@ func TestDataProviderFactorySuite(t *testing.T) { // It initializes the factory with mock instances and validates that it is created successfully. func (s *DataProviderFactorySuite) SetupTest() { log := unittest.Logger() - eventFilterConfig := state_stream.EventFilterConfig{} s.stateStreamApi = statestreammock.NewAPI(s.T()) s.accessApi = accessmock.NewAPI(s.T()) s.ctx = context.Background() s.ch = make(chan interface{}) - s.factory = NewDataProviderFactory(log, eventFilterConfig, s.stateStreamApi, s.accessApi) + s.factory = NewDataProviderFactory(log, s.stateStreamApi, s.accessApi) s.Require().NotNil(s.factory) } +// mockSubscription creates a mock subscription instance for testing purposes. +// It sets up the mock subscription's ID method to return a predefined identifiers +func (s *DataProviderFactorySuite) mockSubscription() *statestreammock.Subscription { + subscription := statestreammock.NewSubscription(s.T()) + subscriptionID := unittest.IdentifierFixture().String() + subscription.On("ID").Return(subscriptionID).Twice() + + return subscription +} + // TODO: add others topic to check when they will be implemented // TestSupportedTopics verifies that supported topics return a valid provider and no errors. // Each test case includes a topic and arguments for which a data provider should be created. @@ -65,12 +73,9 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { topic: BlocksTopic, arguments: map[string]string{"block_status": parser.Finalized}, mockSubscription: func() string { - subscription := statestreammock.NewSubscription(s.T()) - subscriptionID := unittest.IdentifierFixture().String() - subscription.On("ID").Return(subscriptionID).Once() - + subscription := s.mockSubscription() s.accessApi.On("SubscribeBlocksFromLatest", mock.Anything, flow.BlockStatusFinalized).Return(subscription).Once() - return subscriptionID + return subscription.ID() }, assertExpectations: func() { s.accessApi.AssertExpectations(s.T()) @@ -81,12 +86,10 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { topic: BlockHeadersTopic, arguments: map[string]string{"block_status": parser.Finalized}, mockSubscription: func() string { - subscription := statestreammock.NewSubscription(s.T()) - subscriptionID := unittest.IdentifierFixture().String() - subscription.On("ID").Return(subscriptionID).Once() + subscription := s.mockSubscription() s.accessApi.On("SubscribeBlockHeadersFromLatest", mock.Anything, flow.BlockStatusFinalized).Return(subscription).Once() - return subscriptionID + return subscription.ID() }, assertExpectations: func() { s.accessApi.AssertExpectations(s.T()) @@ -97,12 +100,10 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { topic: BlockDigestsTopic, arguments: map[string]string{"block_status": parser.Finalized}, mockSubscription: func() string { - subscription := statestreammock.NewSubscription(s.T()) - subscriptionID := unittest.IdentifierFixture().String() - subscription.On("ID").Return(subscriptionID).Once() + subscription := s.mockSubscription() s.accessApi.On("SubscribeBlockDigestsFromLatest", mock.Anything, flow.BlockStatusFinalized).Return(subscription).Once() - return subscriptionID + return subscription.ID() }, assertExpectations: func() { s.accessApi.AssertExpectations(s.T()) From c3391936e4e4d4b0737bf9f05e6a4ba9eec405e8 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 19 Nov 2024 17:59:36 +0200 Subject: [PATCH 19/35] Updated TestSupportedTopics unit test --- .../websockets/data_providers/factory_test.go | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/engine/access/rest/websockets/data_providers/factory_test.go b/engine/access/rest/websockets/data_providers/factory_test.go index 90c3c8d3b93..1b56a4d7cac 100644 --- a/engine/access/rest/websockets/data_providers/factory_test.go +++ b/engine/access/rest/websockets/data_providers/factory_test.go @@ -46,14 +46,16 @@ func (s *DataProviderFactorySuite) SetupTest() { s.Require().NotNil(s.factory) } -// mockSubscription creates a mock subscription instance for testing purposes. -// It sets up the mock subscription's ID method to return a predefined identifiers -func (s *DataProviderFactorySuite) mockSubscription() *statestreammock.Subscription { +// setupSubscription creates a mock subscription instance for testing purposes. +// It configures the mock subscription's `ID` method to return a predefined subscription identifier. +// Additionally, it sets the return value of the specified API call to the mock subscription. +func (s *DataProviderFactorySuite) setupSubscription(apiCall *mock.Call) string { subscription := statestreammock.NewSubscription(s.T()) subscriptionID := unittest.IdentifierFixture().String() - subscription.On("ID").Return(subscriptionID).Twice() + subscription.On("ID").Return(subscriptionID).Once() - return subscription + apiCall.Return(subscription).Once() + return subscriptionID } // TODO: add others topic to check when they will be implemented @@ -65,17 +67,15 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { name string topic string arguments map[string]string - mockSubscription func() string // return subscription id + setupSubscription func() string // return subscription id assertExpectations func() }{ { name: "block topic", topic: BlocksTopic, arguments: map[string]string{"block_status": parser.Finalized}, - mockSubscription: func() string { - subscription := s.mockSubscription() - s.accessApi.On("SubscribeBlocksFromLatest", mock.Anything, flow.BlockStatusFinalized).Return(subscription).Once() - return subscription.ID() + setupSubscription: func() string { + return s.setupSubscription(s.accessApi.On("SubscribeBlocksFromLatest", mock.Anything, flow.BlockStatusFinalized)) }, assertExpectations: func() { s.accessApi.AssertExpectations(s.T()) @@ -85,11 +85,8 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { name: "block headers topic", topic: BlockHeadersTopic, arguments: map[string]string{"block_status": parser.Finalized}, - mockSubscription: func() string { - subscription := s.mockSubscription() - - s.accessApi.On("SubscribeBlockHeadersFromLatest", mock.Anything, flow.BlockStatusFinalized).Return(subscription).Once() - return subscription.ID() + setupSubscription: func() string { + return s.setupSubscription(s.accessApi.On("SubscribeBlockHeadersFromLatest", mock.Anything, flow.BlockStatusFinalized)) }, assertExpectations: func() { s.accessApi.AssertExpectations(s.T()) @@ -99,11 +96,8 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { name: "block digests topic", topic: BlockDigestsTopic, arguments: map[string]string{"block_status": parser.Finalized}, - mockSubscription: func() string { - subscription := s.mockSubscription() - - s.accessApi.On("SubscribeBlockDigestsFromLatest", mock.Anything, flow.BlockStatusFinalized).Return(subscription).Once() - return subscription.ID() + setupSubscription: func() string { + return s.setupSubscription(s.accessApi.On("SubscribeBlockDigestsFromLatest", mock.Anything, flow.BlockStatusFinalized)) }, assertExpectations: func() { s.accessApi.AssertExpectations(s.T()) @@ -113,7 +107,7 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { for _, test := range testCases { s.Run(test.name, func() { - subscriptionID := test.mockSubscription() + subscriptionID := test.setupSubscription() provider, err := s.factory.NewDataProvider(s.ctx, test.topic, test.arguments, s.ch) s.Require().NotNil(provider, "Expected provider for topic %s", test.topic) From 9cc130194997a6baa1d561322bbb2b571f29e3ba Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Fri, 22 Nov 2024 16:30:17 +0200 Subject: [PATCH 20/35] Updated godoc, fixed warning with naming --- .../data_providers/block_digests_provider.go | 4 ++-- .../data_providers/block_headers_provider.go | 4 ++-- .../websockets/data_providers/blocks_provider.go | 4 ++-- .../access/rest/websockets/data_providers/factory.go | 12 ++++++------ 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/engine/access/rest/websockets/data_providers/block_digests_provider.go b/engine/access/rest/websockets/data_providers/block_digests_provider.go index 3bb9bc64288..1d99f1a7a8d 100644 --- a/engine/access/rest/websockets/data_providers/block_digests_provider.go +++ b/engine/access/rest/websockets/data_providers/block_digests_provider.go @@ -45,13 +45,13 @@ func NewBlockDigestsDataProvider( return nil, fmt.Errorf("invalid arguments: %w", err) } - context, cancel := context.WithCancel(ctx) + ctx, cancel := context.WithCancel(ctx) p.BaseDataProviderImpl = NewBaseDataProviderImpl( topic, cancel, send, - p.createSubscription(context), // Set up a subscription to block digests based on arguments. + p.createSubscription(ctx), // Set up a subscription to block digests based on arguments. ) return p, nil diff --git a/engine/access/rest/websockets/data_providers/block_headers_provider.go b/engine/access/rest/websockets/data_providers/block_headers_provider.go index 4886da3e18f..40bdb2ddd5c 100644 --- a/engine/access/rest/websockets/data_providers/block_headers_provider.go +++ b/engine/access/rest/websockets/data_providers/block_headers_provider.go @@ -45,13 +45,13 @@ func NewBlockHeadersDataProvider( return nil, fmt.Errorf("invalid arguments: %w", err) } - context, cancel := context.WithCancel(ctx) + ctx, cancel := context.WithCancel(ctx) p.BaseDataProviderImpl = NewBaseDataProviderImpl( topic, cancel, send, - p.createSubscription(context), // Set up a subscription to block headers based on arguments. + p.createSubscription(ctx), // Set up a subscription to block headers based on arguments. ) return p, nil diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index eac39983863..f340c27ba54 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -54,13 +54,13 @@ func NewBlocksDataProvider( return nil, fmt.Errorf("invalid arguments: %w", err) } - context, cancel := context.WithCancel(ctx) + ctx, cancel := context.WithCancel(ctx) p.BaseDataProviderImpl = NewBaseDataProviderImpl( topic, cancel, send, - p.createSubscription(context), // Set up a subscription to blocks based on arguments. + p.createSubscription(ctx), // Set up a subscription to blocks based on arguments. ) return p, nil diff --git a/engine/access/rest/websockets/data_providers/factory.go b/engine/access/rest/websockets/data_providers/factory.go index 8761cb35a69..c9c187a0117 100644 --- a/engine/access/rest/websockets/data_providers/factory.go +++ b/engine/access/rest/websockets/data_providers/factory.go @@ -21,9 +21,9 @@ const ( TransactionStatusesTopic = "transaction_statuses" ) -// DataProviderFactory is responsible for creating data providers based on the -// requested topic. It manages access to logging, state stream configuration, -// and relevant APIs needed to retrieve data. +// DataProviderFactory defines an interface for creating data providers +// based on specified topics. The factory abstracts the creation process +// and ensures consistent access to required APIs. type DataProviderFactory interface { // NewDataProvider creates a new data provider based on the specified topic // and configuration parameters. @@ -34,9 +34,9 @@ type DataProviderFactory interface { var _ DataProviderFactory = (*DataProviderFactoryImpl)(nil) -// DataProviderFactoryImpl is responsible for creating data providers based on the -// requested topic. It manages access to logging, state stream configuration, -// and relevant APIs needed to retrieve data. +// DataProviderFactoryImpl is an implementation of the DataProviderFactory interface. +// It is responsible for creating data providers based on the +// requested topic. It manages access to logging and relevant APIs needed to retrieve data. type DataProviderFactoryImpl struct { logger zerolog.Logger From 64716a546f89b1beebdb96a71322d0ffcb922eec Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Fri, 22 Nov 2024 16:34:44 +0200 Subject: [PATCH 21/35] Updated mocks --- .../data_providers/mock/data_provider.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/engine/access/rest/websockets/data_providers/mock/data_provider.go b/engine/access/rest/websockets/data_providers/mock/data_provider.go index 478f1625ad5..48debb23ae3 100644 --- a/engine/access/rest/websockets/data_providers/mock/data_provider.go +++ b/engine/access/rest/websockets/data_providers/mock/data_provider.go @@ -2,7 +2,10 @@ package mock -import mock "github.com/stretchr/testify/mock" +import ( + uuid "github.com/google/uuid" + mock "github.com/stretchr/testify/mock" +) // DataProvider is an autogenerated mock type for the DataProvider type type DataProvider struct { @@ -15,18 +18,20 @@ func (_m *DataProvider) Close() { } // ID provides a mock function with given fields: -func (_m *DataProvider) ID() string { +func (_m *DataProvider) ID() uuid.UUID { ret := _m.Called() if len(ret) == 0 { panic("no return value specified for ID") } - var r0 string - if rf, ok := ret.Get(0).(func() string); ok { + var r0 uuid.UUID + if rf, ok := ret.Get(0).(func() uuid.UUID); ok { r0 = rf() } else { - r0 = ret.Get(0).(string) + if ret.Get(0) != nil { + r0 = ret.Get(0).(uuid.UUID) + } } return r0 From 49906474c1040ff84b630ce6cfcc7a4632219453 Mon Sep 17 00:00:00 2001 From: Jordan Schalm Date: Fri, 22 Nov 2024 14:19:05 -0800 Subject: [PATCH 22/35] improve dkg engine package docs - put the godoc directly above package declaration so it appears as godoc - remove tabs to correctly align diagram --- engine/consensus/dkg/doc.go | 101 +++++++++++++++++------------------- 1 file changed, 48 insertions(+), 53 deletions(-) diff --git a/engine/consensus/dkg/doc.go b/engine/consensus/dkg/doc.go index 2c72beabc5a..af9393534cf 100644 --- a/engine/consensus/dkg/doc.go +++ b/engine/consensus/dkg/doc.go @@ -1,54 +1,49 @@ -/* - -Package dkg implements engines for the DKG protocol. - -ReactorEngine - -ReactorEngine implements triggers to control the lifecycle of DKG runs. A new -DKG protocol is started when an EpochSetup event is sealed and finalized. The -subsequent phase transitions are triggered when specified views are encountered -(specifically when the first block of a given view is finalized). In between -phase transitions the engine regularly queries the DKG smart-contract to read -broadcast messages. - -MessagingEngine - -MessagingEngine is a network engine that enables consensus nodes to securely -exchange private DKG messages. Note that broadcast messages are not exchanged -through this engine, but rather via the DKG smart-contract. - -Architecture - -For every new epoch, the ReactorEngine instantiates a new DKGController with a -new Broker using the provided ControllerFactory. The ControllerFactory ties new -DKGControllers to the MessagingEngine via a BrokerTunnel which exposes channels -to relay incoming and outgoing messages (cf. module/dkg). - - EpochSetup/OnView - | - v - +---------------+ - | ReactorEngine | - +---------------+ - | - v -*~~~~~~~~~~~~~~~~~~~~~* (one/epoch) -| +---------------+ | -| | Controller | | -| +---------------+ | -| | | -| v | -| +---------------+ | -| | Broker | | -| +---------------+ | -*~~~~~~~~|~~~~~~~~~\~~* - tunnel smart-contract client - | \ - +--------------+ +------------------+ - | Messaging | | DKGSmartContract | - | Engine | | | - +--------------+ +------------------+ - -*/ - +// Package dkg implements engines for the DKG protocol. +// +// # Reactor Engine +// +// The [ReactorEngine] implements triggers to control the lifecycle of DKG runs. A new +// DKG protocol is started when an EpochSetup event is sealed and finalized. The +// subsequent phase transitions are triggered when specified views are encountered +// (specifically when the first block of a given view is finalized). In between +// phase transitions the engine regularly queries the DKG smart-contract to read +// broadcast messages. +// +// # Messaging Engine +// +// The [MessagingEngine] is a network engine that enables consensus nodes to securely exchange +// private (not broadcast) DKG messages. Broadcast messages are sent via the DKG smart contract. +// +// # Architecture +// +// For every new epoch, the [ReactorEngine] instantiates a new [module.DKGController] with a +// new [module.DKGBroker] using the provided ControllerFactory. The ControllerFactory ties new +// DKGControllers to the [MessagingEngine] via a BrokerTunnel which exposes channels +// to relay incoming and outgoing messages (see package module/dkg). +// +// EpochSetup/EpochCommit/OnView +// | +// v +// +---------------+ +// | ReactorEngine | +// +---------------+ +// | +// v +// *~~~~~~~~~~~~~~~~~~~~~* <- Epoch-scoped +// | +---------------+ | +// | | Controller | | +// | +---------------+ | +// | | | +// | v | +// | +---------------+ | +// | | Broker | | +// | +---------------+ | +// *~~~~~~~~|~~~~~~~~~\~~* +// | \ +// BrokerTunnel DKGContractClient +// | \ +// +--------------+ +------------------+ +// | Messaging | | FlowDKG smart | +// | Engine | | contract | +// +--------------+ +------------------+ package dkg From 12d132010bb82d92cbfe3d9be3cbadf42317eb90 Mon Sep 17 00:00:00 2001 From: Jordan Schalm Date: Mon, 25 Nov 2024 09:05:28 -0800 Subject: [PATCH 23/35] dkg package documentation tweaks --- engine/consensus/dkg/doc.go | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/engine/consensus/dkg/doc.go b/engine/consensus/dkg/doc.go index af9393534cf..2feff12aa42 100644 --- a/engine/consensus/dkg/doc.go +++ b/engine/consensus/dkg/doc.go @@ -2,12 +2,13 @@ // // # Reactor Engine // -// The [ReactorEngine] implements triggers to control the lifecycle of DKG runs. A new -// DKG protocol is started when an EpochSetup event is sealed and finalized. The -// subsequent phase transitions are triggered when specified views are encountered -// (specifically when the first block of a given view is finalized). In between -// phase transitions the engine regularly queries the DKG smart-contract to read -// broadcast messages. +// The [ReactorEngine] implements triggers to control the lifecycle of DKG instances. +// A new DKG instance is started when an EpochSetup service event is sealed. +// The subsequent phase transitions are triggered when specified views are encountered. +// Specifically, phase transitions for a view V are triggered when the first block with view >=V is finalized. +// Between phase transitions, we periodically query the DKG smart-contract ("whiteboard") to read broadcast messages. +// Before transitioning the state machine to the next phase, we query the whiteboard w.r.t. the final view +// of the phase - this ensures all participants eventually observe the same set of messages for each phase. // // # Messaging Engine // @@ -16,12 +17,12 @@ // // # Architecture // -// For every new epoch, the [ReactorEngine] instantiates a new [module.DKGController] with a -// new [module.DKGBroker] using the provided ControllerFactory. The ControllerFactory ties new -// DKGControllers to the [MessagingEngine] via a BrokerTunnel which exposes channels -// to relay incoming and outgoing messages (see package module/dkg). +// In the happy path, one DKG instance runs every epoch. For each DKG instance, the [ReactorEngine] +// instantiates a new, epoch-scoped [module.DKGController] and [module.DKGBroker] using the provided ControllerFactory. +// The ControllerFactory ties new DKGControllers to the [MessagingEngine] via a BrokerTunnel, +// which exposes channels to relay incoming and outgoing messages (see package module/dkg for details). // -// EpochSetup/EpochCommit/OnView +// EpochSetup/EpochCommit/OnView events // | // v // +---------------+ @@ -29,7 +30,7 @@ // +---------------+ // | // v -// *~~~~~~~~~~~~~~~~~~~~~* <- Epoch-scoped +// *~~~~~~~~~~~~~~~~~~~~~* <- Epoch-scoped components // | +---------------+ | // | | Controller | | // | +---------------+ | From 558155049a830b31cf5700a158ba878abc346c1a Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 26 Nov 2024 18:03:14 +0200 Subject: [PATCH 24/35] Moved Run to separate goroutine, paralleled test as suggested --- engine/access/rest/websockets/controller.go | 10 ++++++++-- .../rest/websockets/data_providers/factory_test.go | 5 +++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/engine/access/rest/websockets/controller.go b/engine/access/rest/websockets/controller.go index 7c77558e7c2..cbcdc55be0f 100644 --- a/engine/access/rest/websockets/controller.go +++ b/engine/access/rest/websockets/controller.go @@ -164,11 +164,17 @@ func (c *Controller) handleSubscribe(ctx context.Context, msg models.SubscribeMe dp, _ := c.dataProviderFactory.NewDataProvider(ctx, msg.Topic, msg.Arguments, c.communicationChannel) // TODO: handle error here c.dataProviders.Add(dp.ID(), dp) - _ = dp.Run() - // TODO: handle error here //TODO: return OK response to client c.communicationChannel <- msg + + go func() { + err := dp.Run() + if err != nil { + // Log or handle the error from Run + c.logger.Error().Err(err).Msgf("error while running data provider for topic: %s", msg.Topic) + } + }() } func (c *Controller) handleUnsubscribe(_ context.Context, msg models.UnsubscribeMessageRequest) { diff --git a/engine/access/rest/websockets/data_providers/factory_test.go b/engine/access/rest/websockets/data_providers/factory_test.go index ce4b16e97f6..602212e08f5 100644 --- a/engine/access/rest/websockets/data_providers/factory_test.go +++ b/engine/access/rest/websockets/data_providers/factory_test.go @@ -57,6 +57,8 @@ func (s *DataProviderFactorySuite) setupSubscription(apiCall *mock.Call) { // TestSupportedTopics verifies that supported topics return a valid provider and no errors. // Each test case includes a topic and arguments for which a data provider should be created. func (s *DataProviderFactorySuite) TestSupportedTopics() { + s.T().Parallel() + // Define supported topics and check if each returns the correct provider without errors testCases := []struct { name string @@ -102,6 +104,7 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { for _, test := range testCases { s.Run(test.name, func() { + s.T().Parallel() test.setupSubscription() provider, err := s.factory.NewDataProvider(s.ctx, test.topic, test.arguments, s.ch) @@ -117,6 +120,8 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { // TestUnsupportedTopics verifies that unsupported topics do not return a provider // and instead return an error indicating the topic is unsupported. func (s *DataProviderFactorySuite) TestUnsupportedTopics() { + s.T().Parallel() + // Define unsupported topics unsupportedTopics := []string{ "unknown_topic", From c5a9f17c216247068050820f927f38c479ee4d69 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Tue, 26 Nov 2024 18:47:58 +0200 Subject: [PATCH 25/35] Added names to interface arguments, created Arguments type for websockets --- .../data_providers/base_provider.go | 5 +++-- .../data_providers/block_digests_provider.go | 2 +- .../data_providers/block_headers_provider.go | 2 +- .../data_providers/blocks_provider.go | 4 ++-- .../data_providers/blocks_provider_test.go | 9 +++++---- .../rest/websockets/data_providers/factory.go | 5 +++-- .../websockets/data_providers/factory_test.go | 9 +++++---- .../data_providers/mock/data_provider.go | 17 ++++++++++++++-- .../mock/data_provider_factory.go | 20 ++++++++++--------- .../rest/websockets/models/subscribe.go | 6 ++++-- 10 files changed, 50 insertions(+), 29 deletions(-) diff --git a/engine/access/rest/websockets/data_providers/base_provider.go b/engine/access/rest/websockets/data_providers/base_provider.go index 7c5e4ccf548..2d81c3bf25d 100644 --- a/engine/access/rest/websockets/data_providers/base_provider.go +++ b/engine/access/rest/websockets/data_providers/base_provider.go @@ -16,7 +16,7 @@ type BaseDataProvider interface { // Topic returns the topic associated with the data provider. Topic() string // Close terminates the data provider. - Close() + Close() error } var _ BaseDataProvider = (*BaseDataProviderImpl)(nil) @@ -58,6 +58,7 @@ func (b *BaseDataProviderImpl) Topic() string { } // Close terminates the data provider. -func (b *BaseDataProviderImpl) Close() { +func (b *BaseDataProviderImpl) Close() error { b.cancel() + return nil } diff --git a/engine/access/rest/websockets/data_providers/block_digests_provider.go b/engine/access/rest/websockets/data_providers/block_digests_provider.go index 1d99f1a7a8d..e94ff1f2c4a 100644 --- a/engine/access/rest/websockets/data_providers/block_digests_provider.go +++ b/engine/access/rest/websockets/data_providers/block_digests_provider.go @@ -30,7 +30,7 @@ func NewBlockDigestsDataProvider( logger zerolog.Logger, api access.API, topic string, - arguments map[string]string, + arguments models.Arguments, send chan<- interface{}, ) (*BlockDigestsDataProvider, error) { p := &BlockDigestsDataProvider{ diff --git a/engine/access/rest/websockets/data_providers/block_headers_provider.go b/engine/access/rest/websockets/data_providers/block_headers_provider.go index 40bdb2ddd5c..42ede67703f 100644 --- a/engine/access/rest/websockets/data_providers/block_headers_provider.go +++ b/engine/access/rest/websockets/data_providers/block_headers_provider.go @@ -30,7 +30,7 @@ func NewBlockHeadersDataProvider( logger zerolog.Logger, api access.API, topic string, - arguments map[string]string, + arguments models.Arguments, send chan<- interface{}, ) (*BlockHeadersDataProvider, error) { p := &BlockHeadersDataProvider{ diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index f340c27ba54..1da9c58f4c8 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -39,7 +39,7 @@ func NewBlocksDataProvider( logger zerolog.Logger, api access.API, topic string, - arguments map[string]string, + arguments models.Arguments, send chan<- interface{}, ) (*BlocksDataProvider, error) { p := &BlocksDataProvider{ @@ -100,7 +100,7 @@ func (p *BlocksDataProvider) handleResponse(send chan<- interface{}) func(*flow. } // ParseBlocksArguments validates and initializes the blocks arguments. -func ParseBlocksArguments(arguments map[string]string) (BlocksArguments, error) { +func ParseBlocksArguments(arguments models.Arguments) (BlocksArguments, error) { var args BlocksArguments // Parse 'block_status' diff --git a/engine/access/rest/websockets/data_providers/blocks_provider_test.go b/engine/access/rest/websockets/data_providers/blocks_provider_test.go index 9771cb54780..8d3e984bd71 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider_test.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider_test.go @@ -10,6 +10,7 @@ import ( accessmock "github.com/onflow/flow-go/access/mock" "github.com/onflow/flow-go/engine/access/rest/common/parser" + "github.com/onflow/flow-go/engine/access/rest/websockets/models" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/utils/unittest" ) @@ -18,7 +19,7 @@ const unknownBlockStatus = "unknown_block_status" type testErrType struct { name string - arguments map[string]string + arguments models.Arguments expectedErrorMsg string } @@ -69,21 +70,21 @@ func (s *BlocksProviderSuite) invalidArgumentsTestCases() []testErrType { return []testErrType{ { name: "missing 'block_status' argument", - arguments: map[string]string{ + arguments: models.Arguments{ "start_block_id": s.rootBlock.ID().String(), }, expectedErrorMsg: "'block_status' must be provided", }, { name: "unknown 'block_status' argument", - arguments: map[string]string{ + arguments: models.Arguments{ "block_status": unknownBlockStatus, }, expectedErrorMsg: fmt.Sprintf("invalid 'block_status', must be '%s' or '%s'", parser.Finalized, parser.Sealed), }, { name: "provide both 'start_block_id' and 'start_block_height' arguments", - arguments: map[string]string{ + arguments: models.Arguments{ "block_status": parser.Finalized, "start_block_id": s.rootBlock.ID().String(), "start_block_height": fmt.Sprintf("%d", s.rootBlock.Header.Height), diff --git a/engine/access/rest/websockets/data_providers/factory.go b/engine/access/rest/websockets/data_providers/factory.go index c9c187a0117..51bf67b900d 100644 --- a/engine/access/rest/websockets/data_providers/factory.go +++ b/engine/access/rest/websockets/data_providers/factory.go @@ -7,6 +7,7 @@ import ( "github.com/rs/zerolog" "github.com/onflow/flow-go/access" + "github.com/onflow/flow-go/engine/access/rest/websockets/models" "github.com/onflow/flow-go/engine/access/state_stream" ) @@ -29,7 +30,7 @@ type DataProviderFactory interface { // and configuration parameters. // // No errors are expected during normal operations. - NewDataProvider(context.Context, string, map[string]string, chan<- interface{}) (DataProvider, error) + NewDataProvider(ctx context.Context, topic string, arguments models.Arguments, ch chan<- interface{}) (DataProvider, error) } var _ DataProviderFactory = (*DataProviderFactoryImpl)(nil) @@ -76,7 +77,7 @@ func NewDataProviderFactory( func (s *DataProviderFactoryImpl) NewDataProvider( ctx context.Context, topic string, - arguments map[string]string, + arguments models.Arguments, ch chan<- interface{}, ) (DataProvider, error) { switch topic { diff --git a/engine/access/rest/websockets/data_providers/factory_test.go b/engine/access/rest/websockets/data_providers/factory_test.go index 602212e08f5..1b33d892573 100644 --- a/engine/access/rest/websockets/data_providers/factory_test.go +++ b/engine/access/rest/websockets/data_providers/factory_test.go @@ -10,6 +10,7 @@ import ( accessmock "github.com/onflow/flow-go/access/mock" "github.com/onflow/flow-go/engine/access/rest/common/parser" + "github.com/onflow/flow-go/engine/access/rest/websockets/models" statestreammock "github.com/onflow/flow-go/engine/access/state_stream/mock" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/utils/unittest" @@ -63,14 +64,14 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { testCases := []struct { name string topic string - arguments map[string]string + arguments models.Arguments setupSubscription func() assertExpectations func() }{ { name: "block topic", topic: BlocksTopic, - arguments: map[string]string{"block_status": parser.Finalized}, + arguments: models.Arguments{"block_status": parser.Finalized}, setupSubscription: func() { s.setupSubscription(s.accessApi.On("SubscribeBlocksFromLatest", mock.Anything, flow.BlockStatusFinalized)) }, @@ -81,7 +82,7 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { { name: "block headers topic", topic: BlockHeadersTopic, - arguments: map[string]string{"block_status": parser.Finalized}, + arguments: models.Arguments{"block_status": parser.Finalized}, setupSubscription: func() { s.setupSubscription(s.accessApi.On("SubscribeBlockHeadersFromLatest", mock.Anything, flow.BlockStatusFinalized)) }, @@ -92,7 +93,7 @@ func (s *DataProviderFactorySuite) TestSupportedTopics() { { name: "block digests topic", topic: BlockDigestsTopic, - arguments: map[string]string{"block_status": parser.Finalized}, + arguments: models.Arguments{"block_status": parser.Finalized}, setupSubscription: func() { s.setupSubscription(s.accessApi.On("SubscribeBlockDigestsFromLatest", mock.Anything, flow.BlockStatusFinalized)) }, diff --git a/engine/access/rest/websockets/data_providers/mock/data_provider.go b/engine/access/rest/websockets/data_providers/mock/data_provider.go index 48debb23ae3..3fe8bc5d15b 100644 --- a/engine/access/rest/websockets/data_providers/mock/data_provider.go +++ b/engine/access/rest/websockets/data_providers/mock/data_provider.go @@ -13,8 +13,21 @@ type DataProvider struct { } // Close provides a mock function with given fields: -func (_m *DataProvider) Close() { - _m.Called() +func (_m *DataProvider) Close() error { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for Close") + } + + var r0 error + if rf, ok := ret.Get(0).(func() error); ok { + r0 = rf() + } else { + r0 = ret.Error(0) + } + + return r0 } // ID provides a mock function with given fields: diff --git a/engine/access/rest/websockets/data_providers/mock/data_provider_factory.go b/engine/access/rest/websockets/data_providers/mock/data_provider_factory.go index 2a959715511..c2e46e58d1d 100644 --- a/engine/access/rest/websockets/data_providers/mock/data_provider_factory.go +++ b/engine/access/rest/websockets/data_providers/mock/data_provider_factory.go @@ -7,6 +7,8 @@ import ( data_providers "github.com/onflow/flow-go/engine/access/rest/websockets/data_providers" mock "github.com/stretchr/testify/mock" + + models "github.com/onflow/flow-go/engine/access/rest/websockets/models" ) // DataProviderFactory is an autogenerated mock type for the DataProviderFactory type @@ -14,9 +16,9 @@ type DataProviderFactory struct { mock.Mock } -// NewDataProvider provides a mock function with given fields: _a0, _a1, _a2, _a3 -func (_m *DataProviderFactory) NewDataProvider(_a0 context.Context, _a1 string, _a2 map[string]string, _a3 chan<- interface{}) (data_providers.DataProvider, error) { - ret := _m.Called(_a0, _a1, _a2, _a3) +// NewDataProvider provides a mock function with given fields: ctx, topic, arguments, ch +func (_m *DataProviderFactory) NewDataProvider(ctx context.Context, topic string, arguments models.Arguments, ch chan<- interface{}) (data_providers.DataProvider, error) { + ret := _m.Called(ctx, topic, arguments, ch) if len(ret) == 0 { panic("no return value specified for NewDataProvider") @@ -24,19 +26,19 @@ func (_m *DataProviderFactory) NewDataProvider(_a0 context.Context, _a1 string, var r0 data_providers.DataProvider var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, map[string]string, chan<- interface{}) (data_providers.DataProvider, error)); ok { - return rf(_a0, _a1, _a2, _a3) + if rf, ok := ret.Get(0).(func(context.Context, string, models.Arguments, chan<- interface{}) (data_providers.DataProvider, error)); ok { + return rf(ctx, topic, arguments, ch) } - if rf, ok := ret.Get(0).(func(context.Context, string, map[string]string, chan<- interface{}) data_providers.DataProvider); ok { - r0 = rf(_a0, _a1, _a2, _a3) + if rf, ok := ret.Get(0).(func(context.Context, string, models.Arguments, chan<- interface{}) data_providers.DataProvider); ok { + r0 = rf(ctx, topic, arguments, ch) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(data_providers.DataProvider) } } - if rf, ok := ret.Get(1).(func(context.Context, string, map[string]string, chan<- interface{}) error); ok { - r1 = rf(_a0, _a1, _a2, _a3) + if rf, ok := ret.Get(1).(func(context.Context, string, models.Arguments, chan<- interface{}) error); ok { + r1 = rf(ctx, topic, arguments, ch) } else { r1 = ret.Error(1) } diff --git a/engine/access/rest/websockets/models/subscribe.go b/engine/access/rest/websockets/models/subscribe.go index d2cd007bd3c..95ad17e3708 100644 --- a/engine/access/rest/websockets/models/subscribe.go +++ b/engine/access/rest/websockets/models/subscribe.go @@ -1,10 +1,12 @@ package models +type Arguments map[string]string + // SubscribeMessageRequest represents a request to subscribe to a topic. type SubscribeMessageRequest struct { BaseMessageRequest - Topic string `json:"topic"` // Topic to subscribe to - Arguments map[string]string `json:"arguments"` // Additional arguments for subscription + Topic string `json:"topic"` // Topic to subscribe to + Arguments Arguments `json:"arguments"` // Additional arguments for subscription } // SubscribeMessageResponse represents the response to a subscription request. From 3cb67c424d6a609f21d85979677ce819f4b99240 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Wed, 27 Nov 2024 12:53:32 +0200 Subject: [PATCH 26/35] Updated ParseBlocksArguments for block data provider --- .../data_providers/blocks_provider.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index 1da9c58f4c8..1c3d454b4c5 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -114,8 +114,16 @@ func ParseBlocksArguments(arguments models.Arguments) (BlocksArguments, error) { return args, fmt.Errorf("'block_status' must be provided") } + startBlockIDIn, hasStartBlockID := arguments["start_block_id"] + startBlockHeightIn, hasStartBlockHeight := arguments["start_block_height"] + + // Ensure only one of start_block_id or start_block_height is provided + if hasStartBlockID && hasStartBlockHeight { + return args, fmt.Errorf("can only provide either 'start_block_id' or 'start_block_height'") + } + // Parse 'start_block_id' if provided - if startBlockIDIn, ok := arguments["start_block_id"]; ok { + if hasStartBlockID { var startBlockID parser.ID err := startBlockID.Parse(startBlockIDIn) if err != nil { @@ -125,7 +133,7 @@ func ParseBlocksArguments(arguments models.Arguments) (BlocksArguments, error) { } // Parse 'start_block_height' if provided - if startBlockHeightIn, ok := arguments["start_block_height"]; ok { + if hasStartBlockHeight { var err error args.StartBlockHeight, err = util.ToUint64(startBlockHeightIn) if err != nil { @@ -135,10 +143,5 @@ func ParseBlocksArguments(arguments models.Arguments) (BlocksArguments, error) { args.StartBlockHeight = request.EmptyHeight } - // Ensure only one of start_block_id or start_block_height is provided - if args.StartBlockID != flow.ZeroID && args.StartBlockHeight != request.EmptyHeight { - return args, fmt.Errorf("can only provide either 'start_block_id' or 'start_block_height'") - } - return args, nil } From b9abf683c1324f9141b06bbac3f61d5b729bccba Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Fri, 29 Nov 2024 13:34:13 +0200 Subject: [PATCH 27/35] Updated comments and messages --- engine/access/rest/websockets/controller.go | 9 ++++++--- engine/access/rest/websockets/data_providers/factory.go | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/engine/access/rest/websockets/controller.go b/engine/access/rest/websockets/controller.go index cbcdc55be0f..df5d48b345c 100644 --- a/engine/access/rest/websockets/controller.go +++ b/engine/access/rest/websockets/controller.go @@ -161,8 +161,11 @@ func (c *Controller) handleAction(ctx context.Context, message interface{}) erro } func (c *Controller) handleSubscribe(ctx context.Context, msg models.SubscribeMessageRequest) { - dp, _ := c.dataProviderFactory.NewDataProvider(ctx, msg.Topic, msg.Arguments, c.communicationChannel) - // TODO: handle error here + dp, err := c.dataProviderFactory.NewDataProvider(ctx, msg.Topic, msg.Arguments, c.communicationChannel) + if err != nil { + // TODO: handle error here + } + c.dataProviders.Add(dp.ID(), dp) //TODO: return OK response to client @@ -171,7 +174,7 @@ func (c *Controller) handleSubscribe(ctx context.Context, msg models.SubscribeMe go func() { err := dp.Run() if err != nil { - // Log or handle the error from Run + //TODO: Log or handle the error from Run c.logger.Error().Err(err).Msgf("error while running data provider for topic: %s", msg.Topic) } }() diff --git a/engine/access/rest/websockets/data_providers/factory.go b/engine/access/rest/websockets/data_providers/factory.go index 51bf67b900d..72f4a6b7633 100644 --- a/engine/access/rest/websockets/data_providers/factory.go +++ b/engine/access/rest/websockets/data_providers/factory.go @@ -91,7 +91,7 @@ func (s *DataProviderFactoryImpl) NewDataProvider( case EventsTopic, AccountStatusesTopic, TransactionStatusesTopic: - return nil, fmt.Errorf("topic \"%s\" not implemented yet", topic) + return nil, fmt.Errorf(`topic "%s" not implemented yet`, topic) default: return nil, fmt.Errorf("unsupported topic \"%s\"", topic) } From 1a2698f62208b855cf7a87105f8ade5cd9038b6f Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Fri, 29 Nov 2024 13:45:12 +0200 Subject: [PATCH 28/35] Removed storing arguments in the data providers --- engine/access/rest/websockets/controller.go | 1 + .../data_providers/block_digests_provider.go | 23 ++++++++----------- .../data_providers/block_headers_provider.go | 23 ++++++++----------- .../data_providers/blocks_provider.go | 23 ++++++++----------- 4 files changed, 31 insertions(+), 39 deletions(-) diff --git a/engine/access/rest/websockets/controller.go b/engine/access/rest/websockets/controller.go index df5d48b345c..38bc7306b55 100644 --- a/engine/access/rest/websockets/controller.go +++ b/engine/access/rest/websockets/controller.go @@ -164,6 +164,7 @@ func (c *Controller) handleSubscribe(ctx context.Context, msg models.SubscribeMe dp, err := c.dataProviderFactory.NewDataProvider(ctx, msg.Topic, msg.Arguments, c.communicationChannel) if err != nil { // TODO: handle error here + c.logger.Error().Err(err).Msgf("error while creating data provider for topic: %s", msg.Topic) } c.dataProviders.Add(dp.ID(), dp) diff --git a/engine/access/rest/websockets/data_providers/block_digests_provider.go b/engine/access/rest/websockets/data_providers/block_digests_provider.go index e94ff1f2c4a..3352e0c0ac0 100644 --- a/engine/access/rest/websockets/data_providers/block_digests_provider.go +++ b/engine/access/rest/websockets/data_providers/block_digests_provider.go @@ -18,7 +18,6 @@ type BlockDigestsDataProvider struct { *BaseDataProviderImpl logger zerolog.Logger - args BlocksArguments api access.API } @@ -38,20 +37,18 @@ func NewBlockDigestsDataProvider( api: api, } - // Initialize arguments passed to the provider. - var err error - p.args, err = ParseBlocksArguments(arguments) + // Parse arguments passed to the provider. + blockArgs, err := ParseBlocksArguments(arguments) if err != nil { return nil, fmt.Errorf("invalid arguments: %w", err) } - ctx, cancel := context.WithCancel(ctx) - + subCtx, cancel := context.WithCancel(ctx) p.BaseDataProviderImpl = NewBaseDataProviderImpl( topic, cancel, send, - p.createSubscription(ctx), // Set up a subscription to block digests based on arguments. + p.createSubscription(subCtx, blockArgs), // Set up a subscription to block digests based on arguments. ) return p, nil @@ -65,16 +62,16 @@ func (p *BlockDigestsDataProvider) Run() error { } // createSubscription creates a new subscription using the specified input arguments. -func (p *BlockDigestsDataProvider) createSubscription(ctx context.Context) subscription.Subscription { - if p.args.StartBlockID != flow.ZeroID { - return p.api.SubscribeBlockDigestsFromStartBlockID(ctx, p.args.StartBlockID, p.args.BlockStatus) +func (p *BlockDigestsDataProvider) createSubscription(ctx context.Context, args BlocksArguments) subscription.Subscription { + if args.StartBlockID != flow.ZeroID { + return p.api.SubscribeBlockDigestsFromStartBlockID(ctx, args.StartBlockID, args.BlockStatus) } - if p.args.StartBlockHeight != request.EmptyHeight { - return p.api.SubscribeBlockDigestsFromStartHeight(ctx, p.args.StartBlockHeight, p.args.BlockStatus) + if args.StartBlockHeight != request.EmptyHeight { + return p.api.SubscribeBlockDigestsFromStartHeight(ctx, args.StartBlockHeight, args.BlockStatus) } - return p.api.SubscribeBlockDigestsFromLatest(ctx, p.args.BlockStatus) + return p.api.SubscribeBlockDigestsFromLatest(ctx, args.BlockStatus) } // handleResponse processes a block digest and sends the formatted response. diff --git a/engine/access/rest/websockets/data_providers/block_headers_provider.go b/engine/access/rest/websockets/data_providers/block_headers_provider.go index 42ede67703f..142b98f9e70 100644 --- a/engine/access/rest/websockets/data_providers/block_headers_provider.go +++ b/engine/access/rest/websockets/data_providers/block_headers_provider.go @@ -18,7 +18,6 @@ type BlockHeadersDataProvider struct { *BaseDataProviderImpl logger zerolog.Logger - args BlocksArguments api access.API } @@ -38,20 +37,18 @@ func NewBlockHeadersDataProvider( api: api, } - // Initialize arguments passed to the provider. - var err error - p.args, err = ParseBlocksArguments(arguments) + // Parse arguments passed to the provider. + blockArgs, err := ParseBlocksArguments(arguments) if err != nil { return nil, fmt.Errorf("invalid arguments: %w", err) } - ctx, cancel := context.WithCancel(ctx) - + subCtx, cancel := context.WithCancel(ctx) p.BaseDataProviderImpl = NewBaseDataProviderImpl( topic, cancel, send, - p.createSubscription(ctx), // Set up a subscription to block headers based on arguments. + p.createSubscription(subCtx, blockArgs), // Set up a subscription to block headers based on arguments. ) return p, nil @@ -65,16 +62,16 @@ func (p *BlockHeadersDataProvider) Run() error { } // createSubscription creates a new subscription using the specified input arguments. -func (p *BlockHeadersDataProvider) createSubscription(ctx context.Context) subscription.Subscription { - if p.args.StartBlockID != flow.ZeroID { - return p.api.SubscribeBlockHeadersFromStartBlockID(ctx, p.args.StartBlockID, p.args.BlockStatus) +func (p *BlockHeadersDataProvider) createSubscription(ctx context.Context, args BlocksArguments) subscription.Subscription { + if args.StartBlockID != flow.ZeroID { + return p.api.SubscribeBlockHeadersFromStartBlockID(ctx, args.StartBlockID, args.BlockStatus) } - if p.args.StartBlockHeight != request.EmptyHeight { - return p.api.SubscribeBlockHeadersFromStartHeight(ctx, p.args.StartBlockHeight, p.args.BlockStatus) + if args.StartBlockHeight != request.EmptyHeight { + return p.api.SubscribeBlockHeadersFromStartHeight(ctx, args.StartBlockHeight, args.BlockStatus) } - return p.api.SubscribeBlockHeadersFromLatest(ctx, p.args.BlockStatus) + return p.api.SubscribeBlockHeadersFromLatest(ctx, args.BlockStatus) } // handleResponse processes a block header and sends the formatted response. diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index 1c3d454b4c5..cccf8b5bed3 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -27,7 +27,6 @@ type BlocksDataProvider struct { *BaseDataProviderImpl logger zerolog.Logger - args BlocksArguments api access.API } @@ -47,20 +46,18 @@ func NewBlocksDataProvider( api: api, } - // Initialize arguments passed to the provider. - var err error - p.args, err = ParseBlocksArguments(arguments) + // Parse arguments passed to the provider. + blockArgs, err := ParseBlocksArguments(arguments) if err != nil { return nil, fmt.Errorf("invalid arguments: %w", err) } - ctx, cancel := context.WithCancel(ctx) - + subCtx, cancel := context.WithCancel(ctx) p.BaseDataProviderImpl = NewBaseDataProviderImpl( topic, cancel, send, - p.createSubscription(ctx), // Set up a subscription to blocks based on arguments. + p.createSubscription(subCtx, blockArgs), // Set up a subscription to blocks based on arguments. ) return p, nil @@ -74,16 +71,16 @@ func (p *BlocksDataProvider) Run() error { } // createSubscription creates a new subscription using the specified input arguments. -func (p *BlocksDataProvider) createSubscription(ctx context.Context) subscription.Subscription { - if p.args.StartBlockID != flow.ZeroID { - return p.api.SubscribeBlocksFromStartBlockID(ctx, p.args.StartBlockID, p.args.BlockStatus) +func (p *BlocksDataProvider) createSubscription(ctx context.Context, args BlocksArguments) subscription.Subscription { + if args.StartBlockID != flow.ZeroID { + return p.api.SubscribeBlocksFromStartBlockID(ctx, args.StartBlockID, args.BlockStatus) } - if p.args.StartBlockHeight != request.EmptyHeight { - return p.api.SubscribeBlocksFromStartHeight(ctx, p.args.StartBlockHeight, p.args.BlockStatus) + if args.StartBlockHeight != request.EmptyHeight { + return p.api.SubscribeBlocksFromStartHeight(ctx, args.StartBlockHeight, args.BlockStatus) } - return p.api.SubscribeBlocksFromLatest(ctx, p.args.BlockStatus) + return p.api.SubscribeBlocksFromLatest(ctx, args.BlockStatus) } // handleResponse processes a block and sends the formatted response. From ff3851b346f1c9a5e638dd1f54ae1a5506c8e0bd Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Fri, 29 Nov 2024 15:07:41 +0200 Subject: [PATCH 29/35] Refactored base data provider according to comments --- .../data_providers/base_provider.go | 34 ++++++------------- .../data_providers/block_digests_provider.go | 4 +-- .../data_providers/block_headers_provider.go | 4 +-- .../data_providers/blocks_provider.go | 4 +-- .../data_providers/data_provider.go | 15 ++++++-- 5 files changed, 30 insertions(+), 31 deletions(-) diff --git a/engine/access/rest/websockets/data_providers/base_provider.go b/engine/access/rest/websockets/data_providers/base_provider.go index 2d81c3bf25d..cf1ee1313d9 100644 --- a/engine/access/rest/websockets/data_providers/base_provider.go +++ b/engine/access/rest/websockets/data_providers/base_provider.go @@ -8,22 +8,8 @@ import ( "github.com/onflow/flow-go/engine/access/subscription" ) -// BaseDataProvider defines the basic interface for a data provider. It provides methods -// for retrieving the provider's unique ID, topic, and a method to close the provider. -type BaseDataProvider interface { - // ID returns the unique identifier of the data provider. - ID() uuid.UUID - // Topic returns the topic associated with the data provider. - Topic() string - // Close terminates the data provider. - Close() error -} - -var _ BaseDataProvider = (*BaseDataProviderImpl)(nil) - -// BaseDataProviderImpl is the concrete implementation of the BaseDataProvider interface. -// It holds common objects for the provider. -type BaseDataProviderImpl struct { +// baseDataProvider holds common objects for the provider +type baseDataProvider struct { id uuid.UUID topic string cancel context.CancelFunc @@ -31,14 +17,14 @@ type BaseDataProviderImpl struct { subscription subscription.Subscription } -// NewBaseDataProviderImpl creates a new instance of BaseDataProviderImpl. -func NewBaseDataProviderImpl( +// newBaseDataProvider creates a new instance of baseDataProvider. +func newBaseDataProvider( topic string, cancel context.CancelFunc, send chan<- interface{}, subscription subscription.Subscription, -) *BaseDataProviderImpl { - return &BaseDataProviderImpl{ +) *baseDataProvider { + return &baseDataProvider{ id: uuid.New(), topic: topic, cancel: cancel, @@ -48,17 +34,19 @@ func NewBaseDataProviderImpl( } // ID returns the unique identifier of the data provider. -func (b *BaseDataProviderImpl) ID() uuid.UUID { +func (b *baseDataProvider) ID() uuid.UUID { return b.id } // Topic returns the topic associated with the data provider. -func (b *BaseDataProviderImpl) Topic() string { +func (b *baseDataProvider) Topic() string { return b.topic } // Close terminates the data provider. -func (b *BaseDataProviderImpl) Close() error { +// +// No errors are expected during normal operations. +func (b *baseDataProvider) Close() error { b.cancel() return nil } diff --git a/engine/access/rest/websockets/data_providers/block_digests_provider.go b/engine/access/rest/websockets/data_providers/block_digests_provider.go index 3352e0c0ac0..b2f5d3496b3 100644 --- a/engine/access/rest/websockets/data_providers/block_digests_provider.go +++ b/engine/access/rest/websockets/data_providers/block_digests_provider.go @@ -15,7 +15,7 @@ import ( // BlockDigestsDataProvider is responsible for providing block digests type BlockDigestsDataProvider struct { - *BaseDataProviderImpl + *baseDataProvider logger zerolog.Logger api access.API @@ -44,7 +44,7 @@ func NewBlockDigestsDataProvider( } subCtx, cancel := context.WithCancel(ctx) - p.BaseDataProviderImpl = NewBaseDataProviderImpl( + p.baseDataProvider = newBaseDataProvider( topic, cancel, send, diff --git a/engine/access/rest/websockets/data_providers/block_headers_provider.go b/engine/access/rest/websockets/data_providers/block_headers_provider.go index 142b98f9e70..7cd91fa4a38 100644 --- a/engine/access/rest/websockets/data_providers/block_headers_provider.go +++ b/engine/access/rest/websockets/data_providers/block_headers_provider.go @@ -15,7 +15,7 @@ import ( // BlockHeadersDataProvider is responsible for providing block headers type BlockHeadersDataProvider struct { - *BaseDataProviderImpl + *baseDataProvider logger zerolog.Logger api access.API @@ -44,7 +44,7 @@ func NewBlockHeadersDataProvider( } subCtx, cancel := context.WithCancel(ctx) - p.BaseDataProviderImpl = NewBaseDataProviderImpl( + p.baseDataProvider = newBaseDataProvider( topic, cancel, send, diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index cccf8b5bed3..794cd8e63f8 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -24,7 +24,7 @@ type BlocksArguments struct { // BlocksDataProvider is responsible for providing blocks type BlocksDataProvider struct { - *BaseDataProviderImpl + *baseDataProvider logger zerolog.Logger api access.API @@ -53,7 +53,7 @@ func NewBlocksDataProvider( } subCtx, cancel := context.WithCancel(ctx) - p.BaseDataProviderImpl = NewBaseDataProviderImpl( + p.baseDataProvider = newBaseDataProvider( topic, cancel, send, diff --git a/engine/access/rest/websockets/data_providers/data_provider.go b/engine/access/rest/websockets/data_providers/data_provider.go index 6a3fcc8991b..9d1d8855f21 100644 --- a/engine/access/rest/websockets/data_providers/data_provider.go +++ b/engine/access/rest/websockets/data_providers/data_provider.go @@ -1,9 +1,20 @@ package data_providers +import ( + "github.com/google/uuid" +) + // The DataProvider is the interface abstracts of the actual data provider used by the WebSocketCollector. +// It provides methods for retrieving the provider's unique ID, topic, and a methods to close and run the provider. type DataProvider interface { - BaseDataProvider - + // ID returns the unique identifier of the data provider. + ID() uuid.UUID + // Topic returns the topic associated with the data provider. + Topic() string + // Close terminates the data provider. + // + // No errors are expected during normal operations. + Close() error // Run starts processing the subscription and handles responses. // // No errors are expected during normal operations. From f8d711f75a9219c44f4b9831f5c713a80c9d28bc Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Mon, 2 Dec 2024 14:53:31 +0200 Subject: [PATCH 30/35] Added happy case unit tests for providers --- .../block_digests_provider_test.go | 85 ++++++++- .../block_headers_provider_test.go | 83 ++++++++- .../data_providers/blocks_provider_test.go | 167 +++++++++++++++++- .../websockets/data_providers/factory_test.go | 2 - 4 files changed, 326 insertions(+), 11 deletions(-) diff --git a/engine/access/rest/websockets/data_providers/block_digests_provider_test.go b/engine/access/rest/websockets/data_providers/block_digests_provider_test.go index 57fdabd7994..476edf77111 100644 --- a/engine/access/rest/websockets/data_providers/block_digests_provider_test.go +++ b/engine/access/rest/websockets/data_providers/block_digests_provider_test.go @@ -2,9 +2,17 @@ package data_providers import ( "context" + "strconv" "testing" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + + "github.com/onflow/flow-go/engine/access/rest/common/parser" + "github.com/onflow/flow-go/engine/access/rest/websockets/models" + statestreamsmock "github.com/onflow/flow-go/engine/access/state_stream/mock" + "github.com/onflow/flow-go/model/flow" ) type BlockDigestsProviderSuite struct { @@ -43,4 +51,79 @@ func (s *BlockDigestsProviderSuite) TestBlockDigestsDataProvider_InvalidArgument } } -// TODO: add tests for responses after the WebsocketController is ready +// validBlockDigestsArgumentsTestCases defines test happy cases for block digests data providers. +// Each test case specifies input arguments, and setup functions for the mock API used in the test. +func (s *BlockDigestsProviderSuite) validBlockDigestsArgumentsTestCases() []testType { + return []testType{ + { + name: "happy path with start_block_id argument", + arguments: models.Arguments{ + "start_block_id": s.rootBlock.ID().String(), + "block_status": parser.Finalized, + }, + setupBackend: func(sub *statestreamsmock.Subscription) { + s.api.On( + "SubscribeBlockDigestsFromStartBlockID", + mock.Anything, + s.rootBlock.ID(), + flow.BlockStatusFinalized, + ).Return(sub).Once() + }, + }, + { + name: "happy path with start_block_height argument", + arguments: models.Arguments{ + "start_block_height": strconv.FormatUint(s.rootBlock.Header.Height, 10), + "block_status": parser.Finalized, + }, + setupBackend: func(sub *statestreamsmock.Subscription) { + s.api.On( + "SubscribeBlockDigestsFromStartHeight", + mock.Anything, + s.rootBlock.Header.Height, + flow.BlockStatusFinalized, + ).Return(sub).Once() + }, + }, + { + name: "happy path without any start argument", + arguments: models.Arguments{ + "block_status": parser.Finalized, + }, + setupBackend: func(sub *statestreamsmock.Subscription) { + s.api.On( + "SubscribeBlockDigestsFromLatest", + mock.Anything, + flow.BlockStatusFinalized, + ).Return(sub).Once() + }, + }, + } +} + +// TestBlockDigestsDataProvider_HappyPath tests the behavior of the block digests data provider +// when it is configured correctly and operating under normal conditions. It +// validates that block digests are correctly streamed to the channel and ensures +// no unexpected errors occur. +func (s *BlockDigestsProviderSuite) TestBlockDigestsDataProvider_HappyPath() { + s.testHappyPath( + BlockDigestsTopic, + s.validBlockDigestsArgumentsTestCases(), + func(dataChan chan interface{}, blocks []*flow.Block) { + for _, block := range blocks { + dataChan <- flow.NewBlockDigest(block.Header.ID(), block.Header.Height, block.Header.Timestamp) + } + }, + s.requireBlockDigests, + ) +} + +// requireBlockHeaders ensures that the received block header information matches the expected data. +func (s *BlocksProviderSuite) requireBlockDigests(v interface{}, expectedBlock *flow.Block) { + actualResponse, ok := v.(*models.BlockDigestMessageResponse) + require.True(s.T(), ok, "unexpected response type: %T", v) + + s.Require().Equal(expectedBlock.Header.ID(), actualResponse.Block.ID()) + s.Require().Equal(expectedBlock.Header.Height, actualResponse.Block.Height) + s.Require().Equal(expectedBlock.Header.Timestamp, actualResponse.Block.Timestamp) +} diff --git a/engine/access/rest/websockets/data_providers/block_headers_provider_test.go b/engine/access/rest/websockets/data_providers/block_headers_provider_test.go index efd94916e92..57c262d8795 100644 --- a/engine/access/rest/websockets/data_providers/block_headers_provider_test.go +++ b/engine/access/rest/websockets/data_providers/block_headers_provider_test.go @@ -2,9 +2,17 @@ package data_providers import ( "context" + "strconv" "testing" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + + "github.com/onflow/flow-go/engine/access/rest/common/parser" + "github.com/onflow/flow-go/engine/access/rest/websockets/models" + statestreamsmock "github.com/onflow/flow-go/engine/access/state_stream/mock" + "github.com/onflow/flow-go/model/flow" ) type BlockHeadersProviderSuite struct { @@ -43,4 +51,77 @@ func (s *BlockHeadersProviderSuite) TestBlockHeadersDataProvider_InvalidArgument } } -// TODO: add tests for responses after the WebsocketController is ready +// validBlockHeadersArgumentsTestCases defines test happy cases for block headers data providers. +// Each test case specifies input arguments, and setup functions for the mock API used in the test. +func (s *BlockHeadersProviderSuite) validBlockHeadersArgumentsTestCases() []testType { + return []testType{ + { + name: "happy path with start_block_id argument", + arguments: models.Arguments{ + "start_block_id": s.rootBlock.ID().String(), + "block_status": parser.Finalized, + }, + setupBackend: func(sub *statestreamsmock.Subscription) { + s.api.On( + "SubscribeBlockHeadersFromStartBlockID", + mock.Anything, + s.rootBlock.ID(), + flow.BlockStatusFinalized, + ).Return(sub).Once() + }, + }, + { + name: "happy path with start_block_height argument", + arguments: models.Arguments{ + "start_block_height": strconv.FormatUint(s.rootBlock.Header.Height, 10), + "block_status": parser.Finalized, + }, + setupBackend: func(sub *statestreamsmock.Subscription) { + s.api.On( + "SubscribeBlockHeadersFromStartHeight", + mock.Anything, + s.rootBlock.Header.Height, + flow.BlockStatusFinalized, + ).Return(sub).Once() + }, + }, + { + name: "happy path without any start argument", + arguments: models.Arguments{ + "block_status": parser.Finalized, + }, + setupBackend: func(sub *statestreamsmock.Subscription) { + s.api.On( + "SubscribeBlockHeadersFromLatest", + mock.Anything, + flow.BlockStatusFinalized, + ).Return(sub).Once() + }, + }, + } +} + +// TestBlockHeadersDataProvider_HappyPath tests the behavior of the block headers data provider +// when it is configured correctly and operating under normal conditions. It +// validates that block headers are correctly streamed to the channel and ensures +// no unexpected errors occur. +func (s *BlockHeadersProviderSuite) TestBlockHeadersDataProvider_HappyPath() { + s.testHappyPath( + BlockHeadersTopic, + s.validBlockHeadersArgumentsTestCases(), + func(dataChan chan interface{}, blocks []*flow.Block) { + for _, block := range blocks { + dataChan <- block.Header + } + }, + s.requireBlockHeaders, + ) +} + +// requireBlockHeaders ensures that the received block header information matches the expected data. +func (s *BlockHeadersProviderSuite) requireBlockHeaders(v interface{}, expectedBlock *flow.Block) { + actualResponse, ok := v.(*models.BlockHeaderMessageResponse) + require.True(s.T(), ok, "unexpected response type: %T", v) + + s.Require().Equal(expectedBlock.Header, actualResponse.Header) +} diff --git a/engine/access/rest/websockets/data_providers/blocks_provider_test.go b/engine/access/rest/websockets/data_providers/blocks_provider_test.go index 8d3e984bd71..83fe16bbae6 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider_test.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider_test.go @@ -3,14 +3,19 @@ package data_providers import ( "context" "fmt" + "strconv" "testing" + "time" "github.com/rs/zerolog" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" accessmock "github.com/onflow/flow-go/access/mock" "github.com/onflow/flow-go/engine/access/rest/common/parser" "github.com/onflow/flow-go/engine/access/rest/websockets/models" + statestreamsmock "github.com/onflow/flow-go/engine/access/state_stream/mock" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/utils/unittest" ) @@ -23,6 +28,13 @@ type testErrType struct { expectedErrorMsg string } +// testType represents a valid test scenario for subscribing +type testType struct { + name string + arguments models.Arguments + setupBackend func(sub *statestreamsmock.Subscription) +} + // BlocksProviderSuite is a test suite for testing the block providers functionality. type BlocksProviderSuite struct { suite.Suite @@ -30,9 +42,11 @@ type BlocksProviderSuite struct { log zerolog.Logger api *accessmock.API - blockMap map[uint64]*flow.Block + blocks []*flow.Block rootBlock flow.Block finalizedBlock *flow.Header + + factory *DataProviderFactoryImpl } func TestBlocksProviderSuite(t *testing.T) { @@ -44,7 +58,8 @@ func (s *BlocksProviderSuite) SetupTest() { s.api = accessmock.NewAPI(s.T()) blockCount := 5 - s.blockMap = make(map[uint64]*flow.Block, blockCount) + s.blocks = make([]*flow.Block, 0, blockCount) + s.rootBlock = unittest.BlockFixture() s.rootBlock.Header.Height = 0 parent := s.rootBlock.Header @@ -53,9 +68,13 @@ func (s *BlocksProviderSuite) SetupTest() { block := unittest.BlockWithParentFixture(parent) // update for next iteration parent = block.Header - s.blockMap[block.Header.Height] = block + s.blocks = append(s.blocks, block) + } s.finalizedBlock = parent + + s.factory = NewDataProviderFactory(s.log, nil, s.api) + s.Require().NotNil(s.factory) } // invalidArgumentsTestCases returns a list of test cases with invalid argument combinations @@ -105,11 +124,9 @@ func (s *BlocksProviderSuite) TestBlocksDataProvider_InvalidArguments() { ctx := context.Background() send := make(chan interface{}) - topic := BlocksTopic - for _, test := range s.invalidArgumentsTestCases() { s.Run(test.name, func() { - provider, err := NewBlocksDataProvider(ctx, s.log, s.api, topic, test.arguments, send) + provider, err := NewBlocksDataProvider(ctx, s.log, s.api, BlocksTopic, test.arguments, send) s.Require().Nil(provider) s.Require().Error(err) s.Require().Contains(err.Error(), test.expectedErrorMsg) @@ -117,4 +134,140 @@ func (s *BlocksProviderSuite) TestBlocksDataProvider_InvalidArguments() { } } -// TODO: add tests for responses after the WebsocketController is ready +// validBlockArgumentsTestCases defines test happy cases for block data providers. +// Each test case specifies input arguments, and setup functions for the mock API used in the test. +func (s *BlocksProviderSuite) validBlockArgumentsTestCases() []testType { + return []testType{ + { + name: "happy path with start_block_id argument", + arguments: models.Arguments{ + "start_block_id": s.rootBlock.ID().String(), + "block_status": parser.Finalized, + }, + setupBackend: func(sub *statestreamsmock.Subscription) { + s.api.On( + "SubscribeBlocksFromStartBlockID", + mock.Anything, + s.rootBlock.ID(), + flow.BlockStatusFinalized, + ).Return(sub).Once() + }, + }, + { + name: "happy path with start_block_height argument", + arguments: models.Arguments{ + "start_block_height": strconv.FormatUint(s.rootBlock.Header.Height, 10), + "block_status": parser.Finalized, + }, + setupBackend: func(sub *statestreamsmock.Subscription) { + s.api.On( + "SubscribeBlocksFromStartHeight", + mock.Anything, + s.rootBlock.Header.Height, + flow.BlockStatusFinalized, + ).Return(sub).Once() + }, + }, + { + name: "happy path without any start argument", + arguments: models.Arguments{ + "block_status": parser.Finalized, + }, + setupBackend: func(sub *statestreamsmock.Subscription) { + s.api.On( + "SubscribeBlocksFromLatest", + mock.Anything, + flow.BlockStatusFinalized, + ).Return(sub).Once() + }, + }, + } +} + +// TestBlocksDataProvider_HappyPath tests the behavior of the block data provider +// when it is configured correctly and operating under normal conditions. It +// validates that blocks are correctly streamed to the channel and ensures +// no unexpected errors occur. +func (s *BlocksProviderSuite) TestBlocksDataProvider_HappyPath() { + s.testHappyPath( + BlocksTopic, + s.validBlockArgumentsTestCases(), + func(dataChan chan interface{}, blocks []*flow.Block) { + for _, block := range blocks { + dataChan <- block + } + }, + s.requireBlock, + ) +} + +// requireBlocks ensures that the received block information matches the expected data. +func (s *BlocksProviderSuite) requireBlock(v interface{}, expectedBlock *flow.Block) { + actualResponse, ok := v.(*models.BlockMessageResponse) + require.True(s.T(), ok, "unexpected response type: %T", v) + + s.Require().Equal(expectedBlock, actualResponse.Block) +} + +// testHappyPath tests a variety of scenarios for data providers in +// happy path scenarios. This function runs parameterized test cases that +// simulate various configurations and verifies that the data provider operates +// as expected without encountering errors. +// +// Arguments: +// - topic: The topic associated with the data provider. +// - tests: A slice of test cases to run, each specifying setup and validation logic. +// - sendData: A function to simulate emitting data into the data channel. +// - requireFn: A function to validate the output received in the send channel. +func (s *BlocksProviderSuite) testHappyPath( + topic string, + tests []testType, + sendData func(chan interface{}, []*flow.Block), + requireFn func(interface{}, *flow.Block), +) { + for _, test := range tests { + s.Run(test.name, func() { + ctx := context.Background() + send := make(chan interface{}, 10) + + // Create a channel to simulate the subscription's data channel + dataChan := make(chan interface{}) + + // // Create a mock subscription and mock the channel + sub := statestreamsmock.NewSubscription(s.T()) + sub.On("Channel").Return((<-chan interface{})(dataChan)) + sub.On("Err").Return(nil) + test.setupBackend(sub) + + // Create the data provider instance + provider, err := s.factory.NewDataProvider(ctx, topic, test.arguments, send) + s.Require().NotNil(provider) + s.Require().NoError(err) + + // Run the provider in a separate goroutine + go func() { + err = provider.Run() + s.Require().NoError(err) + }() + + // Simulate emitting data to the data channel + go func() { + defer close(dataChan) + sendData(dataChan, s.blocks) + }() + + // Collect responses + for _, b := range s.blocks { + unittest.RequireReturnsBefore(s.T(), func() { + v, ok := <-send + s.Require().True(ok, "channel closed while waiting for block %x %v: err: %v", b.Header.Height, b.ID(), sub.Err()) + + requireFn(v, b) + }, time.Second, fmt.Sprintf("timed out waiting for block %d %v", b.Header.Height, b.ID())) + } + + // Ensure the provider is properly closed after the test + provider.Close() + }) + } +} diff --git a/engine/access/rest/websockets/data_providers/factory_test.go b/engine/access/rest/websockets/data_providers/factory_test.go index 1b33d892573..2ed2b075d0c 100644 --- a/engine/access/rest/websockets/data_providers/factory_test.go +++ b/engine/access/rest/websockets/data_providers/factory_test.go @@ -58,8 +58,6 @@ func (s *DataProviderFactorySuite) setupSubscription(apiCall *mock.Call) { // TestSupportedTopics verifies that supported topics return a valid provider and no errors. // Each test case includes a topic and arguments for which a data provider should be created. func (s *DataProviderFactorySuite) TestSupportedTopics() { - s.T().Parallel() - // Define supported topics and check if each returns the correct provider without errors testCases := []struct { name string From 437af4f6cdb52aaddc8b07cb80943b93c88dceed Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Mon, 2 Dec 2024 14:57:31 +0200 Subject: [PATCH 31/35] Added additional description for websocket data providers Run method --- .../rest/websockets/data_providers/data_provider.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/engine/access/rest/websockets/data_providers/data_provider.go b/engine/access/rest/websockets/data_providers/data_provider.go index 9d1d8855f21..08dc497808b 100644 --- a/engine/access/rest/websockets/data_providers/data_provider.go +++ b/engine/access/rest/websockets/data_providers/data_provider.go @@ -17,6 +17,17 @@ type DataProvider interface { Close() error // Run starts processing the subscription and handles responses. // + // The separation of the data provider's creation and its Run() method + // allows for better control over the subscription lifecycle. By doing so, + // a confirmation message can be sent to the client immediately upon + // successful subscription creation or failure. This ensures any required + // setup or preparation steps can be handled prior to initiating the + // subscription and data streaming process. + // + // Run() begins the actual processing of the subscription. At this point, + // the context used for provider creation is no longer needed, as all + // necessary preparation steps should have been completed. + // // No errors are expected during normal operations. Run() error } From 1f2f0aea0dfb1e999ab71c2813772d585a941df4 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Mon, 2 Dec 2024 16:56:29 +0200 Subject: [PATCH 32/35] Added generic HandleResponse, updated providers --- .../data_providers/block_digests_provider.go | 22 +++++++------------ .../data_providers/block_headers_provider.go | 22 +++++++------------ .../data_providers/blocks_provider.go | 22 +++++++------------ .../data_providers/blocks_provider_test.go | 2 +- engine/access/subscription/util.go | 22 +++++++++++++++++++ 5 files changed, 47 insertions(+), 43 deletions(-) diff --git a/engine/access/rest/websockets/data_providers/block_digests_provider.go b/engine/access/rest/websockets/data_providers/block_digests_provider.go index b2f5d3496b3..1fa3f7a6dc7 100644 --- a/engine/access/rest/websockets/data_providers/block_digests_provider.go +++ b/engine/access/rest/websockets/data_providers/block_digests_provider.go @@ -58,7 +58,14 @@ func NewBlockDigestsDataProvider( // // No errors are expected during normal operations. func (p *BlockDigestsDataProvider) Run() error { - return subscription.HandleSubscription(p.subscription, p.handleResponse(p.send)) + return subscription.HandleSubscription( + p.subscription, + subscription.HandleResponse(p.send, func(block *flow.BlockDigest) (interface{}, error) { + return &models.BlockDigestMessageResponse{ + Block: block, + }, nil + }), + ) } // createSubscription creates a new subscription using the specified input arguments. @@ -73,16 +80,3 @@ func (p *BlockDigestsDataProvider) createSubscription(ctx context.Context, args return p.api.SubscribeBlockDigestsFromLatest(ctx, args.BlockStatus) } - -// handleResponse processes a block digest and sends the formatted response. -// -// No errors are expected during normal operations. -func (p *BlockDigestsDataProvider) handleResponse(send chan<- interface{}) func(block *flow.BlockDigest) error { - return func(block *flow.BlockDigest) error { - send <- &models.BlockDigestMessageResponse{ - Block: block, - } - - return nil - } -} diff --git a/engine/access/rest/websockets/data_providers/block_headers_provider.go b/engine/access/rest/websockets/data_providers/block_headers_provider.go index 7cd91fa4a38..4f9e29e2428 100644 --- a/engine/access/rest/websockets/data_providers/block_headers_provider.go +++ b/engine/access/rest/websockets/data_providers/block_headers_provider.go @@ -58,7 +58,14 @@ func NewBlockHeadersDataProvider( // // No errors are expected during normal operations. func (p *BlockHeadersDataProvider) Run() error { - return subscription.HandleSubscription(p.subscription, p.handleResponse(p.send)) + return subscription.HandleSubscription( + p.subscription, + subscription.HandleResponse(p.send, func(header *flow.Header) (interface{}, error) { + return &models.BlockHeaderMessageResponse{ + Header: header, + }, nil + }), + ) } // createSubscription creates a new subscription using the specified input arguments. @@ -73,16 +80,3 @@ func (p *BlockHeadersDataProvider) createSubscription(ctx context.Context, args return p.api.SubscribeBlockHeadersFromLatest(ctx, args.BlockStatus) } - -// handleResponse processes a block header and sends the formatted response. -// -// No errors are expected during normal operations. -func (p *BlockHeadersDataProvider) handleResponse(send chan<- interface{}) func(header *flow.Header) error { - return func(header *flow.Header) error { - send <- &models.BlockHeaderMessageResponse{ - Header: header, - } - - return nil - } -} diff --git a/engine/access/rest/websockets/data_providers/blocks_provider.go b/engine/access/rest/websockets/data_providers/blocks_provider.go index 794cd8e63f8..72cfaa6f554 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider.go @@ -67,7 +67,14 @@ func NewBlocksDataProvider( // // No errors are expected during normal operations. func (p *BlocksDataProvider) Run() error { - return subscription.HandleSubscription(p.subscription, p.handleResponse(p.send)) + return subscription.HandleSubscription( + p.subscription, + subscription.HandleResponse(p.send, func(block *flow.Block) (interface{}, error) { + return &models.BlockMessageResponse{ + Block: block, + }, nil + }), + ) } // createSubscription creates a new subscription using the specified input arguments. @@ -83,19 +90,6 @@ func (p *BlocksDataProvider) createSubscription(ctx context.Context, args Blocks return p.api.SubscribeBlocksFromLatest(ctx, args.BlockStatus) } -// handleResponse processes a block and sends the formatted response. -// -// No errors are expected during normal operations. -func (p *BlocksDataProvider) handleResponse(send chan<- interface{}) func(*flow.Block) error { - return func(block *flow.Block) error { - send <- &models.BlockMessageResponse{ - Block: block, - } - - return nil - } -} - // ParseBlocksArguments validates and initializes the blocks arguments. func ParseBlocksArguments(arguments models.Arguments) (BlocksArguments, error) { var args BlocksArguments diff --git a/engine/access/rest/websockets/data_providers/blocks_provider_test.go b/engine/access/rest/websockets/data_providers/blocks_provider_test.go index 83fe16bbae6..6f46d27ccfe 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider_test.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider_test.go @@ -217,7 +217,7 @@ func (s *BlocksProviderSuite) requireBlock(v interface{}, expectedBlock *flow.Bl // Arguments: // - topic: The topic associated with the data provider. // - tests: A slice of test cases to run, each specifying setup and validation logic. -// - sendData: A function to simulate emitting data into the data channel. +// - sendData: A function to simulate emitting data into the subscription's data channel. // - requireFn: A function to validate the output received in the send channel. func (s *BlocksProviderSuite) testHappyPath( topic string, diff --git a/engine/access/subscription/util.go b/engine/access/subscription/util.go index 2dadac441cf..9ef98044bb8 100644 --- a/engine/access/subscription/util.go +++ b/engine/access/subscription/util.go @@ -54,3 +54,25 @@ func HandleRPCSubscription[T any](sub Subscription, handleResponse func(resp T) return nil } + +// HandleResponse processes a generic response of type and sends it to the provided channel. +// +// Parameters: +// - send: The channel to which the processed response is sent. +// - transform: A function to transform the response into the expected interface{} type. +// +// No errors are expected during normal operations. +func HandleResponse[T any](send chan<- interface{}, transform func(resp T) (interface{}, error)) func(resp T) error { + return func(response T) error { + // Transform the response + resp, err := transform(response) + if err != nil { + return fmt.Errorf("failed to transform response: %w", err) + } + + // send to the channel + send <- resp + + return nil + } +} From f8668ef4840fe743cdbd86839224f747adb5c8e2 Mon Sep 17 00:00:00 2001 From: Uliana Andrukhiv Date: Tue, 3 Dec 2024 17:05:14 +0200 Subject: [PATCH 33/35] Update block data provider test Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com> --- .../rest/websockets/data_providers/blocks_provider_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/access/rest/websockets/data_providers/blocks_provider_test.go b/engine/access/rest/websockets/data_providers/blocks_provider_test.go index 6f46d27ccfe..9e07f9459e9 100644 --- a/engine/access/rest/websockets/data_providers/blocks_provider_test.go +++ b/engine/access/rest/websockets/data_providers/blocks_provider_test.go @@ -233,7 +233,7 @@ func (s *BlocksProviderSuite) testHappyPath( // Create a channel to simulate the subscription's data channel dataChan := make(chan interface{}) - // // Create a mock subscription and mock the channel + // Create a mock subscription and mock the channel sub := statestreamsmock.NewSubscription(s.T()) sub.On("Channel").Return((<-chan interface{})(dataChan)) sub.On("Err").Return(nil) From fa089b0a59a70bf08933f183024c9cd217e223d3 Mon Sep 17 00:00:00 2001 From: Jordan Schalm Date: Tue, 3 Dec 2024 15:22:25 -0800 Subject: [PATCH 34/35] remove ex-package links these don't resolve correctly when testing locally --- engine/consensus/dkg/doc.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/consensus/dkg/doc.go b/engine/consensus/dkg/doc.go index 2feff12aa42..5bf29b43eaa 100644 --- a/engine/consensus/dkg/doc.go +++ b/engine/consensus/dkg/doc.go @@ -18,8 +18,8 @@ // # Architecture // // In the happy path, one DKG instance runs every epoch. For each DKG instance, the [ReactorEngine] -// instantiates a new, epoch-scoped [module.DKGController] and [module.DKGBroker] using the provided ControllerFactory. -// The ControllerFactory ties new DKGControllers to the [MessagingEngine] via a BrokerTunnel, +// instantiates a new, epoch-scoped module.DKGController and module.DKGBroker using the provided dkg.ControllerFactory. +// The dkg.ControllerFactory ties new module.DKGController's to the [MessagingEngine] via a dkg.BrokerTunnel, // which exposes channels to relay incoming and outgoing messages (see package module/dkg for details). // // EpochSetup/EpochCommit/OnView events From 3f1de8f49f95c4bf909ceb192610dca53a3fe4eb Mon Sep 17 00:00:00 2001 From: Jordan Schalm Date: Tue, 3 Dec 2024 15:23:20 -0800 Subject: [PATCH 35/35] Apply suggestions from code review Co-authored-by: Alexander Hentschel --- engine/consensus/dkg/doc.go | 47 +++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/engine/consensus/dkg/doc.go b/engine/consensus/dkg/doc.go index 5bf29b43eaa..15ee9e1e06a 100644 --- a/engine/consensus/dkg/doc.go +++ b/engine/consensus/dkg/doc.go @@ -5,7 +5,7 @@ // The [ReactorEngine] implements triggers to control the lifecycle of DKG instances. // A new DKG instance is started when an EpochSetup service event is sealed. // The subsequent phase transitions are triggered when specified views are encountered. -// Specifically, phase transitions for a view V are triggered when the first block with view >=V is finalized. +// Specifically, phase transitions for a view V are triggered when the first block with view ≥V is finalized. // Between phase transitions, we periodically query the DKG smart-contract ("whiteboard") to read broadcast messages. // Before transitioning the state machine to the next phase, we query the whiteboard w.r.t. the final view // of the phase - this ensures all participants eventually observe the same set of messages for each phase. @@ -22,29 +22,24 @@ // The dkg.ControllerFactory ties new module.DKGController's to the [MessagingEngine] via a dkg.BrokerTunnel, // which exposes channels to relay incoming and outgoing messages (see package module/dkg for details). // -// EpochSetup/EpochCommit/OnView events -// | -// v -// +---------------+ -// | ReactorEngine | -// +---------------+ -// | -// v -// *~~~~~~~~~~~~~~~~~~~~~* <- Epoch-scoped components -// | +---------------+ | -// | | Controller | | -// | +---------------+ | -// | | | -// | v | -// | +---------------+ | -// | | Broker | | -// | +---------------+ | -// *~~~~~~~~|~~~~~~~~~\~~* -// | \ -// BrokerTunnel DKGContractClient -// | \ -// +--------------+ +------------------+ -// | Messaging | | FlowDKG smart | -// | Engine | | contract | -// +--------------+ +------------------+ +// EpochSetup/EpochCommit/OnView events +// ↓ +// ┏━━━━━━━━━━━━━━━━━┓ +// ┃ ReactorEngine ┃ +// ┗━━━━━━━━━━━━━━━━━┛ +// ↓ +// ┏━━━━━━━━━━━━━━━━━┓ ╮ +// ┃ Controller ┃ │ +// ┗━━━━━━━━━━━━━━━━━┛ │ +// ↓ ┝ Epoch-scoped components +// ┏━━━━━━━━━━━━━━━━━┓ │ +// ┃ Broker ┃ │ +// ┗━━━━━━━━━━━━━━━━━┛ ╯ +// │ │ +// BrokerTunnel DKGContractClient +// ↓ ↓ +// ┏━━━━━━━━━━━━━━┓ ┏━━━━━━━━━━━━━━━━━━┓ +// ┃ Messaging ┃ ┃ FlowDKG smart ┃ +// ┃ Engine ┃ ┃ contract ┃ +// ┗━━━━━━━━━━━━━━┛ ┗━━━━━━━━━━━━━━━━━━┛ package dkg