Proposal: API redesign to allow customers to mock Ably in their own unit tests

Open Rosalita opened this issue 2 years ago • 0 comments

What is the problem this proposal will solve?

As a developer which is integrating Ably into my own project code I want to be able to mock responses for both Ably REST and Realtime operations So that I can write tests in my own project which run quickly and independently without making real requests to the Ably platform.

Limitations of the current API design.

Both the REST and Realtime APIs follows a nested design. This design was originally carried over from Ruby to Go. One of the primary reasons for this design was to organise methods by namespace. The current designs are focused around two objects, a client object and a channel object.

In order to mock Ably in tests, an interface is required that allows both a real client with real channels and a mock client with mock channels to pass through it. While in Go it is possible to nest structs inside structs, it's not possible to nest structs inside interfaces. This makes it impossible to mock any of the public methods on nested structs, e.g.

client.Auth.xxx
client.Channels.xxx
channel.Presence.xxx

Proposed approach to solve this problem

For the REST client, it is acceptable to flatten the API so that a single interface can express all functionality. This is because once a channel operation is performed, it is complete. Channels are not long lived. A flat design where all methods are available on the REST object is a simpler design where only one type needs to be mocked in unit tests. Previously methods were spread across five different types.

By adding new wrappers for existing functionality, this would allow the new API design to exist along side the current design without creating a breaking change.

Proposed new interfaces

This proposal would create a new package called ablyiface which would contain the following new interfaces:

REST Client

For the REST client, it is acceptable to flatten the API. Allowing a single interface to express all functionality. This is because once a channel operation is performed, it is complete. REST Channels are not long lived objects.

Proposed interface:

//RESTClient is an interface that has the same methods as ably.REST
type RESTClientAPI interface {
	Time(ctx context.Context) (time.Time, error)
	Stats(o ...ably.StatsOption) ably.StatsRequest
	Request(method string, path string, o ...ably.RequestOption) ably.RESTRequest
	
	// Auth
	ClientID() string
	CreateTokenRequest(params *ably.TokenParams, opts ...ably.AuthOption) (*ably.TokenRequest, error)
	RequestToken(ctx context.Context, params *ably.TokenParams, opts ...ably.AuthOption) (*ably.TokenDetails, error)
	Authorize(ctx context.Context, params *ably.TokenParams, setOpts ...ably.AuthOption) (*ably.TokenDetails, error)
	
	// Channel
	ReleaseChannel(channel string)
	Publish(ctx context.Context, channel string, name string, data interface{}, o ...ably.PublishMultipleOption) error
	PublishMultiple(ctx context.Context, channel string, messages []*ably.Message, o ...ably.PublishMultipleOption) error
	History(channel string, o ...ably.HistoryOption) ably.HistoryRequest
	
	// Presence
	GetPresence(channel string, o ...ably.GetPresenceOption) ably.PresenceRequest
	PresenceHistory(channel string, o ...ably.PresenceHistoryOption) ably.PresenceRequest
}

Flattened methods moved to the top level: ClientID is a wrapper for client.Auth.ClientID CreateTokenRequest is a wrapper for client.Auth.CreateTokenRequest RequestToken is a wrapper for client.Auth.RequestToken Authorize is a wrapper for client.Auth.Authorize Release is a wrapper for client.Channels.Release GetPresence is a wrapper for channel.Presence.Get GetPresenceHistory is a wrapper for channel.Presence.History

The following methods have changed signature to now accept a channel name: Publish is a wrapper for channel.Publish PublishMultiple is a wrapper for channel.PublishMultiple History is a wrapper for channel.History

No wrappers provided for: client.Channels.Get

previously this would return a channel object, with flat designed API this method is no longer required as channel operations now exist at the top level on the client. client.Channels.Iterate - As Channels are changing to map[string]*RESTChannel listing channels can be done directly by ranging over that map. So this method is no longer required. client.Channels.Exists - As Channels are changing to map[string]*RESTChannel checks for existence can be done directly using that map. So this method is no longer required.

Deprecated Type (to eventually be removed): type RESTChannels struct

For Realtime Client

For the Realtime client, the API should not be completely flat. This is because the RealtimeChannel object can exist for a long time. This design will require two interfaces, one for the client and one for a channel.

Proposed client interface:

