From cbd7ccc4ec5aaa43a84f3a63e000beb217fcb6be Mon Sep 17 00:00:00 2001 From: Naveen Goud Date: Tue, 27 Aug 2024 16:21:12 +0530 Subject: [PATCH] =?UTF-8?q?fix:=20updated=20the=20regex=20expression=20to?= =?UTF-8?q?=20correctly=20identify=20the=20table=20nam=E2=80=A6=20(#35361)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Description** When the user tries to add a postgres datasource and add any query, if the table name has hyphens in it, then quotes get assigned to first word after the schema and the dot. For example, public."counter"-with-db The quotes should be applied to the whole table name. For example: public."counter-with-db" Fixes https://github.com/appsmithorg/appsmith/issues/10631 Fixes https://github.com/appsmithorg/appsmith/issues/30692 **changes in PR:** 1.updated the regec expression to idetify the table name properly with table name consists of hypens. 2.added a test case for this scenario. **snapshots:** before: ![Screenshot from 2024-07-30 17-15-27](https://github.com/user-attachments/assets/98f969da-4cf1-4367-be29-1f3465179a9d) After: ![Screenshot from 2024-08-01 10-46-53](https://github.com/user-attachments/assets/29411eb7-77af-4a10-9988-a0c076043945) Hi @ajinkyakulkarni @rohan-arthur @Nikhil-Nandagopal ,Please review this PR. ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Enhanced handling of table names in the Postgres plugin to support a wider variety of characters, including hyphens and special characters. - Introduced multiple new database tables for improved testing coverage. - **Bug Fixes** - Adjusted table name processing to correctly format names containing a broader range of characters. - **Tests** - Added new tests to verify the structure and integrity of tables with various naming conventions. - Expanded existing tests to accommodate additional table structures. --- .../com/external/plugins/PostgresPlugin.java | 2 +- .../external/plugins/PostgresPluginTest.java | 311 +++++++++++++++++- 2 files changed, 311 insertions(+), 2 deletions(-) diff --git a/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java b/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java index 003b8ddde36..218eba72eb7 100644 --- a/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java +++ b/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java @@ -884,7 +884,7 @@ public Mono getStructure( setFragments.deleteCharAt(setFragments.length() - 1); } - final String quotedTableName = table.getName().replaceFirst("\\.(\\w+)", ".\"$1\""); + final String quotedTableName = table.getName().replaceFirst("\\.(.+)", ".\"$1\""); table.getTemplates() .addAll(List.of( new DatasourceStructure.Template( diff --git a/app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java b/app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java index 7c2b084aeb8..b15878ccbc5 100644 --- a/app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java +++ b/app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java @@ -191,6 +191,40 @@ public static void setUp() { + " texts VARCHAR[2] ,\n" + " rating FLOAT4 \n" + ")"); + + statement.execute("CREATE TABLE \"testing-table-data\" (\n" + + " id serial PRIMARY KEY,\n" + + " name VARCHAR,\n" + + " age INTEGER,\n" + + " email VARCHAR (255) UNIQUE,\n" + + " join_date DATE,\n" + + " salary FLOAT4\n" + + ")"); + statement.execute("CREATE TABLE \"table_name-123\" (\n" + + " id serial PRIMARY KEY,\n" + + " name VARCHAR,\n" + + " age INTEGER,\n" + + " email VARCHAR (255) UNIQUE,\n" + + " join_date DATE,\n" + + " salary FLOAT4\n" + + ")"); + statement.execute("CREATE TABLE \"table-name.with.dots\" (\n" + + " id serial PRIMARY KEY,\n" + + " name VARCHAR,\n" + + " age INTEGER,\n" + + " email VARCHAR (255) UNIQUE,\n" + + " join_date DATE,\n" + + " salary FLOAT4\n" + + ")"); + + statement.execute("CREATE TABLE \"table@name#special$chars\" (\n" + + " id serial PRIMARY KEY,\n" + + " name VARCHAR,\n" + + " age INTEGER,\n" + + " email VARCHAR (255) UNIQUE,\n" + + " join_date DATE,\n" + + " salary FLOAT4\n" + + ")"); } try (Statement statement = connection.createStatement()) { @@ -236,6 +270,22 @@ public static void setUp() { + "'{\"country\":\"japan\", \"city\":\"kyoto\"}', 'A Lincoln'" + ")"); } + try (Statement statement = connection.createStatement()) { + statement.execute( + "INSERT INTO \"testing-table-data\" VALUES (" + " 1, '', 30, '', '2023-07-31', 75000.00" + ")"); + } + try (Statement statement = connection.createStatement()) { + statement.execute("INSERT INTO \"table_name-123\" VALUES (" + + " 1, 'John Doe', 30, 'john.doe@example.com', '2023-07-31', 75000.00" + ")"); + } + try (Statement statement = connection.createStatement()) { + statement.execute("INSERT INTO \"table-name.with.dots\" VALUES (" + + " 1, 'Jane Doe', 28, 'jane.doe@example.com', '2023-08-01', 65000.00" + ")"); + } + try (Statement statement = connection.createStatement()) { + statement.execute("INSERT INTO \"table@name#special$chars\" VALUES (" + + " 1, 'Alice Smith', 35, 'alice.smith@example.com', '2023-08-15', 80000.00" + ")"); + } } catch (SQLException throwable) { throwable.printStackTrace(); @@ -299,6 +349,198 @@ private DatasourceStructure.Table findTableByName(List structureMono = pluginExecutor + .datasourceCreate(dsConfig) + .flatMap(connection -> pluginExecutor.getStructure(connection, dsConfig)); + + StepVerifier.create(structureMono) + .assertNext(structure -> { + assertNotNull(structure); + DatasourceStructure.Table sampleTable = + findTableByName(structure.getTables(), "public.table@name#special$chars"); + assertNotNull(sampleTable); + assertEquals(DatasourceStructure.TableType.TABLE, sampleTable.getType()); + assertArrayEquals( + new DatasourceStructure.Column[] { + new DatasourceStructure.Column( + "id", "int4", "nextval('\"table@name#special$chars_id_seq\"'::regclass)", true), + new DatasourceStructure.Column("name", "varchar", null, false), + new DatasourceStructure.Column("age", "int4", null, false), + new DatasourceStructure.Column("email", "varchar", null, false), + new DatasourceStructure.Column("join_date", "date", null, false), + new DatasourceStructure.Column("salary", "float4", null, false), + }, + sampleTable.getColumns().toArray()); + + final DatasourceStructure.PrimaryKey samplePrimaryKey = + new DatasourceStructure.PrimaryKey("table@name#special$chars_pkey", new ArrayList<>()); + samplePrimaryKey.getColumnNames().add("id"); + assertArrayEquals( + new DatasourceStructure.Key[] {samplePrimaryKey}, + sampleTable.getKeys().toArray()); + + assertArrayEquals( + new DatasourceStructure.Template[] { + new DatasourceStructure.Template( + "SELECT", "SELECT * FROM public.\"table@name#special$chars\" LIMIT 10;", true), + new DatasourceStructure.Template( + "INSERT", + "INSERT INTO public.\"table@name#special$chars\" " + + "(\"name\", \"age\", \"email\", \"join_date\", \"salary\")\n " + + "VALUES ('', 1, '', '2019-07-01', 1.0);", + false), + new DatasourceStructure.Template( + "UPDATE", + "UPDATE public.\"table@name#special$chars\" SET\n" + + " \"name\" = '',\n" + + " \"age\" = 1,\n" + + " \"email\" = '',\n" + + " \"join_date\" = '2019-07-01',\n" + + " \"salary\" = 1.0\n" + + " WHERE 1 = 0; -- Specify a valid condition here. Removing the condition may update every row in the table!", + false), + new DatasourceStructure.Template( + "DELETE", + "DELETE FROM public.\"table@name#special$chars\"\n" + + " WHERE 1 = 0; -- Specify a valid condition here. Removing the condition may delete everything in the table!", + false), + }, + sampleTable.getTemplates().toArray()); + }) + .verifyComplete(); + } + + @Test + public void testStructure_containing_underscore_and_hyphen() throws SQLException { + DatasourceConfiguration dsConfig = createDatasourceConfiguration(); + Mono structureMono = pluginExecutor + .datasourceCreate(dsConfig) + .flatMap(connection -> pluginExecutor.getStructure(connection, dsConfig)); + + StepVerifier.create(structureMono) + .assertNext(structure -> { + assertNotNull(structure); + DatasourceStructure.Table sampleTable = + findTableByName(structure.getTables(), "public.table_name-123"); + assertNotNull(sampleTable); + assertEquals(DatasourceStructure.TableType.TABLE, sampleTable.getType()); + assertArrayEquals( + new DatasourceStructure.Column[] { + new DatasourceStructure.Column( + "id", "int4", "nextval('\"table_name-123_id_seq\"'::regclass)", true), + new DatasourceStructure.Column("name", "varchar", null, false), + new DatasourceStructure.Column("age", "int4", null, false), + new DatasourceStructure.Column("email", "varchar", null, false), + new DatasourceStructure.Column("join_date", "date", null, false), + new DatasourceStructure.Column("salary", "float4", null, false), + }, + sampleTable.getColumns().toArray()); + + final DatasourceStructure.PrimaryKey samplePrimaryKey = + new DatasourceStructure.PrimaryKey("table_name-123_pkey", new ArrayList<>()); + samplePrimaryKey.getColumnNames().add("id"); + assertArrayEquals( + new DatasourceStructure.Key[] {samplePrimaryKey}, + sampleTable.getKeys().toArray()); + + assertArrayEquals( + new DatasourceStructure.Template[] { + new DatasourceStructure.Template( + "SELECT", "SELECT * FROM public.\"table_name-123\" LIMIT 10;", true), + new DatasourceStructure.Template( + "INSERT", + "INSERT INTO public.\"table_name-123\" " + + "(\"name\", \"age\", \"email\", \"join_date\", \"salary\")\n " + + "VALUES ('', 1, '', '2019-07-01', 1.0);", + false), + new DatasourceStructure.Template( + "UPDATE", + "UPDATE public.\"table_name-123\" SET\n" + + " \"name\" = '',\n" + + " \"age\" = 1,\n" + + " \"email\" = '',\n" + + " \"join_date\" = '2019-07-01',\n" + + " \"salary\" = 1.0\n" + + " WHERE 1 = 0; -- Specify a valid condition here. Removing the condition may update every row in the table!", + false), + new DatasourceStructure.Template( + "DELETE", + "DELETE FROM public.\"table_name-123\"\n" + + " WHERE 1 = 0; -- Specify a valid condition here. Removing the condition may delete everything in the table!", + false), + }, + sampleTable.getTemplates().toArray()); + }) + .verifyComplete(); + } + + @Test + public void testStructure_containing_dots() { + DatasourceConfiguration dsConfig = createDatasourceConfiguration(); + Mono structureMono = pluginExecutor + .datasourceCreate(dsConfig) + .flatMap(connection -> pluginExecutor.getStructure(connection, dsConfig)); + + StepVerifier.create(structureMono) + .assertNext(structure -> { + assertNotNull(structure); + DatasourceStructure.Table sampleTable = + findTableByName(structure.getTables(), "public.table-name.with.dots"); + assertNotNull(sampleTable); + assertEquals(DatasourceStructure.TableType.TABLE, sampleTable.getType()); + assertArrayEquals( + new DatasourceStructure.Column[] { + new DatasourceStructure.Column( + "id", "int4", "nextval('\"table-name.with.dots_id_seq\"'::regclass)", true), + new DatasourceStructure.Column("name", "varchar", null, false), + new DatasourceStructure.Column("age", "int4", null, false), + new DatasourceStructure.Column("email", "varchar", null, false), + new DatasourceStructure.Column("join_date", "date", null, false), + new DatasourceStructure.Column("salary", "float4", null, false), + }, + sampleTable.getColumns().toArray()); + + final DatasourceStructure.PrimaryKey samplePrimaryKey = + new DatasourceStructure.PrimaryKey("table-name.with.dots_pkey", new ArrayList<>()); + samplePrimaryKey.getColumnNames().add("id"); + assertArrayEquals( + new DatasourceStructure.Key[] {samplePrimaryKey}, + sampleTable.getKeys().toArray()); + + assertArrayEquals( + new DatasourceStructure.Template[] { + new DatasourceStructure.Template( + "SELECT", "SELECT * FROM public.\"table-name.with.dots\" LIMIT 10;", true), + new DatasourceStructure.Template( + "INSERT", + "INSERT INTO public.\"table-name.with.dots\" " + + "(\"name\", \"age\", \"email\", \"join_date\", \"salary\")\n " + + "VALUES ('', 1, '', '2019-07-01', 1.0);", + false), + new DatasourceStructure.Template( + "UPDATE", + "UPDATE public.\"table-name.with.dots\" SET\n" + + " \"name\" = '',\n" + + " \"age\" = 1,\n" + + " \"email\" = '',\n" + + " \"join_date\" = '2019-07-01',\n" + + " \"salary\" = 1.0\n" + + " WHERE 1 = 0; -- Specify a valid condition here. Removing the condition may update every row in the table!", + false), + new DatasourceStructure.Template( + "DELETE", + "DELETE FROM public.\"table-name.with.dots\"\n" + + " WHERE 1 = 0; -- Specify a valid condition here. Removing the condition may delete everything in the table!", + false), + }, + sampleTable.getTemplates().toArray()); + }) + .verifyComplete(); + } + @Test public void testConnectPostgresContainer() { @@ -501,7 +743,7 @@ public void testStructure() { StepVerifier.create(structureMono) .assertNext(structure -> { assertNotNull(structure); - assertEquals(5, structure.getTables().size()); + assertEquals(9, structure.getTables().size()); DatasourceStructure.Table campusTable = findTableByName(structure.getTables(), "public.campus"); assertNotNull(campusTable); @@ -696,6 +938,73 @@ public void testStructure() { .verifyComplete(); } + @Test + public void testStructure_containing_hyphen() { + + DatasourceConfiguration dsConfig = createDatasourceConfiguration(); + Mono structureMono = pluginExecutor + .datasourceCreate(dsConfig) + .flatMap(connection -> pluginExecutor.getStructure(connection, dsConfig)); + + StepVerifier.create(structureMono) + .assertNext(structure -> { + assertNotNull(structure); + assertEquals(9, structure.getTables().size()); + + DatasourceStructure.Table sampleTable = + findTableByName(structure.getTables(), "public.testing-table-data"); + assertNotNull(sampleTable); + assertEquals(DatasourceStructure.TableType.TABLE, sampleTable.getType()); + assertArrayEquals( + new DatasourceStructure.Column[] { + new DatasourceStructure.Column( + "id", "int4", "nextval('\"testing-table-data_id_seq\"'::regclass)", true), + new DatasourceStructure.Column("name", "varchar", null, false), + new DatasourceStructure.Column("age", "int4", null, false), + new DatasourceStructure.Column("email", "varchar", null, false), + new DatasourceStructure.Column("join_date", "date", null, false), + new DatasourceStructure.Column("salary", "float4", null, false), + }, + sampleTable.getColumns().toArray()); + + final DatasourceStructure.PrimaryKey samplePrimaryKey = + new DatasourceStructure.PrimaryKey("testing-table-data_pkey", new ArrayList<>()); + samplePrimaryKey.getColumnNames().add("id"); + assertArrayEquals( + new DatasourceStructure.Key[] {samplePrimaryKey}, + sampleTable.getKeys().toArray()); + + assertArrayEquals( + new DatasourceStructure.Template[] { + new DatasourceStructure.Template( + "SELECT", "SELECT * FROM public.\"testing-table-data\" LIMIT 10;", true), + new DatasourceStructure.Template( + "INSERT", + "INSERT INTO public.\"testing-table-data\" " + + "(\"name\", \"age\", \"email\", \"join_date\", \"salary\")\n " + + "VALUES ('', 1, '', '2019-07-01', 1.0);", + false), + new DatasourceStructure.Template( + "UPDATE", + "UPDATE public.\"testing-table-data\" SET\n" + + " \"name\" = '',\n" + + " \"age\" = 1,\n" + + " \"email\" = '',\n" + + " \"join_date\" = '2019-07-01',\n" + + " \"salary\" = 1.0\n" + + " WHERE 1 = 0; -- Specify a valid condition here. Removing the condition may update every row in the table!", + false), + new DatasourceStructure.Template( + "DELETE", + "DELETE FROM public.\"testing-table-data\"\n" + + " WHERE 1 = 0; -- Specify a valid condition here. Removing the condition may delete everything in the table!", + false), + }, + sampleTable.getTemplates().toArray()); + }) + .verifyComplete(); + } + @Test public void testStaleConnectionCheck() { DatasourceConfiguration dsConfig = createDatasourceConfiguration();