From 8ef766a2deed4829a761fc29d6eac0b9b9457f5d Mon Sep 17 00:00:00 2001 From: Marius Andra Date: Thu, 3 Oct 2024 10:02:11 +0200 Subject: [PATCH] fix(apps): transpile site apps on create (#25349) --- posthog/api/plugin.py | 34 +++++------ posthog/models/plugin.py | 62 ++++++++++++++++++--- posthog/test/__snapshots__/test_plugin.ambr | 6 +- posthog/test/test_plugin.py | 38 +++++++------ 4 files changed, 91 insertions(+), 49 deletions(-) diff --git a/posthog/api/plugin.py b/posthog/api/plugin.py index 1b9b233c62cdd..9678af9d7457b 100644 --- a/posthog/api/plugin.py +++ b/posthog/api/plugin.py @@ -1,8 +1,6 @@ import json -import os import re -import subprocess -from typing import Any, Optional, cast, Literal +from typing import Any, Optional, cast import requests from dateutil.relativedelta import relativedelta @@ -42,6 +40,7 @@ PluginSourceFile, update_validated_data_from_url, validate_plugin_job_payload, + transpile, ) from posthog.models.utils import UUIDT, generate_random_token from posthog.permissions import APIScopePermission @@ -200,24 +199,6 @@ def _fix_formdata_config_json(request: request.Request, validated_data: dict): validated_data["config"] = json.loads(request.POST["config"]) -def transpile(input_string: str, type: Literal["site", "frontend"] = "site") -> Optional[str]: - from posthog.settings.base_variables import BASE_DIR - - transpiler_path = os.path.join(BASE_DIR, "plugin-transpiler/dist/index.js") - if type not in ["site", "frontend"]: - raise Exception('Invalid type. Must be "site" or "frontend".') - - process = subprocess.Popen( - ["node", transpiler_path, "--type", type], stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE - ) - stdout, stderr = process.communicate(input=input_string.encode()) - - if process.returncode != 0: - error = stderr.decode() - raise Exception(error) - return stdout.decode() - - class PlainRenderer(renderers.BaseRenderer): format = "txt" @@ -311,6 +292,17 @@ def create(self, validated_data: dict, *args: Any, **kwargs: Any) -> Plugin: plugin = Plugin.objects.install(**validated_data) + for source_file in PluginSourceFile.objects.filter(plugin=plugin): + if source_file.filename in ("site.tsx", "frontend.tsx"): + try: + source_file.transpiled = transpile(source_file.source, type=source_file.filename.split(".")[0]) + source_file.status = PluginSourceFile.Status.TRANSPILED + source_file.save() + except Exception as e: + source_file.status = PluginSourceFile.Status.ERROR + source_file.error = str(e) + source_file.save() + return plugin def update(self, plugin: Plugin, validated_data: dict, *args: Any, **kwargs: Any) -> Plugin: # type: ignore diff --git a/posthog/models/plugin.py b/posthog/models/plugin.py index 9fe174a9dbe68..d32a18f974939 100644 --- a/posthog/models/plugin.py +++ b/posthog/models/plugin.py @@ -1,8 +1,9 @@ import datetime import os +import subprocess from dataclasses import dataclass from enum import StrEnum -from typing import Any, Optional, cast +from typing import Any, Optional, cast, Literal from uuid import UUID from django.conf import settings @@ -302,6 +303,24 @@ class PluginLogEntryType(StrEnum): ERROR = "ERROR" +def transpile(input_string: str, type: Literal["site", "frontend"] = "site") -> Optional[str]: + from posthog.settings.base_variables import BASE_DIR + + transpiler_path = os.path.join(BASE_DIR, "plugin-transpiler/dist/index.js") + if type not in ["site", "frontend"]: + raise Exception('Invalid type. Must be "site" or "frontend".') + + process = subprocess.Popen( + ["node", transpiler_path, "--type", type], stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE + ) + stdout, stderr = process.communicate(input=input_string.encode()) + + if process.returncode != 0: + error = stderr.decode() + raise Exception(error) + return stdout.decode() + + class PluginSourceFileManager(models.Manager): def sync_from_plugin_archive( self, plugin: Plugin, plugin_json_parsed: Optional[dict[str, Any]] = None @@ -318,8 +337,10 @@ def sync_from_plugin_archive( plugin_json, index_ts, frontend_tsx, site_ts = extract_plugin_code(plugin.archive, plugin_json_parsed) except ValueError as e: raise exceptions.ValidationError(f"{e} in plugin {plugin}") + # If frontend.tsx or index.ts are not present in the archive, make sure they aren't found in the DB either filenames_to_delete = [] + # Save plugin.json plugin_json_instance, _ = PluginSourceFile.objects.update_or_create( plugin=plugin, @@ -331,36 +352,58 @@ def sync_from_plugin_archive( "error": None, }, ) + # Save frontend.tsx frontend_tsx_instance: Optional[PluginSourceFile] = None if frontend_tsx is not None: + transpiled = None + status = None + error = None + try: + transpiled = transpile(frontend_tsx, type="site") + status = PluginSourceFile.Status.TRANSPILED + except Exception as e: + error = str(e) + status = PluginSourceFile.Status.ERROR frontend_tsx_instance, _ = PluginSourceFile.objects.update_or_create( plugin=plugin, filename="frontend.tsx", defaults={ "source": frontend_tsx, - "transpiled": None, - "status": None, - "error": None, + "transpiled": transpiled, + "status": status, + "error": error, }, ) else: filenames_to_delete.append("frontend.tsx") - # Save frontend.tsx + + # Save site.ts site_ts_instance: Optional[PluginSourceFile] = None if site_ts is not None: + transpiled = None + status = None + error = None + try: + transpiled = transpile(site_ts, type="site") + status = PluginSourceFile.Status.TRANSPILED + except Exception as e: + error = str(e) + status = PluginSourceFile.Status.ERROR + site_ts_instance, _ = PluginSourceFile.objects.update_or_create( plugin=plugin, filename="site.ts", defaults={ "source": site_ts, - "transpiled": None, - "status": None, - "error": None, + "transpiled": transpiled, + "status": status, + "error": error, }, ) else: filenames_to_delete.append("site.ts") + # Save index.ts index_ts_instance: Optional[PluginSourceFile] = None if index_ts is not None: @@ -378,10 +421,13 @@ def sync_from_plugin_archive( ) else: filenames_to_delete.append("index.ts") + # Make sure files are gone PluginSourceFile.objects.filter(plugin=plugin, filename__in=filenames_to_delete).delete() + # Trigger plugin server reload and code transpilation plugin.save() + return ( plugin_json_instance, index_ts_instance, diff --git a/posthog/test/__snapshots__/test_plugin.ambr b/posthog/test/__snapshots__/test_plugin.ambr index 9d2f526c3e4d1..2ffedb586b654 100644 --- a/posthog/test/__snapshots__/test_plugin.ambr +++ b/posthog/test/__snapshots__/test_plugin.ambr @@ -209,7 +209,7 @@ FROM "posthog_pluginsourcefile" ''' # --- -# name: TestPluginSourceFile.test_sync_from_plugin_archive_from_zip_without_index_ts_but_site_Ts_works +# name: TestPluginSourceFile.test_sync_from_plugin_archive_from_zip_without_index_ts_but_site_ts_works ''' SELECT "posthog_pluginsourcefile"."id", "posthog_pluginsourcefile"."plugin_id", @@ -227,7 +227,7 @@ UPDATE ''' # --- -# name: TestPluginSourceFile.test_sync_from_plugin_archive_from_zip_without_index_ts_but_site_Ts_works.1 +# name: TestPluginSourceFile.test_sync_from_plugin_archive_from_zip_without_index_ts_but_site_ts_works.1 ''' SELECT "posthog_pluginsourcefile"."id", "posthog_pluginsourcefile"."plugin_id", @@ -245,7 +245,7 @@ UPDATE ''' # --- -# name: TestPluginSourceFile.test_sync_from_plugin_archive_from_zip_without_index_ts_but_site_Ts_works.2 +# name: TestPluginSourceFile.test_sync_from_plugin_archive_from_zip_without_index_ts_but_site_ts_works.2 ''' SELECT COUNT(*) AS "__count" FROM "posthog_pluginsourcefile" diff --git a/posthog/test/test_plugin.py b/posthog/test/test_plugin.py index 8e249de2f7399..65a119b40bd6f 100644 --- a/posthog/test/test_plugin.py +++ b/posthog/test/test_plugin.py @@ -178,7 +178,7 @@ def test_sync_from_plugin_archive_from_zip_with_explicit_index_js_works(self): plugin_json_file, index_ts_file, frontend_tsx_file, - site_Ts_file, + site_ts_file, ) = PluginSourceFile.objects.sync_from_plugin_archive(test_plugin) self.assertEqual(PluginSourceFile.objects.count(), 2) @@ -186,7 +186,7 @@ def test_sync_from_plugin_archive_from_zip_with_explicit_index_js_works(self): assert index_ts_file is not None self.assertEqual(index_ts_file.source, HELLO_WORLD_PLUGIN_GITHUB_INDEX_JS) self.assertIsNone(frontend_tsx_file) - self.assertIsNone(site_Ts_file) + self.assertIsNone(site_ts_file) @snapshot_postgres_queries def test_sync_from_plugin_archive_from_tgz_with_explicit_index_js_works(self): @@ -201,7 +201,7 @@ def test_sync_from_plugin_archive_from_tgz_with_explicit_index_js_works(self): plugin_json_file, index_ts_file, frontend_tsx_file, - site_Ts_file, + site_ts_file, ) = PluginSourceFile.objects.sync_from_plugin_archive(test_plugin) self.assertEqual(PluginSourceFile.objects.count(), 2) @@ -209,14 +209,14 @@ def test_sync_from_plugin_archive_from_tgz_with_explicit_index_js_works(self): assert index_ts_file is not None self.assertEqual(index_ts_file.source, HELLO_WORLD_PLUGIN_NPM_INDEX_JS) self.assertIsNone(frontend_tsx_file) - self.assertIsNone(site_Ts_file) + self.assertIsNone(site_ts_file) # Second time - update ( plugin_json_file, index_ts_file, frontend_tsx_file, - site_Ts_file, + site_ts_file, ) = PluginSourceFile.objects.sync_from_plugin_archive(test_plugin) self.assertEqual(PluginSourceFile.objects.count(), 2) @@ -224,7 +224,7 @@ def test_sync_from_plugin_archive_from_tgz_with_explicit_index_js_works(self): assert index_ts_file is not None self.assertEqual(index_ts_file.source, HELLO_WORLD_PLUGIN_NPM_INDEX_JS) self.assertIsNone(frontend_tsx_file) - self.assertIsNone(site_Ts_file) + self.assertIsNone(site_ts_file) @snapshot_postgres_queries def test_sync_from_plugin_archive_from_zip_with_index_ts_works(self): @@ -239,7 +239,7 @@ def test_sync_from_plugin_archive_from_zip_with_index_ts_works(self): plugin_json_file, index_ts_file, frontend_tsx_file, - site_Ts_file, + site_ts_file, ) = PluginSourceFile.objects.sync_from_plugin_archive(test_plugin) self.assertEqual(PluginSourceFile.objects.count(), 2) @@ -247,7 +247,7 @@ def test_sync_from_plugin_archive_from_zip_with_index_ts_works(self): assert index_ts_file is not None self.assertEqual(index_ts_file.source, HELLO_WORLD_PLUGIN_GITHUB_INDEX_JS) self.assertIsNone(frontend_tsx_file) - self.assertIsNone(site_Ts_file) + self.assertIsNone(site_ts_file) self.assertFalse(self.team.inject_web_apps) @snapshot_postgres_queries @@ -263,19 +263,19 @@ def test_sync_from_plugin_archive_from_zip_without_index_ts_but_frontend_tsx_wor plugin_json_file, index_ts_file, frontend_tsx_file, - site_Ts_file, + site_ts_file, ) = PluginSourceFile.objects.sync_from_plugin_archive(test_plugin) self.assertEqual(PluginSourceFile.objects.count(), 2) self.assertEqual(plugin_json_file.source, HELLO_WORLD_PLUGIN_PLUGIN_JSON_WITHOUT_MAIN) self.assertIsNone(index_ts_file) - self.assertIsNone(site_Ts_file) + self.assertIsNone(site_ts_file) assert frontend_tsx_file is not None self.assertEqual(frontend_tsx_file.source, HELLO_WORLD_PLUGIN_FRONTEND_TSX) self.assertFalse(self.team.inject_web_apps) @snapshot_postgres_queries - def test_sync_from_plugin_archive_from_zip_without_index_ts_but_site_Ts_works(self): + def test_sync_from_plugin_archive_from_zip_without_index_ts_but_site_ts_works(self): self.assertFalse(self.team.inject_web_apps) test_plugin: Plugin = Plugin.objects.create( organization=self.organization, @@ -287,15 +287,17 @@ def test_sync_from_plugin_archive_from_zip_without_index_ts_but_site_Ts_works(se plugin_json_file, index_ts_file, frontend_tsx_file, - site_Ts_file, + site_ts_file, ) = PluginSourceFile.objects.sync_from_plugin_archive(test_plugin) self.assertEqual(PluginSourceFile.objects.count(), 2) self.assertEqual(plugin_json_file.source, HELLO_WORLD_PLUGIN_PLUGIN_JSON_WITHOUT_MAIN) self.assertIsNone(index_ts_file) self.assertIsNone(frontend_tsx_file) - assert site_Ts_file is not None - self.assertEqual(site_Ts_file.source, HELLO_WORLD_PLUGIN_SITE_TS) + assert site_ts_file is not None + self.assertEqual(site_ts_file.source, HELLO_WORLD_PLUGIN_SITE_TS) + self.assertEqual(site_ts_file.status, PluginSourceFile.Status.TRANSPILED) + assert site_ts_file.transpiled is not None and len(site_ts_file.transpiled) > 0 @snapshot_postgres_queries def test_sync_from_plugin_archive_from_zip_without_any_code_fails(self): @@ -325,7 +327,7 @@ def test_sync_from_plugin_archive_twice_from_zip_with_index_ts_replaced_by_front plugin_json_file, index_ts_file, frontend_tsx_file, - site_Ts_file, + site_ts_file, ) = PluginSourceFile.objects.sync_from_plugin_archive(test_plugin) self.assertEqual(PluginSourceFile.objects.count(), 2) @@ -341,7 +343,7 @@ def test_sync_from_plugin_archive_twice_from_zip_with_index_ts_replaced_by_front plugin_json_file, index_ts_file, frontend_tsx_file, - site_Ts_file, + site_ts_file, ) = PluginSourceFile.objects.sync_from_plugin_archive(test_plugin) self.assertEqual(PluginSourceFile.objects.count(), 2) # frontend.tsx replaced by index.ts @@ -349,6 +351,8 @@ def test_sync_from_plugin_archive_twice_from_zip_with_index_ts_replaced_by_front self.assertIsNone(index_ts_file) assert frontend_tsx_file is not None self.assertEqual(frontend_tsx_file.source, HELLO_WORLD_PLUGIN_FRONTEND_TSX) + self.assertEqual(frontend_tsx_file.status, PluginSourceFile.Status.TRANSPILED) + assert frontend_tsx_file.transpiled is not None and len(frontend_tsx_file.transpiled) > 0 @snapshot_postgres_queries def test_sync_from_plugin_archive_with_subdir_works(self): @@ -363,7 +367,7 @@ def test_sync_from_plugin_archive_with_subdir_works(self): plugin_json_file, index_ts_file, frontend_tsx_file, - site_Ts_file, + site_ts_file, ) = PluginSourceFile.objects.sync_from_plugin_archive(test_plugin) self.assertEqual(PluginSourceFile.objects.count(), 2)