//RealtimeClient is an interface that has the same methods as ably.Realtime
type RealtimeClientAPI interface {
    Connect()
    Close()
    Stats(o ...ably.StatsOption) ably.StatsRequest
    Time(ctx context.Context) (time.Time, error)
    ClientID() string
    CreateTokenRequest(params *ably.TokenParams, opts ...ably.AuthOption) (*ably.TokenRequest, error)
    RequestToken(ctx context.Context, params *ably.TokenParams, opts ...ably.AuthOption) (*ably.TokenDetails, error)
    Authorize(ctx context.Context, params *ably.TokenParams, setOpts ...ably.AuthOption) (*ably.TokenDetails, error)
    GetChannel(name string, options ...ably.ChannelOption) RealtimeChannelAPI // This method returns an interface
    ReleaseChannel(ctx context.Context, name string)
    ChannelExists(name string) bool
    IterateChannels() []*ably.RealtimeChannel
    GetConnectionID() string
    GetConnectionKey() string
    ConnectionErrorReason() *ably.ErrorInfo
    ConnectionRecoveryKey() string
    ConnectionSerial() *int64
    ConnectionState() ably.ConnectionState
}

ClientID is a wrapper for client.Auth.ClientID CreateTokenRequest is a wrapper for client.Auth.CreateTokenRequest RequestToken is a wrapper for client.Auth.RequestToken Authorize is a wrapper for client.Auth.Authorize GetChannel is a wrapper for client.Channels.Get ReleaseChannel is a wrapper for client.Channels.Release ChannelExists is a wrapper for client.Channels.Exists IterateChannels is a wrapper for client.Channels.Iterate GetConnectionID is a wrapper for client.Connection.ID GetConnectionKey is a wrapper for client.Connection.Key ConnectionErrorReason is a wrapper for client.Connection.ErrorReason ConnectionRecoveryKey is a wrapper for client.Connection.RecoveryKey ConnectionSerial is a wrapper for client.Connection.Serial ConnectionState is a wrapper for client.Connection.State

Proposed channel interface:

//RealtimeChannel is an interface that has the same methods as ably.RealtimeChannel
type RealtimeChannelAPI interface {
    Attach(ctx context.Context) error
    Detach(ctx context.Context) error
    Subscribe(ctx context.Context, name string, handle func(*ably.Message)) (func(), error)
    SubscribeAll(ctx context.Context, handle func(*ably.Message)) (func(), error)
    Publish(ctx context.Context, name string, data interface{}) error
    PublishMultiple(ctx context.Context, messages []*ably.Message) error
    History(o ...ably.HistoryOption) ably.HistoryRequest
    State() ably.ChannelState
    ErrorReason() *ably.ErrorInfo
    Modes() []ably.ChannelMode
    Params() map[string]string
    PresenceSyncComplete() bool
    GetPresence(ctx context.Context) ([]*ably.PresenceMessage, error)
    GetPresenceWithOptions(ctx context.Context, options ...ably.PresenceGetOption) ([]*ably.PresenceMessage, error)
    PresenceSubscribe(ctx context.Context, action ably.PresenceAction, handle func(*ably.PresenceMessage)) (func(), error)
    PresenceSubscribeAll(ctx context.Context, handle func(*ably.PresenceMessage)) (func(), error)
    PresenceEnter(ctx context.Context, data interface{}) error
    PresenceUpdate(ctx context.Context, data interface{}) error
    PresenceLeave(ctx context.Context, data interface{}) error
    PresenceEnterClient(ctx context.Context, clientID string, data interface{}) error
    PresenceUpdateClient(ctx context.Context, clientID string, data interface{}) error
    PresenceLeaveClient(ctx context.Context, clientID string, data interface{}) error
}

PresenceSyncComplete is a wrapper for channel.Presence.SyncComplete GetPresence is a wrapper for channel.Presence.Get GetPresenceWithOptions is a wrapper for channel.Presence.GetWithOptions PresenceSubscribe is a wrapper for channel.Presence.Subscribe PresenceSubscribeAll is a wrapper for channel.Presence.SubscribeAll PresenceEnter is a wrapper for channel.Presence.Enter PresenceUpdate is a wrapper for channel.Presence.Update PresenceLeave is a wrapper for channel.Presence.Leave PresenceEnterClient is a wrapper for channel.Presence.EnterClient PresenceUpdateClient is a wrapper for channel.Presence.UpdateClient PresenceLeaveClient is a wrapper for channel.Presence.LeaveClient

Proposed struct field exposure

The current API design contains some private struct fields which prevent data from being easily mocked in tests. As part of this redesign, all structs which have the primary purpose of holding data, should be made public with public fields.

One example of this is the the ably.ErrorInfo struct. This object describes errors returned by Ably. As it is a response from Ably, it is something which will be important to be able to mock in tests. Currently it looks like

