Skip to content

Commit

Permalink
clientOId refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
Luke Rogerson committed Oct 23, 2023
1 parent 7f1df80 commit 21ccf5a
Show file tree
Hide file tree
Showing 18 changed files with 225 additions and 32 deletions.
33 changes: 28 additions & 5 deletions data/redisrepo/find_order_by_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,42 @@ package redisrepo

import (
"context"
"fmt"

"github.com/google/uuid"
"github.com/orbs-network/order-book/models"
"github.com/orbs-network/order-book/utils/logger"
"github.com/orbs-network/order-book/utils/logger/logctx"
"github.com/redis/go-redis/v9"
)

func (r *redisRepository) FindOrderById(ctx context.Context, orderId uuid.UUID) (*models.Order, error) {
orderIDKey := CreateOrderIDKey(orderId)
// FindOrderById finds an order by its ID or clientOId. If `isClientOId` is true, the `id` is treated as a clientOId, otherwise it is treated as an orderId.
func (r *redisRepository) FindOrderById(ctx context.Context, id uuid.UUID, isClientOId bool) (*models.Order, error) {
var orderId uuid.UUID

if isClientOId {
logctx.Info(ctx, "finding order by clientOId", logger.String("clientOId", id.String()))

orderIdStr, err := r.client.Get(ctx, CreateClientOIDKey(id)).Result()
if err != nil {
if err == redis.Nil {
return nil, models.ErrOrderNotFound
}
logctx.Error(ctx, "could not get order ID by clientOId", logger.Error(err))
return nil, err
}

orderId, err = uuid.Parse(orderIdStr)
if err != nil {
logctx.Error(ctx, "invalid order ID format retrieved by clientOId", logger.Error(err))
return nil, fmt.Errorf("invalid order ID format retrieved by clientOId: %s", err)
}
} else {
logctx.Info(ctx, "finding order by orderId", logger.String("orderId", id.String()))
orderId = id
}

orderMap, err := r.client.HGetAll(ctx, orderIDKey).Result()
orderMap, err := r.client.HGetAll(ctx, CreateOrderIDKey(orderId)).Result()
if err != nil {
logctx.Error(ctx, "could not get order", logger.Error(err))
return nil, err
Expand All @@ -23,9 +48,7 @@ func (r *redisRepository) FindOrderById(ctx context.Context, orderId uuid.UUID)
}

order := &models.Order{}

err = order.MapToOrder(orderMap)

if err != nil {
logctx.Error(ctx, "could not map order", logger.Error(err))
return nil, err
Expand Down
57 changes: 44 additions & 13 deletions data/redisrepo/find_order_by_id_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package redisrepo
import (
"context"
"errors"
"fmt"
"testing"

"github.com/go-redis/redismock/v9"
Expand All @@ -23,41 +24,71 @@ func TestRedisRepository_FindOrderById(t *testing.T) {

sizeDec, _ := decimal.NewFromString("126")

orderID := uuid.New()
orderID := uuid.MustParse("00000000-0000-0000-0000-000000000008")
clientOId := uuid.MustParse("00000000-0000-0000-0000-000000000009")
symbol, _ := models.StrToSymbol("USDC-ETH")
price := decimal.NewFromFloat(10000.55)
order := models.Order{
Id: orderID,
UserId: uuid.New(),
Price: price,
Size: sizeDec,
Symbol: symbol,
Side: models.BUY,
Status: models.STATUS_OPEN,
Id: orderID,
ClientOId: clientOId,
UserId: uuid.New(),
Price: price,
Size: sizeDec,
Symbol: symbol,
Side: models.BUY,
Status: models.STATUS_OPEN,
}

t.Run("order found", func(t *testing.T) {
t.Run("order found by orderID", func(t *testing.T) {
mock.ExpectHGetAll(CreateOrderIDKey(orderID)).SetVal(order.OrderToMap())

foundOrder, err := repo.FindOrderById(ctx, orderID)
foundOrder, err := repo.FindOrderById(ctx, orderID, false)
assert.NoError(t, err)
assert.Equal(t, order, *foundOrder)
})

t.Run("order not found", func(t *testing.T) {
t.Run("order found by clientOId", func(t *testing.T) {
mock.ExpectGet(CreateClientOIDKey(clientOId)).SetVal(orderID.String())
mock.ExpectHGetAll(CreateOrderIDKey(orderID)).SetVal(order.OrderToMap())

foundOrder, err := repo.FindOrderById(ctx, clientOId, true)
assert.NoError(t, err)
assert.Equal(t, order, *foundOrder)
})

t.Run("order not found by orderId", func(t *testing.T) {
nonExistentOrderID := uuid.New()
mock.ExpectHGetAll(CreateOrderIDKey(orderID)).SetVal(map[string]string{})

foundOrder, err := repo.FindOrderById(ctx, nonExistentOrderID)
foundOrder, err := repo.FindOrderById(ctx, nonExistentOrderID, false)
assert.Error(t, err, models.ErrOrderNotFound.Error())
assert.Nil(t, foundOrder)
})

t.Run("order not found by clientOId", func(t *testing.T) {
nonExistentClientOId := uuid.New()
mock.ExpectGet(CreateClientOIDKey(nonExistentClientOId)).RedisNil()

foundOrder, err := repo.FindOrderById(ctx, nonExistentClientOId, true)
assert.Error(t, err, models.ErrOrderNotFound.Error())
assert.Nil(t, foundOrder)
})

t.Run("invalid order ID format retrieved by clientOId", func(t *testing.T) {
invalidOrderID := "invalid-order-id"
mock.ExpectGet(CreateClientOIDKey(clientOId)).SetVal(invalidOrderID)

foundOrder, err := repo.FindOrderById(ctx, clientOId, true)

assert.Error(t, err, fmt.Sprintf("invalid order ID format retrieved by clientOId: %s", err))
assert.Nil(t, foundOrder)
})

t.Run("unexpected error", func(t *testing.T) {
var redisErr = errors.New("unexpected error")
mock.ExpectHGetAll(CreateOrderIDKey(orderID)).SetErr(redisErr)

foundOrder, err := repo.FindOrderById(ctx, orderID)
foundOrder, err := repo.FindOrderById(ctx, orderID, false)
assert.Error(t, err, redisErr.Error())
assert.Nil(t, foundOrder)
})
Expand Down
2 changes: 1 addition & 1 deletion data/redisrepo/get_best_price_for.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (r *redisRepository) GetBestPriceFor(ctx context.Context, symbol models.Sym
return models.Order{}, err
}

order, err := r.FindOrderById(ctx, orderID)
order, err := r.FindOrderById(ctx, orderID, false)

if err != nil {
logctx.Error(ctx, "unexpected error when getting order for best price", logger.String("orderId", orderID.String()), logger.Error(err))
Expand Down
4 changes: 2 additions & 2 deletions data/redisrepo/get_market_depth.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (r *redisRepository) GetMarketDepth(ctx context.Context, symbol models.Symb
logctx.Error(ctx, "Error parsing ask order id", logger.Error(err))
continue
}
order, err := r.FindOrderById(ctx, orderId)
order, err := r.FindOrderById(ctx, orderId, false)
if err != nil {
logctx.Error(ctx, "Error fetching order", logger.Error(err))
continue
Expand All @@ -64,7 +64,7 @@ func (r *redisRepository) GetMarketDepth(ctx context.Context, symbol models.Symb
logctx.Error(ctx, "Error parsing bid order id", logger.Error(err))
continue
}
order, err := r.FindOrderById(ctx, orderId)
order, err := r.FindOrderById(ctx, orderId, false)
if err != nil {
logctx.Error(ctx, "Error fetching order", logger.Error(err))
continue
Expand Down
2 changes: 1 addition & 1 deletion mocks/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func (m *MockOrderBookStore) RemoveOrder(ctx context.Context, order models.Order
return m.Error
}

func (m *MockOrderBookStore) FindOrderById(ctx context.Context, orderId uuid.UUID) (*models.Order, error) {
func (m *MockOrderBookStore) FindOrderById(ctx context.Context, id uuid.UUID, isClientOId bool) (*models.Order, error) {
if m.Error != nil {
return nil, m.Error
}
Expand Down
4 changes: 4 additions & 0 deletions mocks/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ func (m *MockOrderBookService) GetOrderById(ctx context.Context, orderId uuid.UU
return m.Order, m.Error
}

func (m *MockOrderBookService) GetOrderByClientOId(ctx context.Context, clientOId uuid.UUID) (*models.Order, error) {
return m.Order, m.Error
}

func (m *MockOrderBookService) GetMarketDepth(ctx context.Context, symbol models.Symbol, depth int) (models.MarketDepth, error) {
return m.MarketDepth, m.Error
}
2 changes: 1 addition & 1 deletion models/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func StrToStatus(s string) (Status, error) {
return STATUS_PENDING, nil
case "FILLED":
return STATUS_FILLED, nil
case "CANCELED":
case "CANCELLED":
return STATUS_CANCELLED, nil
default:
return "", ErrInvalidStatus
Expand Down
2 changes: 1 addition & 1 deletion service/cancel_order.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func (s *Service) CancelOrder(ctx context.Context, orderId uuid.UUID) error {
return models.ErrNoUserInContext
}

order, err := s.orderBookStore.FindOrderById(ctx, orderId)
order, err := s.orderBookStore.FindOrderById(ctx, orderId, false)
if err != nil {
logctx.Error(ctx, "error occured when finding order", logger.Error(err))
return err
Expand Down
27 changes: 27 additions & 0 deletions service/get_order_by_client_oid.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package service

import (
"context"

"github.com/google/uuid"
"github.com/orbs-network/order-book/models"
"github.com/orbs-network/order-book/utils/logger"
"github.com/orbs-network/order-book/utils/logger/logctx"
)

func (s *Service) GetOrderByClientOId(ctx context.Context, clientOId uuid.UUID) (*models.Order, error) {
order, err := s.orderBookStore.FindOrderById(ctx, clientOId, true)

if err == models.ErrOrderNotFound {
logctx.Info(ctx, "order not found", logger.String("clientOId", clientOId.String()))
return nil, nil
}

if err != nil {
logctx.Error(ctx, "unexpected error when getting order", logger.String("clientOId", clientOId.String()), logger.Error(err))
return nil, err
}

logctx.Info(ctx, "order found for orderId", logger.String("clientOId", clientOId.String()))
return order, nil
}
48 changes: 48 additions & 0 deletions service/get_order_by_client_oid_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package service_test

import (
"context"
"testing"

"github.com/google/uuid"
"github.com/orbs-network/order-book/mocks"
"github.com/orbs-network/order-book/models"
"github.com/orbs-network/order-book/service"
"github.com/stretchr/testify/assert"
)

func TestService_GetOrderByClientOId(t *testing.T) {

ctx := context.Background()

clientOId := uuid.MustParse("e577273e-12de-4acc-a4f8-de7fb5b86e37")

t.Run("successfully retrieve order by client order ID - should return order", func(t *testing.T) {
o := &models.Order{ClientOId: clientOId}
svc, _ := service.New(&mocks.MockOrderBookStore{Order: o})

order, err := svc.GetOrderByClientOId(ctx, clientOId)

assert.NoError(t, err)
assert.Equal(t, o, order)
})

t.Run("order not found - should return nil", func(t *testing.T) {
svc, _ := service.New(&mocks.MockOrderBookStore{Error: models.ErrOrderNotFound})

order, err := svc.GetOrderByClientOId(ctx, clientOId)

assert.NoError(t, err)
assert.Nil(t, order)
})

t.Run("unexpected error - should return error", func(t *testing.T) {
svc, _ := service.New(&mocks.MockOrderBookStore{Error: assert.AnError})

order, err := svc.GetOrderByClientOId(ctx, clientOId)

assert.ErrorIs(t, err, assert.AnError)
assert.Nil(t, order)
})

}
2 changes: 1 addition & 1 deletion service/get_order_by_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

func (s *Service) GetOrderById(ctx context.Context, orderId uuid.UUID) (*models.Order, error) {
order, err := s.orderBookStore.FindOrderById(ctx, orderId)
order, err := s.orderBookStore.FindOrderById(ctx, orderId, false)

if err == models.ErrOrderNotFound {
logctx.Info(ctx, "order not found", logger.String("orderId", orderId.String()))
Expand Down
13 changes: 9 additions & 4 deletions service/process_order.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (s *Service) ProcessOrder(ctx context.Context, input ProcessOrderInput) (mo

// TODO: logic for when order can be immediately filled and does not need to be added to the order book

existingOrder, err := s.orderBookStore.FindOrderById(ctx, input.ClientOrderID)
existingOrder, err := s.orderBookStore.FindOrderById(ctx, input.ClientOrderID, true)

if err != nil && err != models.ErrOrderNotFound {
logctx.Error(ctx, "unexpected error when finding order by clientOrderId", logger.Error(err))
Expand All @@ -46,19 +46,24 @@ func (s *Service) ProcessOrder(ctx context.Context, input ProcessOrderInput) (mo
return models.Order{}, ErrClashingOrderId
}

if existingOrder.Id == input.ClientOrderID {
if existingOrder.ClientOId == input.ClientOrderID {
logctx.Warn(ctx, "order already exists with same clientOrderId", logger.Error(err), logger.String("clientOrderId", input.ClientOrderID.String()))
return models.Order{}, models.ErrOrderAlreadyExists
}

logctx.Error(ctx, "did not follow any cases when processing order", logger.String("clientOrderId", input.ClientOrderID.String()), logger.String("userId", input.UserId.String()), logger.String("price", input.Price.String()), logger.String("size", input.Size.String()), logger.String("symbol", input.Symbol.String()), logger.String("side", input.Side.String()))
return models.Order{}, models.ErrUnexpectedError

return models.Order{}, models.ErrUnexpectedError
}

func (s *Service) createNewOrder(ctx context.Context, input ProcessOrderInput, orderId uuid.UUID) (models.Order, error) {
func (s *Service) createNewOrder(ctx context.Context, input ProcessOrderInput, clientOId uuid.UUID) (models.Order, error) {
orderId := uuid.New()

logctx.Info(ctx, "creating new order", logger.String("orderId", orderId.String()), logger.String("clientOrderId", clientOId.String()))

order := models.Order{
Id: orderId,
ClientOId: clientOId,
UserId: input.UserId,
Price: input.Price,
Symbol: input.Symbol,
Expand Down
3 changes: 2 additions & 1 deletion service/process_order_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ func TestService_ProcessOrder(t *testing.T) {

assert.NoError(t, err)
// TODO: I am not asserting against the full order as timestamp is always different
assert.Equal(t, newOrder.Id, orderId)
assert.NotEqual(t, newOrder.Id, orderId)
assert.Equal(t, newOrder.ClientOId, orderId)
assert.Equal(t, newOrder.UserId, userId)
assert.Equal(t, newOrder.Price, price)
assert.Equal(t, newOrder.Symbol, symbol)
Expand Down
2 changes: 1 addition & 1 deletion service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
type OrderBookStore interface {
StoreOrder(ctx context.Context, order models.Order) error
RemoveOrder(ctx context.Context, order models.Order) error
FindOrderById(ctx context.Context, orderId uuid.UUID) (*models.Order, error)
FindOrderById(ctx context.Context, id uuid.UUID, isClientOId bool) (*models.Order, error)
GetOrdersAtPrice(ctx context.Context, symbol models.Symbol, price decimal.Decimal) ([]models.Order, error)
GetBestPriceFor(ctx context.Context, symbol models.Symbol, side models.Side) (models.Order, error)
GetMarketDepth(ctx context.Context, symbol models.Symbol, depth int) (models.MarketDepth, error)
Expand Down
Loading

0 comments on commit 21ccf5a

Please sign in to comment.