diff --git a/Dockerfile b/Dockerfile index aba3f90..4e50553 100644 --- a/Dockerfile +++ b/Dockerfile @@ -18,8 +18,6 @@ ENV TRMNL_LIQUID_ENABLED=1 # Switch to the root user so we can do root things USER root -COPY --chown=www-data:www-data --from=bnussbau/trmnl-liquid-cli:0.1.0 /usr/local/bin/trmnl-liquid-cli /usr/local/bin/ - # Set the working directory WORKDIR /var/www/html @@ -53,5 +51,7 @@ FROM base AS production # Copy the assets from the assets image COPY --chown=www-data:www-data --from=assets /app/public/build /var/www/html/public/build COPY --chown=www-data:www-data --from=assets /app/node_modules /var/www/html/node_modules + +COPY --chown=www-data:www-data --from=bnussbau/trmnl-liquid-cli:latest /usr/local/bin/trmnl-liquid-cli /usr/local/bin/ # Drop back to the www-data user USER www-data diff --git a/app/Models/Plugin.php b/app/Models/Plugin.php index 5df7205..ab83514 100644 --- a/app/Models/Plugin.php +++ b/app/Models/Plugin.php @@ -130,10 +130,9 @@ class Plugin extends Model } } - // Resolve Liquid variables in the entire polling_url field first, then split by newline - $resolvedPollingUrls = $this->resolveLiquidVariables($this->polling_url); + // Split URLs by newline and filter out empty lines $urls = array_filter( - array_map('trim', explode("\n", $resolvedPollingUrls)), + array_map('trim', explode("\n", $this->polling_url)), fn ($url): bool => ! empty($url) ); @@ -148,8 +147,8 @@ class Plugin extends Model $httpRequest = $httpRequest->withBody($resolvedBody); } - // URL is already resolved, use it directly - $resolvedUrl = $url; + // Resolve Liquid variables in the polling URL + $resolvedUrl = $this->resolveLiquidVariables($url); try { // Make the request based on the verb @@ -184,8 +183,8 @@ class Plugin extends Model $httpRequest = $httpRequest->withBody($resolvedBody); } - // URL is already resolved, use it directly - $resolvedUrl = $url; + // Resolve Liquid variables in the polling URL + $resolvedUrl = $this->resolveLiquidVariables($url); try { // Make the request based on the verb @@ -242,10 +241,10 @@ class Plugin extends Model try { // Attempt to parse it into JSON $json = $httpResponse->json(); - if ($json !== null) { + if($json !== null) { return $json; } - + // Response doesn't seem to be JSON, wrap the response body text as a JSON object return ['data' => $httpResponse->body()]; } catch (Exception $e) { @@ -345,48 +344,19 @@ class Plugin extends Model return $template; } - /** - * Check if a template contains a Liquid for loop pattern - * - * @param string $template The template string to check - * @return bool True if the template contains a for loop pattern - */ - private function containsLiquidForLoop(string $template): bool - { - return preg_match('/{%-?\s*for\s+/i', $template) === 1; - } - /** * Resolve Liquid variables in a template string using the Liquid template engine * - * Uses the external trmnl-liquid renderer when: - * - preferred_renderer is 'trmnl-liquid' - * - External renderer is enabled in config - * - Template contains a Liquid for loop pattern - * - * Otherwise uses the internal PHP-based Liquid renderer. - * * @param string $template The template string containing Liquid variables * @return string The resolved template with variables replaced with their values * * @throws LiquidException - * @throws Exception */ public function resolveLiquidVariables(string $template): string { // Get configuration variables - make them available at root level $variables = $this->configuration ?? []; - // Check if external renderer should be used - $useExternalRenderer = $this->preferred_renderer === 'trmnl-liquid' - && config('services.trmnl.liquid_enabled') - && $this->containsLiquidForLoop($template); - - if ($useExternalRenderer) { - // Use external Ruby liquid renderer - return $this->renderWithExternalLiquidRenderer($template, $variables); - } - // Use the Liquid template engine to resolve variables $environment = App::make('liquid.environment'); $environment->filterRegistry->register(StandardFilters::class); @@ -434,7 +404,7 @@ class Plugin extends Model '--context', $jsonContext, ]); - + if (! $process->successful()) { $errorOutput = $process->errorOutput() ?: $process->output(); throw new Exception('External liquid renderer failed: '.$errorOutput); diff --git a/config/app.php b/config/app.php index c7cb051..73bcaaf 100644 --- a/config/app.php +++ b/config/app.php @@ -130,7 +130,7 @@ return [ 'force_https' => env('FORCE_HTTPS', false), 'puppeteer_docker' => env('PUPPETEER_DOCKER', false), 'puppeteer_mode' => env('PUPPETEER_MODE', 'local'), - 'puppeteer_wait_for_network_idle' => env('PUPPETEER_WAIT_FOR_NETWORK_IDLE', true), + 'puppeteer_wait_for_network_idle' => env('PUPPETEER_WAIT_FOR_NETWORK_IDLE', false), 'puppeteer_window_size_strategy' => env('PUPPETEER_WINDOW_SIZE_STRATEGY', null), 'notifications' => [ diff --git a/resources/views/livewire/plugins/index.blade.php b/resources/views/livewire/plugins/index.blade.php index 0482068..4f4da8b 100644 --- a/resources/views/livewire/plugins/index.blade.php +++ b/resources/views/livewire/plugins/index.blade.php @@ -296,16 +296,6 @@ new class extends Component { Import from TRMNL Recipe Catalog Alpha - - Limitations - - Please report issues, aside from the known limitations, on GitHub. Include the recipe URL. - diff --git a/tests/Unit/Models/PluginTest.php b/tests/Unit/Models/PluginTest.php index e212cb8..ef054b1 100644 --- a/tests/Unit/Models/PluginTest.php +++ b/tests/Unit/Models/PluginTest.php @@ -357,233 +357,3 @@ test('resolveLiquidVariables handles empty configuration', function (): void { expect($plugin->resolveLiquidVariables($template))->toBe($expected); }); - -test('resolveLiquidVariables uses external renderer when preferred_renderer is trmnl-liquid and template contains for loop', function (): void { - Illuminate\Support\Facades\Process::fake([ - '*' => Illuminate\Support\Facades\Process::result( - output: 'https://api1.example.com/data\nhttps://api2.example.com/data', - exitCode: 0 - ), - ]); - - config(['services.trmnl.liquid_enabled' => true]); - config(['services.trmnl.liquid_path' => '/usr/local/bin/trmnl-liquid-cli']); - - $plugin = Plugin::factory()->create([ - 'preferred_renderer' => 'trmnl-liquid', - 'configuration' => [ - 'recipe_ids' => '1,2', - ], - ]); - - $template = <<<'LIQUID' -{% assign ids = recipe_ids | split: "," %} -{% for id in ids %} -https://api{{ id }}.example.com/data -{% endfor %} -LIQUID; - - $result = $plugin->resolveLiquidVariables($template); - - // Trim trailing newlines that may be added by the process - expect(mb_trim($result))->toBe('https://api1.example.com/data\nhttps://api2.example.com/data'); - - Illuminate\Support\Facades\Process::assertRan(function ($process): bool { - $command = is_array($process->command) ? implode(' ', $process->command) : $process->command; - - return str_contains($command, 'trmnl-liquid-cli') && - str_contains($command, '--template') && - str_contains($command, '--context'); - }); -}); - -test('resolveLiquidVariables uses internal renderer when preferred_renderer is not trmnl-liquid', function (): void { - $plugin = Plugin::factory()->create([ - 'preferred_renderer' => 'php', - 'configuration' => [ - 'recipe_ids' => '1,2', - ], - ]); - - $template = <<<'LIQUID' -{% assign ids = recipe_ids | split: "," %} -{% for id in ids %} -https://api{{ id }}.example.com/data -{% endfor %} -LIQUID; - - // Should use internal renderer even with for loop - $result = $plugin->resolveLiquidVariables($template); - - // Internal renderer should process the template - expect($result)->toBeString(); -}); - -test('resolveLiquidVariables uses internal renderer when external renderer is disabled', function (): void { - config(['services.trmnl.liquid_enabled' => false]); - - $plugin = Plugin::factory()->create([ - 'preferred_renderer' => 'trmnl-liquid', - 'configuration' => [ - 'recipe_ids' => '1,2', - ], - ]); - - $template = <<<'LIQUID' -{% assign ids = recipe_ids | split: "," %} -{% for id in ids %} -https://api{{ id }}.example.com/data -{% endfor %} -LIQUID; - - // Should use internal renderer when external is disabled - $result = $plugin->resolveLiquidVariables($template); - - expect($result)->toBeString(); -}); - -test('resolveLiquidVariables uses internal renderer when template does not contain for loop', function (): void { - config(['services.trmnl.liquid_enabled' => true]); - config(['services.trmnl.liquid_path' => '/usr/local/bin/trmnl-liquid-cli']); - - $plugin = Plugin::factory()->create([ - 'preferred_renderer' => 'trmnl-liquid', - 'configuration' => [ - 'api_key' => 'test123', - ], - ]); - - $template = 'https://api.example.com/data?key={{ api_key }}'; - - // Should use internal renderer when no for loop - $result = $plugin->resolveLiquidVariables($template); - - expect($result)->toBe('https://api.example.com/data?key=test123'); - - Illuminate\Support\Facades\Process::assertNothingRan(); -}); - -test('resolveLiquidVariables detects for loop with standard opening tag', function (): void { - Illuminate\Support\Facades\Process::fake([ - '*' => Illuminate\Support\Facades\Process::result( - output: 'resolved', - exitCode: 0 - ), - ]); - - config(['services.trmnl.liquid_enabled' => true]); - config(['services.trmnl.liquid_path' => '/usr/local/bin/trmnl-liquid-cli']); - - $plugin = Plugin::factory()->create([ - 'preferred_renderer' => 'trmnl-liquid', - 'configuration' => [], - ]); - - // Test {% for pattern - $template = '{% for item in items %}test{% endfor %}'; - $plugin->resolveLiquidVariables($template); - - Illuminate\Support\Facades\Process::assertRan(function ($process): bool { - $command = is_array($process->command) ? implode(' ', $process->command) : (string) $process->command; - - return str_contains($command, 'trmnl-liquid-cli'); - }); -}); - -test('resolveLiquidVariables detects for loop with whitespace stripping tag', function (): void { - Illuminate\Support\Facades\Process::fake([ - '*' => Illuminate\Support\Facades\Process::result( - output: 'resolved', - exitCode: 0 - ), - ]); - - config(['services.trmnl.liquid_enabled' => true]); - config(['services.trmnl.liquid_path' => '/usr/local/bin/trmnl-liquid-cli']); - - $plugin = Plugin::factory()->create([ - 'preferred_renderer' => 'trmnl-liquid', - 'configuration' => [], - ]); - - // Test {%- for pattern (with whitespace stripping) - $template = '{%- for item in items %}test{% endfor %}'; - $plugin->resolveLiquidVariables($template); - - Illuminate\Support\Facades\Process::assertRan(function ($process): bool { - $command = is_array($process->command) ? implode(' ', $process->command) : (string) $process->command; - - return str_contains($command, 'trmnl-liquid-cli'); - }); -}); - -test('updateDataPayload resolves entire polling_url field first then splits by newline', function (): void { - Http::fake([ - 'https://api1.example.com/data' => Http::response(['data' => 'test1'], 200), - 'https://api2.example.com/data' => Http::response(['data' => 'test2'], 200), - ]); - - $plugin = Plugin::factory()->create([ - 'data_strategy' => 'polling', - 'polling_url' => "https://api1.example.com/data\nhttps://api2.example.com/data", - 'polling_verb' => 'get', - 'configuration' => [ - 'recipe_ids' => '1,2', - ], - ]); - - $plugin->updateDataPayload(); - - // Should have split the multi-line URL and generated two requests - expect($plugin->data_payload)->toHaveKey('IDX_0'); - expect($plugin->data_payload)->toHaveKey('IDX_1'); - expect($plugin->data_payload['IDX_0'])->toBe(['data' => 'test1']); - expect($plugin->data_payload['IDX_1'])->toBe(['data' => 'test2']); -}); - -test('updateDataPayload handles multi-line polling_url with for loop using external renderer', function (): void { - Illuminate\Support\Facades\Process::fake([ - '*' => Illuminate\Support\Facades\Process::result( - output: "https://api1.example.com/data\nhttps://api2.example.com/data", - exitCode: 0 - ), - ]); - - Http::fake([ - 'https://api1.example.com/data' => Http::response(['data' => 'test1'], 200), - 'https://api2.example.com/data' => Http::response(['data' => 'test2'], 200), - ]); - - config(['services.trmnl.liquid_enabled' => true]); - config(['services.trmnl.liquid_path' => '/usr/local/bin/trmnl-liquid-cli']); - - $plugin = Plugin::factory()->create([ - 'data_strategy' => 'polling', - 'preferred_renderer' => 'trmnl-liquid', - 'polling_url' => <<<'LIQUID' -{% assign ids = recipe_ids | split: "," %} -{% for id in ids %} -https://api{{ id }}.example.com/data -{% endfor %} -LIQUID - , - 'polling_verb' => 'get', - 'configuration' => [ - 'recipe_ids' => '1,2', - ], - ]); - - $plugin->updateDataPayload(); - - // Should have used external renderer and generated two URLs - expect($plugin->data_payload)->toHaveKey('IDX_0'); - expect($plugin->data_payload)->toHaveKey('IDX_1'); - expect($plugin->data_payload['IDX_0'])->toBe(['data' => 'test1']); - expect($plugin->data_payload['IDX_1'])->toBe(['data' => 'test2']); - - Illuminate\Support\Facades\Process::assertRan(function ($process): bool { - $command = is_array($process->command) ? implode(' ', $process->command) : (string) $process->command; - - return str_contains($command, 'trmnl-liquid-cli'); - }); -});