From c4480690a35ed65be08520f3aaeaa43b183d645c Mon Sep 17 00:00:00 2001 From: jerremyng Date: Fri, 9 Jan 2026 07:58:43 +0000 Subject: [PATCH] fix idx bug and add tests --- app/Models/Plugin.php | 158 ++++++++++++------------------- tests/Unit/Models/PluginTest.php | 29 ++++++ 2 files changed, 89 insertions(+), 98 deletions(-) diff --git a/app/Models/Plugin.php b/app/Models/Plugin.php index c791333..524f26a 100644 --- a/app/Models/Plugin.php +++ b/app/Models/Plugin.php @@ -153,105 +153,67 @@ class Plugin extends Model public function updateDataPayload(): void { - if ($this->data_strategy === 'polling' && $this->polling_url) { - - $headers = ['User-Agent' => 'usetrmnl/byos_laravel', 'Accept' => 'application/json']; - - if ($this->polling_header) { - // Resolve Liquid variables in the polling header - $resolvedHeader = $this->resolveLiquidVariables($this->polling_header); - $headerLines = explode("\n", mb_trim($resolvedHeader)); - foreach ($headerLines as $line) { - $parts = explode(':', $line, 2); - if (count($parts) === 2) { - $headers[mb_trim($parts[0])] = mb_trim($parts[1]); - } - } - } - - // Resolve Liquid variables in the entire polling_url field first, then split by newline - $resolvedPollingUrls = $this->resolveLiquidVariables($this->polling_url); - $urls = array_filter( - array_map('trim', explode("\n", $resolvedPollingUrls)), - fn ($url): bool => ! empty($url) - ); - - // If only one URL, use the original logic without nesting - if (count($urls) === 1) { - $url = reset($urls); - $httpRequest = Http::withHeaders($headers); - - if ($this->polling_verb === 'post' && $this->polling_body) { - // Resolve Liquid variables in the polling body - $resolvedBody = $this->resolveLiquidVariables($this->polling_body); - $httpRequest = $httpRequest->withBody($resolvedBody); - } - - // URL is already resolved, use it directly - $resolvedUrl = $url; - - try { - // Make the request based on the verb - $httpResponse = $this->polling_verb === 'post' ? $httpRequest->post($resolvedUrl) : $httpRequest->get($resolvedUrl); - - $response = $this->parseResponse($httpResponse); - - $this->update([ - 'data_payload' => $response, - 'data_payload_updated_at' => now(), - ]); - } catch (Exception $e) { - Log::warning("Failed to fetch data from URL {$resolvedUrl}: ".$e->getMessage()); - $this->update([ - 'data_payload' => ['error' => 'Failed to fetch data'], - 'data_payload_updated_at' => now(), - ]); - } - - return; - } - - // Multiple URLs - use nested response logic - $combinedResponse = []; - - foreach ($urls as $index => $url) { - $httpRequest = Http::withHeaders($headers); - - if ($this->polling_verb === 'post' && $this->polling_body) { - // Resolve Liquid variables in the polling body - $resolvedBody = $this->resolveLiquidVariables($this->polling_body); - $httpRequest = $httpRequest->withBody($resolvedBody); - } - - // URL is already resolved, use it directly - $resolvedUrl = $url; - - try { - // Make the request based on the verb - $httpResponse = $this->polling_verb === 'post' ? $httpRequest->post($resolvedUrl) : $httpRequest->get($resolvedUrl); - - $response = $this->parseResponse($httpResponse); - - // Check if response is an array at root level - if (array_keys($response) === range(0, count($response) - 1)) { - // Response is a sequential array, nest under .data - $combinedResponse["IDX_{$index}"] = ['data' => $response]; - } else { - // Response is an object or associative array, keep as is - $combinedResponse["IDX_{$index}"] = $response; - } - } catch (Exception $e) { - // Log error and continue with other URLs - Log::warning("Failed to fetch data from URL {$resolvedUrl}: ".$e->getMessage()); - $combinedResponse["IDX_{$index}"] = ['error' => 'Failed to fetch data']; - } - } - - $this->update([ - 'data_payload' => $combinedResponse, - 'data_payload_updated_at' => now(), - ]); + if ($this->data_strategy !== 'polling' || !$this->polling_url) { + return; } + $headers = ['User-Agent' => 'usetrmnl/byos_laravel', 'Accept' => 'application/json']; + + // resolve headers + if ($this->polling_header) { + $resolvedHeader = $this->resolveLiquidVariables($this->polling_header); + $headerLines = explode("\n", mb_trim($resolvedHeader)); + foreach ($headerLines as $line) { + $parts = explode(':', $line, 2); + if (count($parts) === 2) { + $headers[mb_trim($parts[0])] = mb_trim($parts[1]); + } + } + } + + // resolve and clean URLs + $resolvedPollingUrls = $this->resolveLiquidVariables($this->polling_url); + $urls = array_values(array_filter( // array_values ensures 0, 1, 2... + array_map('trim', explode("\n", $resolvedPollingUrls)), + fn ($url): bool => filled($url) + )); + + $combinedResponse = []; + + // Loop through all URLs (Handles 1 or many) + foreach ($urls as $index => $url) { + $httpRequest = Http::withHeaders($headers); + + if ($this->polling_verb === 'post' && $this->polling_body) { + $resolvedBody = $this->resolveLiquidVariables($this->polling_body); + $httpRequest = $httpRequest->withBody($resolvedBody); + } + + try { + $httpResponse = ($this->polling_verb === 'post') + ? $httpRequest->post($url) + : $httpRequest->get($url); + + $response = $this->parseResponse($httpResponse); + + // Nest if it's a sequential array + if (array_keys($response) === range(0, count($response) - 1)) { + $combinedResponse["IDX_{$index}"] = ['data' => $response]; + } else { + $combinedResponse["IDX_{$index}"] = $response; + } + } catch (Exception $e) { + Log::warning("Failed to fetch data from URL {$url}: ".$e->getMessage()); + $combinedResponse["IDX_{$index}"] = ['error' => 'Failed to fetch data']; + } + } + + // unwrap IDX_0 if only one URL + $finalPayload = (count($urls) === 1) ? reset($combinedResponse) : $combinedResponse; + + $this->update([ + 'data_payload' => $finalPayload, + 'data_payload_updated_at' => now(), + ]); } private function parseResponse(Response $httpResponse): array diff --git a/tests/Unit/Models/PluginTest.php b/tests/Unit/Models/PluginTest.php index 2771f81..aa9a28e 100644 --- a/tests/Unit/Models/PluginTest.php +++ b/tests/Unit/Models/PluginTest.php @@ -99,6 +99,35 @@ test('updateDataPayload handles multiple URLs with IDX_ prefixes', function (): expect($plugin->data_payload['IDX_2'])->toBe(['headline' => 'test']); }); +test('updateDataPayload skips empty lines in polling_url and maintains sequential IDX keys', function (): void { + $plugin = Plugin::factory()->create([ + 'data_strategy' => 'polling', + // empty lines and extra spaces between the URL to generate empty entries + 'polling_url' => "https://api1.example.com/data\n \n\nhttps://api2.example.com/weather\n ", + 'polling_verb' => 'get', + ]); + + // Mock only the valid URLs + Http::fake([ + 'https://api1.example.com/data' => Http::response(['item' => 'first'], 200), + 'https://api2.example.com/weather' => Http::response(['item' => 'second'], 200), + ]); + + $plugin->updateDataPayload(); + + // payload should only have 2 items, and they should be indexed 0 and 1 + expect($plugin->data_payload)->toHaveCount(2); + expect($plugin->data_payload)->toHaveKey('IDX_0'); + expect($plugin->data_payload)->toHaveKey('IDX_1'); + + // data is correct + expect($plugin->data_payload['IDX_0'])->toBe(['item' => 'first']); + expect($plugin->data_payload['IDX_1'])->toBe(['item' => 'second']); + + // no empty index exists + expect($plugin->data_payload)->not->toHaveKey('IDX_2'); +}); + test('updateDataPayload handles single URL without nesting', function (): void { $plugin = Plugin::factory()->create([ 'data_strategy' => 'polling',