Engage persist compiler plugin with persist multi-model support#318
Engage persist compiler plugin with persist multi-model support#318TharmiganK merged 9 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds multi-model support: new PersistModelInformation and Utils APIs to locate Ballerina.toml and resolve model-specific datastores, Constants for Changes
Sequence Diagram(s)sequenceDiagram
participant Compiler as Compiler (SyntaxNodeAnalysisContext)
participant Validator as PersistModelDefinitionValidator
participant Utils as Utils
participant FileSystem as Ballerina.toml / persist files
Compiler->>Validator: provide SyntaxNodeAnalysisContext
Validator->>Utils: getPersistModelInfo(ctx)
Utils->>FileSystem: locate Ballerina.toml and persist entries (model, filePath)
FileSystem-->>Utils: tomlPath + entries
Utils-->>Validator: PersistModelInformation(modelName, tomlPath)
Validator->>Utils: getDatastore(tomlPath, modelName)
Utils->>FileSystem: extract datastore for matching model/filePath
FileSystem-->>Utils: datastoreName
Utils-->>Validator: datastoreName
Validator->>Compiler: validate model definitions & emit diagnostics
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #318 +/- ##
============================================
+ Coverage 87.71% 87.79% +0.08%
- Complexity 373 389 +16
============================================
Files 30 30
Lines 1188 1229 +41
Branches 149 153 +4
============================================
+ Hits 1042 1079 +37
- Misses 92 94 +2
- Partials 54 56 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@compiler-plugin-test/src/test/resources/project_12/Ballerina.toml`:
- Around line 6-9: The TOML entry under [[tool.persist]] points filePath to
"persist/medical/model.bal" which does not exist; change the filePath value to
one of the actual model files in persist/medical (for example
"persist/medical/valid-persist-model-path.bal" or the appropriate model file
among "field-types.bal"/"record-properties.bal") so the [[tool.persist]]
configuration references an existing model file; update the filePath key in the
Ballerina.toml accordingly.
In `@compiler-plugin-test/src/test/resources/project_13/Ballerina.toml`:
- Around line 6-9: The Ballerina.toml entry under [[tool.persist]] for id
"project13" references a non-existent model file via filePath
"persist/inventory/model.bal"; either add the missing file at that path or
update the filePath to the correct existing file (e.g.,
"persist/inventory/field-types.bal") so the tool.persist configuration points to
a valid model file and the project can resolve the model during compilation.
In `@compiler-plugin-test/src/test/resources/project_14/Ballerina.toml`:
- Around line 6-12: The Ballerina.toml references non-existent model.bal files
under the [[tool.persist]] entries; either update the file paths or rename test
files—easiest fix: change the persist filePath values to point to the existing
field-types.bal files (e.g., set the primary [[tool.persist]] to use filePath
"persist/field-types.bal" and the secondary [[tool.persist]] to use filePath
"persist/secondary/field-types.bal"), keeping the existing
options.datastore/model keys intact so the runtime finds the correct resource
files.
In
`@compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java`:
- Around line 188-190: In Utils.java replace the empty catch block for
UnsupportedOperationException in the path-resolution code with proper logging:
obtain the project logger used across the module (e.g., a class-level org.slf4j
Logger in Utils) and call logger.error or logger.warn with a clear message like
"Failed to resolve path" and include the exception (e) so the stacktrace is
recorded; if the resolution failure should stop processing, rethrow or wrap the
exception (e.g., throw new RuntimeException(..., e)) instead of silently
swallowing it.
🧹 Nitpick comments (3)
compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/PersistModelDefinitionValidator.java (1)
134-139: Consider more specific exception handling.Wrapping the checked
BalExceptionin an uncheckedRuntimeExceptionloses context and could make debugging difficult. Consider logging the error or using a more specific runtime exception with a descriptive message.Proposed improvement
try { datastore = getDatastore(ballerinaTomlPath, persistModelInformation.modelName()); } catch (BalException e) { - throw new RuntimeException(e); + throw new RuntimeException("Failed to resolve datastore for model: " + + persistModelInformation.modelName(), e); }compiler-plugin-test/src/test/java/io/ballerina/stdlib/persist/compiler/CompilerPluginTest.java (1)
819-827: Test comment implies new behavior but tests existing project.The comment states "A file in a non-persist subdirectory should not be identified as a model file", but this test uses
project_1/resources, which is the same as the existingidentifyModelFileFailure2test. Consider using a multi-model project directory to specifically test multi-model edge cases.compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java (1)
361-363: Potential NPE if field.value() returns null.The
getValuemethod callsfield.value().toSourceCode()without null-checkingfield.value(). While TOML syntax typically requires values, defensive programming would add a null check.Proposed defensive null check
private static String getValue(KeyValueNode field) { - return field.value().toSourceCode().trim().replace("\"", ""); + var value = field.value(); + return value != null ? value.toSourceCode().trim().replace("\"", "") : null; }
compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java
Show resolved
Hide resolved
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @TharmiganK. * #318 (comment) The following files were modified: * `compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/PersistModelDefinitionValidator.java` * `compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java`
e1a161a to
2079071
Compare
There was a problem hiding this comment.
Pull request overview
Updates the persist compiler plugin to apply validations for the new multi-model persist directory structure by resolving the correct model context (model name + associated Ballerina.toml) and selecting model-specific datastore configuration.
Changes:
- Added persist model resolution utilities (model name +
Ballerina.tomlpath) and updated datastore resolution to be model-aware. - Updated
PersistModelDefinitionValidatorto use model info + model-scoped datastore lookup for validations. - Added multi-model test fixtures and new test cases; bumped module/plugin versions and updated changelog.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle.properties | Bumps project version to 1.7.0-SNAPSHOT. |
| compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java | Adds model discovery + model-aware datastore extraction from TOML. |
| compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/PersistModelDefinitionValidator.java | Switches validation entry logic to use resolved model/TOML info and model-scoped datastore lookup. |
| compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/Constants.java | Introduces model and filePath TOML key constants. |
| compiler-plugin-test/src/test/resources/project_14/persist/secondary/field-types.bal | Adds multi-model fixture file. |
| compiler-plugin-test/src/test/resources/project_14/persist/field-types.bal | Adds default-model fixture file. |
| compiler-plugin-test/src/test/resources/project_14/Ballerina.toml | Adds TOML with default + named model [[tool.persist]] entries. |
| compiler-plugin-test/src/test/resources/project_13/persist/inventory/field-types.bal | Adds multi-model fixture file. |
| compiler-plugin-test/src/test/resources/project_13/Ballerina.toml | Adds model selection via filePath fixture. |
| compiler-plugin-test/src/test/resources/project_12/persist/medical/valid-persist-model-path.bal | Adds multi-model fixture for model detection. |
| compiler-plugin-test/src/test/resources/project_12/persist/medical/record-properties.bal | Adds multi-model fixture for record validations. |
| compiler-plugin-test/src/test/resources/project_12/persist/medical/field-types.bal | Adds multi-model fixture for type validations. |
| compiler-plugin-test/src/test/resources/project_12/Ballerina.toml | Adds multi-model TOML fixture for model selection. |
| compiler-plugin-test/src/test/java/io/ballerina/stdlib/persist/compiler/CompilerPluginTest.java | Adds multi-model test coverage and helpers. |
| changelog.md | Notes multi-model validation support in Unreleased section. |
| ballerina/Dependencies.toml | Bumps dependency version to 1.7.0. |
| ballerina/CompilerPlugin.toml | Updates embedded plugin jar path to 1.7.0-SNAPSHOT. |
| ballerina/Ballerina.toml | Bumps package and platform dependency versions/paths to 1.7.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ugin/src/main/java/io/ballerina/stdlib/persist/compiler/PersistModelDefinitionValidator.java
Show resolved
Hide resolved
compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java`:
- Around line 198-208: getPersistModelInfo currently only handles
SINGLE_FILE_PROJECT and returns an empty PersistModelInformation for
BuildProject/other kinds, causing PersistModelDefinitionValidator.perform to
skip validations; fix by adding an else branch that resolves the current
document's path for non-single-file projects (use the SyntaxNodeAnalysisContext
APIs to get the source file path for the node—e.g., obtain the file path from
ctx.node().location().lineRange().filePath() or the equivalent filePath()
accessor exposed by the analysis/context API) and pass that resolved Path into
getPersistModelInformation so getPersistModelInfo returns a populated
PersistModelInformation for BuildProject too; keep the try/catch for
UnsupportedOperationException and ensure no null ballerinaTomlPath is returned.
compiler-plugin/src/main/java/io/ballerina/stdlib/persist/compiler/utils/Utils.java
Show resolved
Hide resolved
compiler-plugin-test/src/test/resources/project_12/persist/medical/record-properties.bal
Show resolved
Hide resolved
daneshk
left a comment
There was a problem hiding this comment.
Let's add License headers to test resource files. Others LGTM
|



Purpose
Before the multi-model support, only the model file in the persist directory gets the compiler validations for persist package. But with the new multi-model structure, we need to engage the compiler plugin to the sub-directories which represent the models
Part of: ballerina-platform/ballerina-library#8503
Examples
N/A
Checklist
Updated the specificationChecked native-image compatibilityThis pull request enables the persist compiler plugin to run validations in repositories that organize persistence models into multiple subdirectories. Previously the plugin only validated a single model file under persist/; with these changes it discovers model-specific metadata, scopes validations per model, and resolves per-model configuration.
Key changes
Model-aware configuration
Validator updates
Utilities and TOML parsing
Tests and fixtures
Release and metadata
Outcome