From c107489f89324963a8fe70f18552609fe10cd258 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Thu, 25 Jun 2026 17:30:57 +0700 Subject: [PATCH] refactor(stock): drop symbol cache --- internal/modules/stock/handlers.go | 60 +++++++------ internal/modules/stock/income_events.go | 10 +-- internal/modules/stock/portfolio.go | 3 +- internal/modules/stock/prices.go | 3 +- internal/modules/stock/symbols.go | 50 ++--------- internal/modules/stock/symbols_test.go | 107 ++++++++---------------- 6 files changed, 76 insertions(+), 157 deletions(-) diff --git a/internal/modules/stock/handlers.go b/internal/modules/stock/handlers.go index d7d0d23..705b978 100644 --- a/internal/modules/stock/handlers.go +++ b/internal/modules/stock/handlers.go @@ -119,22 +119,21 @@ func (s *state) handleBuy(ctx context.Context, b *bot.Bot, update *models.Update return chathelper.Reply(ctx, b, update.Message, "Quantity must be a positive whole number.") } - resolved, err := ResolveSymbol(ctx, s.kv, s.prices, args[1]) + symbol, err := normalizeStockSymbol(args[1]) if err != nil { if errors.Is(err, ErrUnknownTicker) { return chathelper.Reply(ctx, b, update.Message, "Unknown stock ticker \""+strings.ToUpper(args[1])+"\".\n"+s.comingSoonMessage) } - log.Error("stock_resolve_symbol", "ticker", args[1], "err", err) - return chathelper.Reply(ctx, b, update.Message, "Could not look up that ticker. Try again later.") + return chathelper.Reply(ctx, b, update.Message, "Could not parse that ticker. Try again later.") } - price, err := s.prices.FetchPrice(ctx, resolved.Symbol) + price, err := s.prices.FetchPrice(ctx, symbol) if err != nil { if errors.Is(err, ErrNoPrice) { - return chathelper.Reply(ctx, b, update.Message, "No price available for "+resolved.Symbol+".") + return chathelper.Reply(ctx, b, update.Message, "No price available for "+symbol+".") } - log.Error("stock_fetch_price", "ticker", resolved.Symbol, "err", err) + log.Error("stock_fetch_price", "ticker", symbol, "err", err) return chathelper.Reply(ctx, b, update.Message, "Could not fetch price. Try again later.") } cost := float64(qty) * price @@ -151,13 +150,13 @@ func (s *state) handleBuy(ctx context.Context, b *bot.Bot, update *models.Update return chathelper.Reply(ctx, b, update.Message, "Insufficient VND. Need "+FormatVND(cost)+", have "+FormatVND(balance)+".") } - p.AddAsset(resolved.Symbol, qty) + p.AddAsset(symbol, qty) if err := SavePortfolio(ctx, s.kv, userID, p); err != nil { log.Error("stock_save_portfolio", "user", userID, "err", err) return chathelper.Reply(ctx, b, update.Message, "Could not save portfolio. Try again later.") } return chathelper.Reply(ctx, b, update.Message, - "Bought "+FormatStock(float64(qty))+" "+resolved.Symbol+ + "Bought "+FormatStock(float64(qty))+" "+symbol+ " @ "+FormatVND(price)+"\nCost: "+FormatVND(cost)+ "\nRemaining: "+FormatVND(p.Currency["VND"])) } @@ -177,24 +176,23 @@ func (s *state) handleSell(ctx context.Context, b *bot.Bot, update *models.Updat return chathelper.Reply(ctx, b, update.Message, "Quantity must be a positive whole number.") } - // Resolve + fetch price BEFORE taking the per-user lock. Mirrors handleBuy: + // Normalize + fetch price BEFORE taking the per-user lock. Mirrors handleBuy: // keeps the critical section to a fast Get→mutate→Put, and removes any need // for a rollback path (no in-memory mutation precedes the network call). - resolved, err := ResolveSymbol(ctx, s.kv, s.prices, args[1]) + symbol, err := normalizeStockSymbol(args[1]) if err != nil { if errors.Is(err, ErrUnknownTicker) { return chathelper.Reply(ctx, b, update.Message, "Unknown stock ticker \""+strings.ToUpper(args[1])+"\".") } - log.Error("stock_resolve_symbol", "ticker", args[1], "err", err) - return chathelper.Reply(ctx, b, update.Message, "Could not look up that ticker. Try again later.") + return chathelper.Reply(ctx, b, update.Message, "Could not parse that ticker. Try again later.") } - price, err := s.prices.FetchPrice(ctx, resolved.Symbol) + price, err := s.prices.FetchPrice(ctx, symbol) if err != nil { if errors.Is(err, ErrNoPrice) { - return chathelper.Reply(ctx, b, update.Message, "No price available for "+resolved.Symbol+".") + return chathelper.Reply(ctx, b, update.Message, "No price available for "+symbol+".") } - log.Error("stock_fetch_price", "ticker", resolved.Symbol, "err", err) + log.Error("stock_fetch_price", "ticker", symbol, "err", err) return chathelper.Reply(ctx, b, update.Message, "Could not fetch price. Try again later.") } @@ -205,10 +203,10 @@ func (s *state) handleSell(ctx context.Context, b *bot.Bot, update *models.Updat log.Error("stock_load_portfolio", "user", userID, "err", err) return chathelper.Reply(ctx, b, update.Message, "Could not load portfolio. Try again later.") } - ok, held := p.DeductAsset(resolved.Symbol, qty) + ok, held := p.DeductAsset(symbol, qty) if !ok { return chathelper.Reply(ctx, b, update.Message, - "Insufficient "+resolved.Symbol+". You have: "+FormatStock(float64(held))) + "Insufficient "+symbol+". You have: "+FormatStock(float64(held))) } revenue := float64(qty) * price p.AddCurrency("VND", revenue) @@ -217,7 +215,7 @@ func (s *state) handleSell(ctx context.Context, b *bot.Bot, update *models.Updat return chathelper.Reply(ctx, b, update.Message, "Could not save portfolio. Try again later.") } return chathelper.Reply(ctx, b, update.Message, - "Sold "+FormatStock(float64(qty))+" "+resolved.Symbol+ + "Sold "+FormatStock(float64(qty))+" "+symbol+ " @ "+FormatVND(price)+"\nRevenue: "+FormatVND(revenue)+ "\nRemaining: "+FormatVND(p.Currency["VND"])) } @@ -238,14 +236,13 @@ func (s *state) handleIncomeStock(ctx context.Context, b *bot.Bot, update *model return chathelper.Reply(ctx, b, update.Message, "Quantity must be a positive whole number.") } - resolved, err := ResolveSymbol(ctx, s.kv, s.prices, args[1]) + symbol, err := normalizeStockSymbol(args[1]) if err != nil { if errors.Is(err, ErrUnknownTicker) { return chathelper.Reply(ctx, b, update.Message, "Unknown stock ticker \""+strings.ToUpper(args[1])+"\".") } - log.Error("stock_resolve_symbol", "ticker", args[1], "err", err) - return chathelper.Reply(ctx, b, update.Message, "Could not look up that ticker. Try again later.") + return chathelper.Reply(ctx, b, update.Message, "Could not parse that ticker. Try again later.") } defer s.locks.Acquire(strconv.FormatInt(userID, 10))() @@ -255,19 +252,19 @@ func (s *state) handleIncomeStock(ctx context.Context, b *bot.Bot, update *model log.Error("stock_load_portfolio", "user", userID, "err", err) return chathelper.Reply(ctx, b, update.Message, "Could not load portfolio. Try again later.") } - held := p.Assets[resolved.Symbol] + held := p.Assets[symbol] if held == 0 { return chathelper.Reply(ctx, b, update.Message, - "You don't hold any "+resolved.Symbol+" to receive a stock dividend.") + "You don't hold any "+symbol+" to receive a stock dividend.") } - p.AddAsset(resolved.Symbol, qty) + p.AddAsset(symbol, qty) if err := SavePortfolio(ctx, s.kv, userID, p); err != nil { log.Error("stock_save_portfolio", "user", userID, "err", err) return chathelper.Reply(ctx, b, update.Message, "Could not save portfolio. Try again later.") } return chathelper.Reply(ctx, b, update.Message, - "Stock dividend: +"+FormatStock(float64(qty))+" "+resolved.Symbol+ - "\nHolding: "+FormatStock(float64(held))+" → "+FormatStock(float64(p.Assets[resolved.Symbol]))) + "Stock dividend: +"+FormatStock(float64(qty))+" "+symbol+ + "\nHolding: "+FormatStock(float64(held))+" → "+FormatStock(float64(p.Assets[symbol]))) } func (s *state) handleIncomeVND(ctx context.Context, b *bot.Bot, update *models.Update) error { @@ -286,14 +283,13 @@ func (s *state) handleIncomeVND(ctx context.Context, b *bot.Bot, update *models. return chathelper.Reply(ctx, b, update.Message, "Amount per share must be a positive number.") } - resolved, err := ResolveSymbol(ctx, s.kv, s.prices, args[1]) + symbol, err := normalizeStockSymbol(args[1]) if err != nil { if errors.Is(err, ErrUnknownTicker) { return chathelper.Reply(ctx, b, update.Message, "Unknown stock ticker \""+strings.ToUpper(args[1])+"\".") } - log.Error("stock_resolve_symbol", "ticker", args[1], "err", err) - return chathelper.Reply(ctx, b, update.Message, "Could not look up that ticker. Try again later.") + return chathelper.Reply(ctx, b, update.Message, "Could not parse that ticker. Try again later.") } defer s.locks.Acquire(strconv.FormatInt(userID, 10))() @@ -303,10 +299,10 @@ func (s *state) handleIncomeVND(ctx context.Context, b *bot.Bot, update *models. log.Error("stock_load_portfolio", "user", userID, "err", err) return chathelper.Reply(ctx, b, update.Message, "Could not load portfolio. Try again later.") } - held := p.Assets[resolved.Symbol] + held := p.Assets[symbol] if held == 0 { return chathelper.Reply(ctx, b, update.Message, - "You don't hold any "+resolved.Symbol+" to receive a cash dividend.") + "You don't hold any "+symbol+" to receive a cash dividend.") } total := amountPerShare * float64(held) p.AddCurrency("VND", total) @@ -315,7 +311,7 @@ func (s *state) handleIncomeVND(ctx context.Context, b *bot.Bot, update *models. return chathelper.Reply(ctx, b, update.Message, "Could not save portfolio. Try again later.") } return chathelper.Reply(ctx, b, update.Message, - "Cash dividend: "+FormatVND(amountPerShare)+" × "+FormatStock(float64(held))+" "+resolved.Symbol+ + "Cash dividend: "+FormatVND(amountPerShare)+" × "+FormatStock(float64(held))+" "+symbol+ " = "+FormatVND(total)+ "\nRemaining: "+FormatVND(p.Currency["VND"])) } diff --git a/internal/modules/stock/income_events.go b/internal/modules/stock/income_events.go index d35b855..afd49fb 100644 --- a/internal/modules/stock/income_events.go +++ b/internal/modules/stock/income_events.go @@ -80,9 +80,9 @@ var ( ) func (c *IncomeEventClient) FetchRecent(ctx context.Context, ticker string, since, until time.Time) ([]IncomeEvent, error) { - ticker = strings.ToUpper(strings.TrimSpace(ticker)) - if !tickerRe.MatchString(ticker) { - return nil, ErrUnknownTicker + ticker, err := normalizeStockSymbol(ticker) + if err != nil { + return nil, err } if strings.TrimSpace(c.URL) == "" { return nil, ErrIncomeEventClientNotConfigured @@ -341,11 +341,11 @@ func (s *state) handleIncomeEvents(ctx context.Context, b *bot.Bot, update *mode func (s *state) incomeEventSymbols(ctx context.Context, userID int64, args []string) ([]string, error) { if len(args) > 0 { - resolved, err := ResolveSymbol(ctx, s.kv, s.prices, args[0]) + symbol, err := normalizeStockSymbol(args[0]) if err != nil { return nil, err } - return []string{resolved.Symbol}, nil + return []string{symbol}, nil } p, err := LoadPortfolio(ctx, s.kv, userID, s.now().UnixMilli()) diff --git a/internal/modules/stock/portfolio.go b/internal/modules/stock/portfolio.go index c72f3fe..49c3b40 100644 --- a/internal/modules/stock/portfolio.go +++ b/internal/modules/stock/portfolio.go @@ -10,8 +10,7 @@ import ( ) // Portfolio is the per-user stock state. Currency is a map for forward- -// compat with USD/EUR (currently VND-only). Assets is a flat ticker→qty map -// — category lives in the symbol cache, not the portfolio. +// compat with USD/EUR (currently VND-only). Assets is a flat ticker→qty map. type Portfolio struct { Currency map[string]float64 `json:"currency"` Assets map[string]int64 `json:"assets"` diff --git a/internal/modules/stock/prices.go b/internal/modules/stock/prices.go index 4451490..c1fd4fc 100644 --- a/internal/modules/stock/prices.go +++ b/internal/modules/stock/prices.go @@ -142,8 +142,7 @@ func normalizeTickers(tickers []string) []string { return out } -// ErrNoPrice means no provider returned a usable price for the ticker. Used by -// symbol resolution to detect "is this a real ticker". +// ErrNoPrice means no provider returned a usable price for the ticker. var ErrNoPrice = errors.New("stock: no price available") type providerError struct { diff --git a/internal/modules/stock/symbols.go b/internal/modules/stock/symbols.go index 80ec28e..9520cac 100644 --- a/internal/modules/stock/symbols.go +++ b/internal/modules/stock/symbols.go @@ -1,62 +1,24 @@ package stock import ( - "context" "errors" - "fmt" "regexp" "strings" - - "github.com/tiennm99/miti99bot/internal/storage" ) -// tickerRe restricts tickers to ASCII alphanumeric, 1-16 chars. Stops -// Cyrillic / unicode-lookalike inputs from amplifying price-provider lookups, and -// guards the cache key alphabet (sym:) from oddities. +// tickerRe restricts stock tickers to ASCII alphanumeric, 1-16 chars. This +// keeps provider lookups predictable and rejects unicode-lookalike inputs. var tickerRe = regexp.MustCompile(`^[A-Z0-9]{1,16}$`) -// ResolvedSymbol is the cached entry written under "sym:". Category -// is currently always "stock" — crypto/gold/forex are upstream future-work. -type ResolvedSymbol struct { - Symbol string `json:"symbol"` - Category string `json:"category"` - Label string `json:"label"` -} - -// ErrUnknownTicker means the price provider has no price data for the given ticker — i.e. -// the symbol is not a tradeable VN stock as far as our source is concerned. +// ErrUnknownTicker means the user input is not a valid stock ticker shape. var ErrUnknownTicker = errors.New("stock: unknown ticker") -// ResolveSymbol returns the cached ResolvedSymbol if any, otherwise queries -// the price provider to validate the ticker and caches the result permanently. Tickers -// don't change; permanent caching is correct. -// // The empty-input case returns ErrUnknownTicker to keep the caller's branch // shape simple (one error path covers both empty + unknown). -func ResolveSymbol(ctx context.Context, kv storage.KVStore, prices *PriceClient, ticker string) (ResolvedSymbol, error) { +func normalizeStockSymbol(ticker string) (string, error) { ticker = strings.ToUpper(strings.TrimSpace(ticker)) if !tickerRe.MatchString(ticker) { - return ResolvedSymbol{}, ErrUnknownTicker + return "", ErrUnknownTicker } - cacheKey := "sym:" + ticker - - var cached ResolvedSymbol - if err := kv.GetJSON(ctx, cacheKey, &cached); err == nil { - return cached, nil - } else if !errors.Is(err, storage.ErrNotFound) { - return ResolvedSymbol{}, fmt.Errorf("stock: cache read %s: %w", ticker, err) - } - - // Cache miss → validate against the price provider by attempting a price fetch. - if _, err := prices.FetchPrice(ctx, ticker); err != nil { - if errors.Is(err, ErrNoPrice) { - return ResolvedSymbol{}, ErrUnknownTicker - } - return ResolvedSymbol{}, err - } - - resolved := ResolvedSymbol{Symbol: ticker, Category: "stock", Label: ticker} - // Cache write failure is non-fatal — next call will resolve again. - _ = kv.PutJSON(ctx, cacheKey, resolved) - return resolved, nil + return ticker, nil } diff --git a/internal/modules/stock/symbols_test.go b/internal/modules/stock/symbols_test.go index c2053a5..283a7b2 100644 --- a/internal/modules/stock/symbols_test.go +++ b/internal/modules/stock/symbols_test.go @@ -1,86 +1,49 @@ package stock import ( - "context" "errors" - "net/http" - "net/http/httptest" - "sync/atomic" "testing" - - "github.com/tiennm99/miti99bot/internal/storage" ) -func TestResolveSymbol_FirstTime_QueriesAndCaches(t *testing.T) { - kv := storage.NewMemoryKVStore() - - var hits int32 - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - atomic.AddInt32(&hits, 1) - _, _ = w.Write([]byte(`{"data":{"stockSymbol":"TCB","matchedPrice":24500}}`)) - })) - defer srv.Close() - prices := &PriceClient{URL: srv.URL} - - got, err := ResolveSymbol(context.Background(), kv, prices, "tcb") - if err != nil { - t.Fatalf("ResolveSymbol: %v", err) - } - if got.Symbol != "TCB" || got.Category != "stock" { - t.Errorf("resolved: got %+v, want {TCB stock TCB}", got) - } - if atomic.LoadInt32(&hits) != 1 { - t.Errorf("price hits: got %d, want 1", hits) +func TestNormalizeStockSymbol(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + {name: "uppercases", input: "tcb", want: "TCB"}, + {name: "trims", input: " fpt ", want: "FPT"}, + {name: "allows digits", input: "abc123", want: "ABC123"}, + {name: "allows sixteen chars", input: "abcdefghijklmnop", want: "ABCDEFGHIJKLMNOP"}, } - // Second call should hit the cache, not the price provider. - _, err = ResolveSymbol(context.Background(), kv, prices, "TCB") - if err != nil { - t.Fatalf("ResolveSymbol (cached): %v", err) - } - if atomic.LoadInt32(&hits) != 1 { - t.Errorf("price hits after cache: got %d, want 1 (cached)", hits) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := normalizeStockSymbol(tt.input) + if err != nil { + t.Fatalf("normalizeStockSymbol(%q): %v", tt.input, err) + } + if got != tt.want { + t.Errorf("normalizeStockSymbol(%q) = %q, want %q", tt.input, got, tt.want) + } + }) } } -func TestResolveSymbol_Unknown(t *testing.T) { - kv := storage.NewMemoryKVStore() - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`{"data":{"stockSymbol":"NOPE","matchedPrice":0}}`)) - })) - defer srv.Close() - prices := &PriceClient{URL: srv.URL} - - _, err := ResolveSymbol(context.Background(), kv, prices, "NOPE") - if !errors.Is(err, ErrUnknownTicker) { - t.Errorf("got %v, want ErrUnknownTicker", err) - } -} - -func TestResolveSymbol_EmptyInput(t *testing.T) { - _, err := ResolveSymbol(context.Background(), storage.NewMemoryKVStore(), &PriceClient{}, " ") - if !errors.Is(err, ErrUnknownTicker) { - t.Errorf("got %v, want ErrUnknownTicker for empty input", err) - } -} - -func TestResolveSymbol_NormalizesCase(t *testing.T) { - kv := storage.NewMemoryKVStore() - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // SSI endpoint should receive the upper-cased ticker. - if r.URL.Path != "/stock/FPT" { - t.Errorf("ticker not upper-cased in URL: %s", r.URL.Path) - } - _, _ = w.Write([]byte(`{"data":{"stockSymbol":"FPT","matchedPrice":120000}}`)) - })) - defer srv.Close() - prices := &PriceClient{URL: srv.URL} - - got, err := ResolveSymbol(context.Background(), kv, prices, " fpt ") - if err != nil { - t.Fatalf("ResolveSymbol: %v", err) - } - if got.Symbol != "FPT" { - t.Errorf("normalised: got %q, want FPT", got.Symbol) +func TestNormalizeStockSymbolRejectsInvalid(t *testing.T) { + for _, input := range []string{ + "", + " ", + "FPT.VN", + "FPT-VN", + "đxg", + "abcdefghijklmnopq", + } { + t.Run(input, func(t *testing.T) { + _, err := normalizeStockSymbol(input) + if !errors.Is(err, ErrUnknownTicker) { + t.Fatalf("normalizeStockSymbol(%q) error = %v, want ErrUnknownTicker", input, err) + } + }) } }