type ErrorInfo struct {
    StatusCode int
    Code       ErrorCode
    HRef       string
    Cause      *ErrorInfo
    err        error
}

Every field is public apart from err which should be changed to Err so that it can be populated and manipulated by mocks in tests.

Example Use of Interfaces in Tests

main.go

package main

import (
	"context"
	"fmt"
	"os"
	"github.com/ably/ably-go/ably"
	"github.com/ably/ably-go/ablyiface"
)

//MyFunc uses an Ably REST Client, its signature accepts an ablyiface.RESTClientAPI interface.
func MyFunc(ctx context.Context, channelName string, client ablyiface.RESTClientAPI) error{
	return client.Publish(ctx, channelName, "temperature", "12.5" )
}

func main() {
	key, _ := os.LookupEnv("ABLY_PRIVATE_KEY")
	restClient, _ := ably.NewREST(
		ably.WithKey(key),
	)
	result := MyFunc(context.Background(), "myChannel", restClient)
	
    // Do something with the result
	fmt.Println(result)
}

main_test.go

package main_test

import (
	"context"
	"errors"
	"net/http"
	"testing"

	a "path/to/your/code/under/test"
	"github.com/ably/ably-go/ably"
	"github.com/ably/ably-go/ablyiface"
	"github.com/stretchr/testify/assert"
)

// Define mock structs to be used in your unit tests.
// MockRESTClient mocks ably.RESTClient.
type mockRESTClient struct {
	ablyiface.RESTClientAPI
}

// Publish is a mock implementation which will return an error if the channel name is "error".
func (m mockRESTClient) Publish(ctx context.Context, channel string, name string, data interface{}, options ...ably.PublishMultipleOption) error {
	if channel == "error" {
		return &ably.ErrorInfo{
			Code:       50000,
			StatusCode: http.StatusBadGateway,
			Err:        errors.New("an error"),
		}
	}
	return nil
}

func TestMyFunc(t *testing.T) {

	// Instantiate mocks.
	mockClient := mockRESTClient{}

	// Declare table tests
	tests := map[string]struct {
		channelName string
		expectedErr error
	}{
		"Can successfully publish": {
			channelName: "ok",
			expectedErr: nil,
		},
		"Can handle and error when publishing": {
			channelName: "error",
			expectedErr: &ably.ErrorInfo{
				Code:       50000,
				StatusCode: http.StatusBadGateway,
				Err:        errors.New("an error"),
			},
		},
	}

	// Run tests.
	for testName, test := range tests {
		t.Run(testName, func(t *testing.T) {

			// Pass a mock client into your functions instead of a real client.
			result := a.MyFunc(context.Background(), test.channelName, mockClient)

			// assert on results
			assert.Equal(t, test.expectedErr, result)
		})
	}
}

lmars has also created the following example which shows how the Realtime client GetChannel method can return a RealtimeChannel interface which allows a mock to be constructed over two interfaces that work together.

package main

import (
	"context"
	"testing"
)

// RealtimeClient represents what would be the RealtimeClient interface from the ably
// package.
type RealtimeClient interface {
	GetChannel(name string) RealtimeChannel
}

// RealtimeChannel represents what would be the RealtimeChannel interface from the ably
// package.
type RealtimeChannel interface {
	Publish(ctx context.Context, name string, data interface{}) error
}

// mockClient is a mock implementation of the RealtimeClient interface.
//
// It keeps a reference to each channel initialised via GetChannel so that
// tests can make assertions about what channels were retrieved.
type mockClient struct {
	channels []*mockChannel
}

// GetChannel returns a mock RealtimeChannel object that can be used to check
// what messages get published.
func (m *mockClient) GetChannel(name string) RealtimeChannel {
	channel := &mockChannel{
		name: name,
	}
	m.channels = append(m.channels, channel)
	return channel
}

// mockChannel is a mock implementation of the RealtimeChannel interface.
//
// It keeps a reference to each message published so that tests can make
// assertions about what was published.
type mockChannel struct {
	name              string
	publishedMessages []*mockMessage
}

// Publish tracks that a message was published with the given name and data.
func (m *mockChannel) Publish(ctx context.Context, name string, data interface{}) error {
	m.publishedMessages = append(m.publishedMessages, &mockMessage{
		name: name,
		data: data,
	})
	return nil
}

// mockMessage represents a message published to a mockChannel.
type mockMessage struct {
	name string
	data interface{}
}

// Server is my own HTTP server that I want to test.
type Server struct {
	ably RealtimeClient
}

