From 2ad7671a003c30b390acc76e16708686058d4422 Mon Sep 17 00:00:00 2001 From: bhashinee Date: Mon, 25 Mar 2024 10:01:56 +0530 Subject: [PATCH 1/6] Fix persist compiler crash when no persist configurations are found --- .../stdlib/persist/compiler/CompilerPluginTest.java | 10 +++------- .../compiler/PersistModelDefinitionValidator.java | 3 +++ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/compiler-plugin-test/src/test/java/io/ballerina/stdlib/persist/compiler/CompilerPluginTest.java b/compiler-plugin-test/src/test/java/io/ballerina/stdlib/persist/compiler/CompilerPluginTest.java index 1fb02711..b28306c4 100644 --- a/compiler-plugin-test/src/test/java/io/ballerina/stdlib/persist/compiler/CompilerPluginTest.java +++ b/compiler-plugin-test/src/test/java/io/ballerina/stdlib/persist/compiler/CompilerPluginTest.java @@ -20,6 +20,7 @@ import io.ballerina.projects.DiagnosticResult; import io.ballerina.projects.Package; +import io.ballerina.projects.PackageCompilation; import io.ballerina.projects.directory.BuildProject; import io.ballerina.projects.directory.SingleFileProject; import io.ballerina.tools.diagnostics.Diagnostic; @@ -104,13 +105,8 @@ public void validateWithOldPersistTomlConfig() { @Test public void testWithoutDatastore() { - try { - loadPersistModelFile("project_9", "field-types.bal").getCompilation(); - Assert.fail("Compilation should fail"); - } catch (Exception e) { - Assert.assertTrue(e.getMessage().contains("the persist.datastore configuration does not exist in " + - "the Ballerina.toml file")); - } + PackageCompilation loadedProject = loadPersistModelFile("project_9", "field-types.bal").getCompilation(); + Assert.assertEquals(loadedProject.diagnosticResult().errorCount(), 0); } @Test diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/PersistModelDefinitionValidator.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/PersistModelDefinitionValidator.java index 2ea3de93..ebc7d214 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/PersistModelDefinitionValidator.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/PersistModelDefinitionValidator.java @@ -134,6 +134,9 @@ public void perform(SyntaxNodeAnalysisContext ctx) { try { datastore = getDatastore(ctx); } catch (BalException e) { + if (e.getMessage().contains("the persist.datastore configuration does not exist")) { + return; + } throw new RuntimeException(e); } From 7f04b4c136d0094d963821238db74cbc27eda8c3 Mon Sep 17 00:00:00 2001 From: bhashinee Date: Mon, 25 Mar 2024 10:07:00 +0530 Subject: [PATCH 2/6] Update the changelog --- changelog.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/changelog.md b/changelog.md index e2d1fbbd..f8a1bed1 100644 --- a/changelog.md +++ b/changelog.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- [Fixed the persist compiler crash when no persist configurations are found](https://github.com/ballerina-platform/ballerina-library/issues/6187) + ### Added - [Added compiler plugin validations for Postgresql as a datasource](https://github.com/ballerina-platform/ballerina-library/issues/5829) From 19832c00117d42d97048b443dd2824c6da3ec37d Mon Sep 17 00:00:00 2001 From: bhashinee Date: Mon, 1 Apr 2024 11:50:36 +0530 Subject: [PATCH 3/6] Address review suggstions --- .../compiler/PersistModelDefinitionValidator.java | 3 --- .../stdlib/persist/compiler/utils/Utils.java | 2 +- .../compiler/utils/ValidatorsByDatastore.java | 14 +++++++++++++- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/PersistModelDefinitionValidator.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/PersistModelDefinitionValidator.java index ebc7d214..2ea3de93 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/PersistModelDefinitionValidator.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/PersistModelDefinitionValidator.java @@ -134,9 +134,6 @@ public void perform(SyntaxNodeAnalysisContext ctx) { try { datastore = getDatastore(ctx); } catch (BalException e) { - if (e.getMessage().contains("the persist.datastore configuration does not exist")) { - return; - } throw new RuntimeException(e); } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java index 8940892f..e5e7d4a1 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java @@ -221,7 +221,7 @@ private static String getDataStoreName(Path configPath) throws BalException { } } } - throw new BalException("the persist.datastore configuration does not exist in the Ballerina.toml file"); + return null; } catch (IOException e) { throw new BalException("error while reading persist configurations. " + e.getMessage()); } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/ValidatorsByDatastore.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/ValidatorsByDatastore.java index b58d4bbd..811856ca 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/ValidatorsByDatastore.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/ValidatorsByDatastore.java @@ -55,7 +55,7 @@ public static boolean validateSimpleTypes(Entity entity, Node typeNode, String t List> properties, String type, String datastore) { boolean validFlag = true; - if (isOptionalType && datastore.equals(Constants.Datastores.GOOGLE_SHEETS)) { + if (isOptionalType && Constants.Datastores.GOOGLE_SHEETS.equals(datastore)) { entity.reportDiagnostic(PERSIST_308.getCode(), MessageFormat.format(PERSIST_308.getMessage(), type), PERSIST_308.getSeverity(), typeNode.location(), properties); @@ -87,6 +87,10 @@ public static boolean validateSimpleTypes(Entity entity, Node typeNode, String t } public static boolean isValidSimpleType(String type, String datastore) { + // If the datastore is null(ex: generate command), ignore the data type validation. + if (null == datastore) { + return true; + } switch (datastore) { case Constants.Datastores.MYSQL: return isValidMysqlType(type); @@ -104,6 +108,10 @@ public static boolean isValidSimpleType(String type, String datastore) { } public static boolean isValidArrayType(String type, String datastore) { + // If the datastore is null(ex: generate command), ignore the data type validation. + if (null == datastore) { + return true; + } switch (datastore) { case Constants.Datastores.MYSQL: return isValidMysqlArrayType(type); @@ -121,6 +129,10 @@ public static boolean isValidArrayType(String type, String datastore) { } public static boolean isValidImportedType(String modulePrefix, String identifier, String datastore) { + // If the datastore is null(ex: generate command), ignore the data type validation. + if (null == datastore) { + return true; + } switch (datastore) { case Constants.Datastores.MYSQL: return isValidMysqlImportedType(modulePrefix, identifier); From 1097d4f8de8ab17f37e88ac047311125d0ec9b14 Mon Sep 17 00:00:00 2001 From: bhashinee Date: Mon, 1 Apr 2024 12:02:49 +0530 Subject: [PATCH 4/6] Fix the wordings in the code comment --- .../persist/compiler/utils/ValidatorsByDatastore.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/ValidatorsByDatastore.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/ValidatorsByDatastore.java index 95290c41..27e942be 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/ValidatorsByDatastore.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/ValidatorsByDatastore.java @@ -127,7 +127,7 @@ public static boolean validateImportedTypes(Entity entity, Node typeNode, } public static boolean isValidSimpleType(String type, String datastore) { - // Ignore the validation if the datastore is not specified in toml files(ex:before executing generate command). + // If the datastore is null(ex: before executing the generate command), ignore the data type validation. if (null == datastore) { return true; } @@ -150,7 +150,7 @@ public static boolean isValidSimpleType(String type, String datastore) { } public static boolean isValidArrayType(String type, String datastore) { - // Ignore the validation if the datastore is not specified in toml files(ex:before executing generate command). + // If the datastore is null(ex: before executing the generate command), ignore the data type validation. if (null == datastore) { return true; } @@ -173,7 +173,7 @@ public static boolean isValidArrayType(String type, String datastore) { } public static boolean isValidImportedType(String modulePrefix, String identifier, String datastore) { - // Ignore the validation if the datastore is not specified in toml files(ex:before executing generate command). + // If the datastore is null(ex: before executing the generate command), ignore the data type validation. if (null == datastore) { return true; } From ba66f36cc2cffe63005baac170412ffa2b913d96 Mon Sep 17 00:00:00 2001 From: Bhashinee Date: Tue, 2 Apr 2024 10:26:05 +0530 Subject: [PATCH 5/6] Address review suggestions Co-authored-by: Danesh Kuruppu --- .../java/io/ballerina/stdlib/persist/compiler/utils/Utils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java index e5e7d4a1..f0f123be 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java @@ -221,6 +221,7 @@ private static String getDataStoreName(Path configPath) throws BalException { } } } + // Skip data store-specific validations if the `tool.persist` configuration does not exist in the `toml` files, instead of returning an error. return null; } catch (IOException e) { throw new BalException("error while reading persist configurations. " + e.getMessage()); From 125487e3820f49d76783fac04594446dad88a80c Mon Sep 17 00:00:00 2001 From: bhashinee Date: Tue, 2 Apr 2024 10:36:31 +0530 Subject: [PATCH 6/6] Fix the checkstyle issues --- .../java/io/ballerina/stdlib/persist/compiler/utils/Utils.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java index f0f123be..6c98013f 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java @@ -221,7 +221,8 @@ private static String getDataStoreName(Path configPath) throws BalException { } } } - // Skip data store-specific validations if the `tool.persist` configuration does not exist in the `toml` files, instead of returning an error. + // Skip data store-specific validations if the `tool.persist` configuration does not exist in the `toml` + // files, instead of returning an error. return null; } catch (IOException e) { throw new BalException("error while reading persist configurations. " + e.getMessage());