From 399858d09b97272a8c457d043206e7549ba4d3e2 Mon Sep 17 00:00:00 2001 From: hackerESQ Date: Tue, 28 Jan 2025 19:35:15 -0600 Subject: [PATCH 1/5] fix: strongly type symbol for market data and quote --- app/Console/Commands/RefreshMarketData.php | 7 ++++++- .../MarketData/AlphaVantageMarketData.php | 14 +++++++++----- app/Interfaces/MarketData/FallbackInterface.php | 1 + app/Interfaces/MarketData/FinnhubMarketData.php | 14 +++++++++----- app/Interfaces/MarketData/Types/Quote.php | 4 ++-- app/Interfaces/MarketData/YahooMarketData.php | 14 +++++++++++--- 6 files changed, 38 insertions(+), 16 deletions(-) diff --git a/app/Console/Commands/RefreshMarketData.php b/app/Console/Commands/RefreshMarketData.php index 68325b1..5518581 100644 --- a/app/Console/Commands/RefreshMarketData.php +++ b/app/Console/Commands/RefreshMarketData.php @@ -7,6 +7,7 @@ use App\Models\Holding; use App\Models\MarketData; use Illuminate\Console\Command; +use Illuminate\Support\Facades\Log; class RefreshMarketData extends Command { @@ -57,7 +58,11 @@ public function handle() foreach ($holdings->get() as $holding) { $this->line('Refreshing '.$holding->symbol); - MarketData::getMarketData($holding->symbol, $force); + try { + MarketData::getMarketData($holding->symbol, $force); + } catch (\Throwable $e) { + Log::error('Could not refresh '.$holding->symbol.' ('.$e->getMessage().')'); + } } } } diff --git a/app/Interfaces/MarketData/AlphaVantageMarketData.php b/app/Interfaces/MarketData/AlphaVantageMarketData.php index a139a60..524a587 100644 --- a/app/Interfaces/MarketData/AlphaVantageMarketData.php +++ b/app/Interfaces/MarketData/AlphaVantageMarketData.php @@ -18,7 +18,15 @@ class AlphaVantageMarketData implements MarketDataInterface public function exists(string $symbol): bool { - return $this->quote($symbol)->isNotEmpty(); + try { + $this->quote($symbol); + + return true; + + } catch (\Throwable $e) { + + return false; + } } public function quote(string $symbol): Quote @@ -26,10 +34,6 @@ public function quote(string $symbol): Quote $quote = Alphavantage::core()->quoteEndpoint($symbol); $quote = Arr::get($quote, 'Global Quote', []); - if (empty($quote)) { - return new Quote; - } - $fundamental = cache()->remember( 'av-symbol-'.$symbol, 1440, diff --git a/app/Interfaces/MarketData/FallbackInterface.php b/app/Interfaces/MarketData/FallbackInterface.php index 194758a..ec6bcad 100644 --- a/app/Interfaces/MarketData/FallbackInterface.php +++ b/app/Interfaces/MarketData/FallbackInterface.php @@ -20,6 +20,7 @@ public function __call($method, $arguments) $provider = trim($provider); try { + Log::warning("Calling method {$method} ({$provider})"); if (! in_array($provider, array_keys(config('investbrain.interfaces', [])))) { diff --git a/app/Interfaces/MarketData/FinnhubMarketData.php b/app/Interfaces/MarketData/FinnhubMarketData.php index 8c16554..244fa14 100644 --- a/app/Interfaces/MarketData/FinnhubMarketData.php +++ b/app/Interfaces/MarketData/FinnhubMarketData.php @@ -28,17 +28,21 @@ public function __construct() public function exists(string $symbol): bool { - return $this->quote($symbol)->isNotEmpty(); + try { + $this->quote($symbol); + + return true; + + } catch (\Throwable $e) { + + return false; + } } public function quote(string $symbol): Quote { $quote = $this->client->quote($symbol); - if (empty($quote)) { - return new Quote; - } - $fundamental = cache()->remember( 'fh-symbol-'.$symbol, 1440, diff --git a/app/Interfaces/MarketData/Types/Quote.php b/app/Interfaces/MarketData/Types/Quote.php index f8db045..ae49470 100644 --- a/app/Interfaces/MarketData/Types/Quote.php +++ b/app/Interfaces/MarketData/Types/Quote.php @@ -9,7 +9,7 @@ class Quote extends MarketDataType { - public function setName($name): self + public function setName(string $name): self { $this->items['name'] = (string) $name; @@ -21,7 +21,7 @@ public function getName(): string return $this->items['name'] ?? ''; } - public function setSymbol($symbol): self + public function setSymbol(string $symbol): self { $this->items['symbol'] = (string) $symbol; diff --git a/app/Interfaces/MarketData/YahooMarketData.php b/app/Interfaces/MarketData/YahooMarketData.php index cb3c48f..c9830e1 100644 --- a/app/Interfaces/MarketData/YahooMarketData.php +++ b/app/Interfaces/MarketData/YahooMarketData.php @@ -26,7 +26,15 @@ public function __construct() public function exists(string $symbol): bool { - return $this->quote($symbol)->isNotEmpty(); + try { + $this->quote($symbol); + + return true; + + } catch (\Throwable $e) { + + return false; + } } public function quote(string $symbol): Quote @@ -34,8 +42,8 @@ public function quote(string $symbol): Quote $quote = $this->client->getQuote($symbol); - if (empty($quote)) { - return collect(); + if (is_null($quote)) { + throw new \Exception('Symbol ('.$symbol.') does not exist'); } return new Quote([ From eac5de0d4af05b1c3057efcf02d612d77c165428 Mon Sep 17 00:00:00 2001 From: hackerESQ Date: Tue, 28 Jan 2025 19:46:37 -0600 Subject: [PATCH 2/5] fix: adds appropriate return types --- app/Models/Transaction.php | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/app/Models/Transaction.php b/app/Models/Transaction.php index f4c2ebb..b734633 100644 --- a/app/Models/Transaction.php +++ b/app/Models/Transaction.php @@ -4,10 +4,13 @@ namespace App\Models; +use Illuminate\Contracts\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Database\Eloquent\Concerns\HasUuids; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\Relations\BelongsTo; +use Illuminate\Database\Eloquent\Relations\HasOne; use Illuminate\Support\Arr; class Transaction extends Model @@ -79,7 +82,7 @@ protected function symbol(): Attribute * * @return void */ - public function market_data() + public function market_data(): HasOne { return $this->hasOne(MarketData::class, 'symbol', 'symbol'); } @@ -89,12 +92,12 @@ public function market_data() * * @return void */ - public function portfolio() + public function portfolio(): BelongsTo { return $this->belongsTo(Portfolio::class); } - public function scopeWithMarketData($query) + public function scopeWithMarketData($query): Builder { return $query->withAggregate('market_data', 'name') ->withAggregate('market_data', 'market_value') @@ -104,32 +107,32 @@ public function scopeWithMarketData($query) ->join('market_data', 'transactions.symbol', 'market_data.symbol'); } - public function scopePortfolio($query, $portfolio) + public function scopePortfolio($query, $portfolio): Builder { return $query->where('portfolio_id', $portfolio); } - public function scopeSymbol($query, $symbol) + public function scopeSymbol($query, $symbol): Builder { return $query->where('symbol', $symbol); } - public function scopeBuy($query) + public function scopeBuy($query): Builder { return $query->where('transaction_type', 'BUY'); } - public function scopeSell($query) + public function scopeSell($query): Builder { return $query->where('transaction_type', 'SELL'); } - public function scopeBeforeDate($query, $date) + public function scopeBeforeDate($query, $date): Builder { return $query->whereDate('date', '<=', $date); } - public function scopeMyTransactions() + public function scopeMyTransactions(): Builder { return $this->whereHas('portfolio', function ($query) { $query->whereHas('users', function ($query) { @@ -138,17 +141,15 @@ public function scopeMyTransactions() }); } - public function refreshMarketData() + public function refreshMarketData(): void { - return MarketData::getMarketData($this->attributes['symbol']); + MarketData::getMarketData($this->attributes['symbol']); } /** * Writes average cost basis to a sale transaction - * - * @return Transaction */ - public function ensureCostBasisIsAddedToSale() + public function ensureCostBasisIsAddedToSale(): Transaction { $average_cost_basis = Transaction::where([ 'portfolio_id' => $this->portfolio_id, @@ -164,10 +165,8 @@ public function ensureCostBasisIsAddedToSale() /** * Syncs the holding related to this transaction - * - * @return void */ - public function syncToHolding() + public function syncToHolding(): void { // if symbol name changed, sync previous symbol too From 0f135f4024583773e5e73f91f5736db95c3b7d4a Mon Sep 17 00:00:00 2001 From: hackerESQ Date: Tue, 28 Jan 2025 19:48:20 -0600 Subject: [PATCH 3/5] fix: gracefully fail if symbol not found --- app/Http/ApiControllers/MarketDataController.php | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/app/Http/ApiControllers/MarketDataController.php b/app/Http/ApiControllers/MarketDataController.php index f128a68..a815ea6 100644 --- a/app/Http/ApiControllers/MarketDataController.php +++ b/app/Http/ApiControllers/MarketDataController.php @@ -14,8 +14,17 @@ class MarketDataController extends ApiController public function show(Request $request, string $symbol) { - return MarketDataResource::make( - MarketData::getMarketData($symbol) - ); + try { + + return MarketDataResource::make( + MarketData::getMarketData($symbol) + ); + } catch (\Throwable $e) { + + return response([ + 'message' => 'Symbol '.$symbol.' not found.', + ], 404); + } + } } From 4ece09368e056ecf6b6330a31b2ccaf87bc570b6 Mon Sep 17 00:00:00 2001 From: hackerESQ Date: Tue, 28 Jan 2025 20:32:43 -0600 Subject: [PATCH 4/5] fix: upgrade the exists() market data provider method --- .../MarketData/AlphaVantageMarketData.php | 10 +----- .../MarketData/FallbackInterface.php | 7 ++++ .../MarketData/FinnhubMarketData.php | 10 +----- app/Interfaces/MarketData/YahooMarketData.php | 34 ++++++------------- app/Models/Transaction.php | 1 + app/Rules/SymbolValidationRule.php | 3 +- tests/FallbackInterfaceTest.php | 26 ++++++++++++++ 7 files changed, 49 insertions(+), 42 deletions(-) diff --git a/app/Interfaces/MarketData/AlphaVantageMarketData.php b/app/Interfaces/MarketData/AlphaVantageMarketData.php index 524a587..17c4856 100644 --- a/app/Interfaces/MarketData/AlphaVantageMarketData.php +++ b/app/Interfaces/MarketData/AlphaVantageMarketData.php @@ -18,15 +18,7 @@ class AlphaVantageMarketData implements MarketDataInterface public function exists(string $symbol): bool { - try { - $this->quote($symbol); - - return true; - - } catch (\Throwable $e) { - - return false; - } + return (bool) $this->quote($symbol); } public function quote(string $symbol): Quote diff --git a/app/Interfaces/MarketData/FallbackInterface.php b/app/Interfaces/MarketData/FallbackInterface.php index ec6bcad..ff25749 100644 --- a/app/Interfaces/MarketData/FallbackInterface.php +++ b/app/Interfaces/MarketData/FallbackInterface.php @@ -39,6 +39,13 @@ public function __call($method, $arguments) } } + // don't need to throw error if calling exists + if ($method == 'exists') { + + // symbol prob just doesn't exist + return false; + } + throw new \Exception("Could not get market data: {$this->latest_error}"); } } diff --git a/app/Interfaces/MarketData/FinnhubMarketData.php b/app/Interfaces/MarketData/FinnhubMarketData.php index 244fa14..8cd561e 100644 --- a/app/Interfaces/MarketData/FinnhubMarketData.php +++ b/app/Interfaces/MarketData/FinnhubMarketData.php @@ -28,15 +28,7 @@ public function __construct() public function exists(string $symbol): bool { - try { - $this->quote($symbol); - - return true; - - } catch (\Throwable $e) { - - return false; - } + return (bool) $this->quote($symbol); } public function quote(string $symbol): Quote diff --git a/app/Interfaces/MarketData/YahooMarketData.php b/app/Interfaces/MarketData/YahooMarketData.php index c9830e1..3261a2f 100644 --- a/app/Interfaces/MarketData/YahooMarketData.php +++ b/app/Interfaces/MarketData/YahooMarketData.php @@ -26,15 +26,7 @@ public function __construct() public function exists(string $symbol): bool { - try { - $this->quote($symbol); - - return true; - - } catch (\Throwable $e) { - - return false; - } + return (bool) $this->quote($symbol); } public function quote(string $symbol): Quote @@ -42,22 +34,18 @@ public function quote(string $symbol): Quote $quote = $this->client->getQuote($symbol); - if (is_null($quote)) { - throw new \Exception('Symbol ('.$symbol.') does not exist'); - } - return new Quote([ - 'name' => $quote->getLongName() ?? $quote->getShortName(), + 'name' => $quote?->getLongName() ?? $quote?->getShortName(), 'symbol' => $symbol, - 'market_value' => $quote->getRegularMarketPrice(), - 'fifty_two_week_high' => $quote->getFiftyTwoWeekHigh(), - 'fifty_two_week_low' => $quote->getFiftyTwoWeekLow(), - 'forward_pe' => $quote->getForwardPE(), - 'trailing_pe' => $quote->getTrailingPE(), - 'market_cap' => $quote->getMarketCap(), - 'book_value' => $quote->getBookValue(), - 'last_dividend_date' => $quote->getDividendDate(), - 'dividend_yield' => $quote->getTrailingAnnualDividendYield() * 100, + 'market_value' => $quote?->getRegularMarketPrice(), + 'fifty_two_week_high' => $quote?->getFiftyTwoWeekHigh(), + 'fifty_two_week_low' => $quote?->getFiftyTwoWeekLow(), + 'forward_pe' => $quote?->getForwardPE(), + 'trailing_pe' => $quote?->getTrailingPE(), + 'market_cap' => $quote?->getMarketCap(), + 'book_value' => $quote?->getBookValue(), + 'last_dividend_date' => $quote?->getDividendDate(), + 'dividend_yield' => $quote?->getTrailingAnnualDividendYield() * 100, ]); } diff --git a/app/Models/Transaction.php b/app/Models/Transaction.php index b734633..0962232 100644 --- a/app/Models/Transaction.php +++ b/app/Models/Transaction.php @@ -143,6 +143,7 @@ public function scopeMyTransactions(): Builder public function refreshMarketData(): void { + MarketData::getMarketData($this->attributes['symbol']); } diff --git a/app/Rules/SymbolValidationRule.php b/app/Rules/SymbolValidationRule.php index c01e844..428b6b9 100644 --- a/app/Rules/SymbolValidationRule.php +++ b/app/Rules/SymbolValidationRule.php @@ -29,12 +29,13 @@ public function validate(string $attribute, mixed $value, \Closure $fail): void { $this->symbol = $value; + // Check if the symbol exists in the Market Data table first (avoid API call) if (MarketData::find($this->symbol)) { return; } - // Check if the symbol exists in the Market Data table first (avoid API call) + // Then check against market data provider if (! app(MarketDataInterface::class)->exists($value)) { $fail('The symbol provided ('.$this->symbol.') is not valid'); } diff --git a/tests/FallbackInterfaceTest.php b/tests/FallbackInterfaceTest.php index 6053fe2..484fae0 100644 --- a/tests/FallbackInterfaceTest.php +++ b/tests/FallbackInterfaceTest.php @@ -77,4 +77,30 @@ public function test_all_providers_fail() Log::shouldHaveReceived('warning')->with('Failed calling method quote (yahoo): Yahoo failed'); Log::shouldHaveReceived('warning')->with('Failed calling method quote (alpha): Alpha failed'); } + + public function test_exists_method_fails_without_exception() + { + config()->set('investbrain.provider', 'yahoo,alpha'); + config()->set('investbrain.interfaces', [ + 'yahoo' => YahooMarketData::class, + 'alphavantage' => AlphaVantageMarketData::class, + ]); + + $yahooMock = Mockery::mock(YahooMarketData::class); + $yahooMock->shouldReceive('exists') + ->andThrow(new \Exception('Yahoo failed')); + + $alphaMock = Mockery::mock(AlphaVantageMarketData::class); + $alphaMock->shouldReceive('exists') + ->andThrow(new \Exception('Alpha failed')); + + $this->app->instance(YahooMarketData::class, $yahooMock); + $this->app->instance(AlphaVantageMarketData::class, $alphaMock); + + $fallbackInterface = new FallbackInterface; + + $result = $fallbackInterface->exists('ZZZ'); + + $this->assertFalse($result); + } } From 8c94fbf299a6e8e182b71f1ce65b55d9ec1382b7 Mon Sep 17 00:00:00 2001 From: hackerESQ Date: Tue, 28 Jan 2025 20:33:28 -0600 Subject: [PATCH 5/5] fix: ensure failed exists() is boolean --- tests/FallbackInterfaceTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/FallbackInterfaceTest.php b/tests/FallbackInterfaceTest.php index 484fae0..99282d2 100644 --- a/tests/FallbackInterfaceTest.php +++ b/tests/FallbackInterfaceTest.php @@ -101,6 +101,7 @@ public function test_exists_method_fails_without_exception() $result = $fallbackInterface->exists('ZZZ'); + $this->assertIsBool($result); $this->assertFalse($result); } }