From 7610793f0a3a8efd547028437d2590a3085d2f82 Mon Sep 17 00:00:00 2001 From: Bartek Nowotarski Date: Wed, 19 Sep 2018 17:04:58 +0200 Subject: [PATCH] Feature flag to allow null ledger data in responses (#672) --- protocols/horizon/main.go | 20 ++++---- services/horizon/internal/actions_offer.go | 26 ++++++---- .../horizon/internal/actions_offer_test.go | 48 +++++++++++++++++++ services/horizon/internal/config.go | 4 ++ .../horizon/internal/resourceadapter/offer.go | 12 +++-- services/horizon/main.go | 38 ++++++++------- 6 files changed, 107 insertions(+), 41 deletions(-) diff --git a/protocols/horizon/main.go b/protocols/horizon/main.go index c5544724e6..c3bcef1d6e 100644 --- a/protocols/horizon/main.go +++ b/protocols/horizon/main.go @@ -177,16 +177,16 @@ type Offer struct { OfferMaker hal.Link `json:"offer_maker"` } `json:"_links"` - ID int64 `json:"id"` - PT string `json:"paging_token"` - Seller string `json:"seller"` - Selling Asset `json:"selling"` - Buying Asset `json:"buying"` - Amount string `json:"amount"` - PriceR Price `json:"price_r"` - Price string `json:"price"` - LastModifiedLedger int32 `json:"last_modified_ledger"` - LastModifiedTime time.Time `json:"last_modified_time"` + ID int64 `json:"id"` + PT string `json:"paging_token"` + Seller string `json:"seller"` + Selling Asset `json:"selling"` + Buying Asset `json:"buying"` + Amount string `json:"amount"` + PriceR Price `json:"price_r"` + Price string `json:"price"` + LastModifiedLedger int32 `json:"last_modified_ledger"` + LastModifiedTime *time.Time `json:"last_modified_time"` } func (this Offer) PagingToken() string { diff --git a/services/horizon/internal/actions_offer.go b/services/horizon/internal/actions_offer.go index b0172f3d2f..0f0f023bfb 100644 --- a/services/horizon/internal/actions_offer.go +++ b/services/horizon/internal/actions_offer.go @@ -50,13 +50,18 @@ func (action *OffersByAccountAction) SSE(stream sse.Stream) { stream.SetLimit(int(action.PageQuery.Limit)) for _, record := range action.Records[stream.SentCount():] { ledger, found := action.Ledgers.Records[record.Lastmodified] + ledgerPtr := &ledger if !found { - msg := fmt.Sprintf("could not find ledger data for sequence %d", record.Lastmodified) - stream.Err(errors.New(msg)) - return + if action.App.config.AllowEmptyLedgerDataResponses { + ledgerPtr = nil + } else { + msg := fmt.Sprintf("could not find ledger data for sequence %d", record.Lastmodified) + stream.Err(errors.New(msg)) + return + } } var res horizon.Offer - resourceadapter.PopulateOffer(action.R.Context(), &res, record, ledger) + resourceadapter.PopulateOffer(action.R.Context(), &res, record, ledgerPtr) stream.Send(sse.Event{ID: res.PagingToken(), Data: res}) } }, @@ -87,14 +92,19 @@ func (action *OffersByAccountAction) loadRecords() { func (action *OffersByAccountAction) loadPage() { for _, record := range action.Records { ledger, found := action.Ledgers.Records[record.Lastmodified] + ledgerPtr := &ledger if !found { - msg := fmt.Sprintf("could not find ledger data for sequence %d", record.Lastmodified) - action.Err = errors.New(msg) - return + if action.App.config.AllowEmptyLedgerDataResponses { + ledgerPtr = nil + } else { + msg := fmt.Sprintf("could not find ledger data for sequence %d", record.Lastmodified) + action.Err = errors.New(msg) + return + } } var res horizon.Offer - resourceadapter.PopulateOffer(action.R.Context(), &res, record, ledger) + resourceadapter.PopulateOffer(action.R.Context(), &res, record, ledgerPtr) action.Page.Add(res) } diff --git a/services/horizon/internal/actions_offer_test.go b/services/horizon/internal/actions_offer_test.go index 3808a6bf1b..5d3b3e74ae 100644 --- a/services/horizon/internal/actions_offer_test.go +++ b/services/horizon/internal/actions_offer_test.go @@ -3,6 +3,8 @@ package horizon import ( "testing" "time" + + "github.com/stellar/go/services/horizon/internal/test" ) func TestOfferActions_Index(t *testing.T) { @@ -26,3 +28,49 @@ func TestOfferActions_Index(t *testing.T) { ht.Assert.EqualValues(5, records[2]["last_modified_ledger"]) } } + +func TestOfferActions_IndexNoLedgerData(t *testing.T) { + ht := StartHTTPTest(t, "trades") + defer ht.Finish() + + // Remove ledger data + _, err := ht.App.HistoryQ().ExecRaw("DELETE FROM history_ledgers WHERE sequence=?", 7) + ht.Assert.NoError(err) + + w := ht.Get( + "/accounts/GA5WBPYA5Y4WAEHXWR2UKO2UO4BUGHUQ74EUPKON2QHV4WRHOIRNKKH2/offers", + ) + + ht.Assert.Equal(500, w.Code) +} + +func TestOfferActions_IndexNoLedgerDataFeatureFlag(t *testing.T) { + ht := StartHTTPTest(t, "trades") + defer ht.Finish() + + // Ugly but saves us time needed to change each `StartHTTPTest` occurence. + appConfig := NewTestConfig() + appConfig.AllowEmptyLedgerDataResponses = true + var err error + ht.App, err = NewApp(appConfig) + ht.Assert.NoError(err) + ht.RH = test.NewRequestHelper(ht.App.web.router) + + // Remove ledger data + _, err = ht.App.HistoryQ().ExecRaw("DELETE FROM history_ledgers WHERE sequence=?", 5) + ht.Assert.NoError(err) + + w := ht.Get( + "/accounts/GA5WBPYA5Y4WAEHXWR2UKO2UO4BUGHUQ74EUPKON2QHV4WRHOIRNKKH2/offers", + ) + + if ht.Assert.Equal(200, w.Code) { + ht.Assert.PageOf(3, w.Body) + + //test last modified timestamp + var records []map[string]interface{} + ht.UnmarshalPage(w.Body, &records) + ht.Assert.NotEmpty(records[2]["last_modified_ledger"]) + ht.Assert.Nil(records[2]["last_modified_time"]) + } +} diff --git a/services/horizon/internal/config.go b/services/horizon/internal/config.go index 0a1f069136..53aae371c3 100644 --- a/services/horizon/internal/config.go +++ b/services/horizon/internal/config.go @@ -45,4 +45,8 @@ type Config struct { // Disabling it will save CPU when ingesting ledgers full of many different // assets related operations. DisableAssetStats bool + // AllowEmptyLedgerDataResponses is a feature flag that sets unavailable + // ledger data (like `close_time`) to `nil` instead of returning 500 error + // response. + AllowEmptyLedgerDataResponses bool } diff --git a/services/horizon/internal/resourceadapter/offer.go b/services/horizon/internal/resourceadapter/offer.go index 96fb9d6ab3..a62c012f4e 100644 --- a/services/horizon/internal/resourceadapter/offer.go +++ b/services/horizon/internal/resourceadapter/offer.go @@ -4,15 +4,15 @@ import ( "context" "github.com/stellar/go/amount" + . "github.com/stellar/go/protocols/horizon" "github.com/stellar/go/services/horizon/internal/assets" "github.com/stellar/go/services/horizon/internal/db2/core" + "github.com/stellar/go/services/horizon/internal/db2/history" "github.com/stellar/go/services/horizon/internal/httpx" - . "github.com/stellar/go/protocols/horizon" "github.com/stellar/go/support/render/hal" - "github.com/stellar/go/services/horizon/internal/db2/history" ) -func PopulateOffer(ctx context.Context, dest *Offer, row core.Offer, ledger history.Ledger) { +func PopulateOffer(ctx context.Context, dest *Offer, row core.Offer, ledger *history.Ledger) { dest.ID = row.OfferID dest.PT = row.PagingToken() dest.Seller = row.SellerID @@ -31,9 +31,11 @@ func PopulateOffer(ctx context.Context, dest *Offer, row core.Offer, ledger hist Issuer: row.SellingIssuer.String, } dest.LastModifiedLedger = row.Lastmodified - dest.LastModifiedTime = ledger.ClosedAt + if ledger != nil { + dest.LastModifiedTime = &ledger.ClosedAt + } lb := hal.LinkBuilder{httpx.BaseURL(ctx)} dest.Links.Self = lb.Linkf("/offers/%d", row.OfferID) dest.Links.OfferMaker = lb.Linkf("/accounts/%s", row.SellerID) return -} \ No newline at end of file +} diff --git a/services/horizon/main.go b/services/horizon/main.go index 4297ed9cde..770bb83d00 100644 --- a/services/horizon/main.go +++ b/services/horizon/main.go @@ -45,6 +45,7 @@ func init() { viper.BindEnv("history-stale-threshold", "HISTORY_STALE_THRESHOLD") viper.BindEnv("skip-cursor-update", "SKIP_CURSOR_UPDATE") viper.BindEnv("disable-asset-stats", "DISABLE_ASSET_STATS") + viper.BindEnv("allow-empty-ledger-data-responses", "ALLOW_EMPTY_LEDGER_DATA_RESPONSES") rootCmd = &cobra.Command{ Use: "horizon", @@ -214,23 +215,24 @@ func initConfig() { } config = horizon.Config{ - DatabaseURL: viper.GetString("db-url"), - StellarCoreDatabaseURL: viper.GetString("stellar-core-db-url"), - StellarCoreURL: viper.GetString("stellar-core-url"), - Port: viper.GetInt("port"), - RateLimit: throttled.PerHour(viper.GetInt("per-hour-rate-limit")), - RedisURL: viper.GetString("redis-url"), - FriendbotURL: friendbotURL, - LogLevel: ll, - SentryDSN: viper.GetString("sentry-dsn"), - LogglyToken: viper.GetString("loggly-token"), - LogglyTag: viper.GetString("loggly-tag"), - TLSCert: cert, - TLSKey: key, - Ingest: viper.GetBool("ingest"), - HistoryRetentionCount: uint(viper.GetInt("history-retention-count")), - StaleThreshold: uint(viper.GetInt("history-stale-threshold")), - SkipCursorUpdate: viper.GetBool("skip-cursor-update"), - DisableAssetStats: viper.GetBool("disable-asset-stats"), + DatabaseURL: viper.GetString("db-url"), + StellarCoreDatabaseURL: viper.GetString("stellar-core-db-url"), + StellarCoreURL: viper.GetString("stellar-core-url"), + Port: viper.GetInt("port"), + RateLimit: throttled.PerHour(viper.GetInt("per-hour-rate-limit")), + RedisURL: viper.GetString("redis-url"), + FriendbotURL: friendbotURL, + LogLevel: ll, + SentryDSN: viper.GetString("sentry-dsn"), + LogglyToken: viper.GetString("loggly-token"), + LogglyTag: viper.GetString("loggly-tag"), + TLSCert: cert, + TLSKey: key, + Ingest: viper.GetBool("ingest"), + HistoryRetentionCount: uint(viper.GetInt("history-retention-count")), + StaleThreshold: uint(viper.GetInt("history-stale-threshold")), + SkipCursorUpdate: viper.GetBool("skip-cursor-update"), + DisableAssetStats: viper.GetBool("disable-asset-stats"), + AllowEmptyLedgerDataResponses: viper.GetBool("allow-empty-ledger-data-responses"), } }