// DoSomething is called by my own code, and I expect it to publish a message
// to an Ably channel.
func (srv *Server) DoSomething(ctx context.Context) error {
	channel := srv.ably.GetChannel("foo")
	return channel.Publish(ctx, "hey", "there")
}

// TestDoSomething tests that calling DoSomething leads to a message being
// published to Ably.
func TestDoSomething(t *testing.T) {
	// initialise my server with a mock client
	client := &mockClient{}
	srv := &Server{
		ably: client,
	}

	// call the DoSomething method
	if err := srv.DoSomething(context.TODO()); err != nil {
		t.Fatal(err)
	}

	// check that a channel was initialised with the correct name
	if len(client.channels) != 1 {
		t.Fatal("expected an Ably channel to be initialised")
	}
	channel := client.channels[0]
	if channel.name != "foo" {
		t.Fatalf("expected channel with name %q, got %q", "foo", channel.name)
	}

	// check that a message was published to the channel with the correct
	// data
	if len(channel.publishedMessages) != 1 {
		t.Fatal("expected a published message")
	}
	msg := channel.publishedMessages[0]
	if msg.name != "hey" {
		t.Fatalf("expected message to be published with name %q, got %q", "hey", msg.name)
	}
	if msg.data != "there" {
		t.Fatalf("expected message to be published with data %q, got %q", "there", msg.data)
	}
}

Pagination

Pagination in ably-go takes a cursor based approach as opposed to an offset based approach.

Requests for Stats, History, Presence and PresenceHistory all done via REST http requests that return a cursor which can be used to iterate through paginated data.

With the current API design, the example below show how a user can retrieve stats data. A minimum of three calls are required when working with items, or four calls if the user wants to work with pages of data.

// a user wants to be able to retrieve stats using a REST client.
func MyStats(ctx context.Context, client ablyiface.RESTClientAPI) ([]ably.Stats, error){

	// First the user must call Stats() which returns a statsRequest
	statsRequest := client.Stats()

	// The user must then decide if they want to work with all items
	// of if they would prefer to work with pages of items

	// They make one of two calls to either request items or pages.
	items, _ := statsRequest.Items(ctx) 
	pages, _ := statsRequest.Pages(ctx) 

	// If working with items, to get to the stats data, the user must call items.Next()
	// in a loop to loop through all items and handle each item of data.
	var itemStats []ably.Stats
	for items.Next(ctx){
		itemStats = append(itemStats, *items.Item())
	}

	// If working with pages, to get to the stats data, the user must call pages.Next()
	// in a loop to loop through all pages, then they must call pages.Items to get the 
	// items for each page. As items is a slice of pointers, the user must then loop
	// again over each chunk of items to handle each item of data.
	var pagesStats []ably.Stats
	for pages.Next(ctx){
		items := pages.Items()
		for _, item := range items {
			pagesStats = append(pagesStats, *item)
		}
	}

	// Both ways of extracting the stats data give the same end result. 
	if len(itemStats) == len(pagesStats) {
		fmt.Println("items and pages contain the same amount of data")
	}

	return itemStats, nil
}

In other SDKs paddybyers confirmed that the call to Stats() directly returns an object (pagination cursor) which can be iterated on to access the data.

To make it easier to mock pagination cursors, we should hide the request under the hood so that a developer integrating ably into their code does not have to mock a *http.Response body from Ably. By handling the request under the hood this will allow code which performs REST http requests to remain private.

The current API design has two types of cursor object which can be iterated on to access data. There are cursors which iterate items and cursors which iterate pages. These two types of cursor exist for Stats, Messages and Presence making 6 types of cursor available in total in the current API design.

Because the return types are embedded in the item and items methods for these cursors one design which would allow mocking of paginated data would be a design where the user passes their own object into a paginated request and the Ably platform deserialises data directly into the provided object. Similar to how the Go standard library deserialises when it unmarshals JSON into a struct.

When redesigning this area of the API thought should be given to expressing a cursor which can be used to iterate over paginated data as an interface by defining a common collection of methods.

┆Issue is synchronized with this Jira Task by Unito

Apr 28 '22 16:04 Rosalita

ably-go ably-go copied to clipboard

Proposal: API redesign to allow customers to mock Ably in their own unit tests

What is the problem this proposal will solve?

Limitations of the current API design.

Proposed approach to solve this problem

Proposed new interfaces

REST Client

For Realtime Client

Proposed struct field exposure

Example Use of Interfaces in Tests

Pagination

ably-go
ably-go copied to clipboard