Fix mapping of constraint Date annotation to OpenAPI#1887
Fix mapping of constraint Date annotation to OpenAPI#1887sathindudezoysa wants to merge 5 commits into
Conversation
sathindudezoysa
requested review from
TharmiganK,
daneshk,
lnash94 and
shafreenAnfar
as code owners
📝 WalkthroughWalkthroughThe pull request adds support for date constraints in the Ballerina-to-OpenAPI converter. It introduces new constant definitions, extends the constraint annotation model with message and date-option fields, updates constraint extraction logic to parse and map date constraint metadata, adds detection for Changes
Sequence DiagramsequenceDiagram
participant ConstraintMapper as ConstraintMapper
participant ConstraintAnnotation as ConstraintAnnotation
participant ReferenceTypeMapper as ReferenceTypeMapper
participant SchemaGenerator as SchemaGenerator
ConstraintMapper->>ConstraintMapper: Extract MESSAGE & OPTION fields
ConstraintMapper->>ConstraintAnnotation: withRootMessage(msg)
ConstraintMapper->>ConstraintAnnotation: withDateOption(option)
ConstraintMapper->>ConstraintAnnotation: withOptionMessage(optMsg)
ReferenceTypeMapper->>ReferenceTypeMapper: Check if time:Date type
ReferenceTypeMapper->>SchemaGenerator: Map to StringSchema(format=date)
SchemaGenerator->>SchemaGenerator: Concatenate rootMessage + optionMessage
SchemaGenerator->>SchemaGenerator: Set schema description
SchemaGenerator->>SchemaGenerator: Add x-ballerina-constraint extension
SchemaGenerator-->>ConstraintMapper: Return OpenAPI schema with metadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
openapi-cli/src/test/resources/ballerina-to-openapi/constraint/dateConstraint.bal (1)
20-35: Extend this fixture to includeFUTURE_OR_PRESENTandPAST_OR_PRESENT.The current resource validates only
FUTUREandPAST; adding the other two options here would better lock down the new mapping behavior end-to-end.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi-cli/src/test/resources/ballerina-to-openapi/constraint/dateConstraint.bal` around lines 20 - 35, Update the test fixture by adding constraint:Date annotations that use the FUTURE_OR_PRESENT and PAST_OR_PRESENT options alongside the existing FUTURE and PAST cases: update the MyDate/type-level annotation or add a new typedef (e.g., MyDateFutureOrPresent) with option: constraint:FUTURE_OR_PRESENT and message, and likewise update or add a field/record-level annotation on RegDate (or a new record field) using option: constraint:PAST_OR_PRESENT with an appropriate message; ensure you reference the existing constraint:Date annotations on MyDate and the RegDate.lastLogin field so the new cases exercise both type-level and field-level mappings.ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/constraint/ConstraintMapperImpl.java (1)
468-478: Reuse the literal sanitizer in both paths.Lines 468-469 and Lines 477-478 are stripping quotes separately. Reusing
sanitizeStringLiteral()fromextractFieldValue()keeps the behavior aligned and removes one copy of the regex.♻️ Suggested cleanup
private String sanitizeStringLiteral(String raw) { - return raw.trim().replaceAll("^\"|\"$", ""); + return raw.trim().replaceAll("(^\")|(\"$)", ""); } @@ - case STRING_LITERAL: - return Optional.of(exprNode.toString().trim().replaceAll("^\"|\"$", "")); + case STRING_LITERAL: + return Optional.of(sanitizeStringLiteral(exprNode.toString()));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/constraint/ConstraintMapperImpl.java` around lines 468 - 478, The code duplicates quote-stripping logic: replace the inline replaceAll in extractFieldValue with a call to the existing sanitizeStringLiteral(String) helper so both NUMERIC_LITERAL and STRING_LITERAL paths reuse the same sanitizer; update extractFieldValue to invoke sanitizeStringLiteral(exprNode.toString().trim()) for STRING_LITERAL and remove the duplicated regex call so behavior is consistent with sanitizeStringLiteral.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/constraint/ConstraintMapperImpl.java`:
- Around line 257-265: ConstraintMapperImpl currently builds a description from
constraintAnnot.getRootMessage()/getOptionMessage() and then calls
properties.setDescription(...), which overwrites any existing docs-derived
description; change this to preserve and merge with an existing description on
the schema: read the current properties.getDescription() (if any), build the
constraint description string from
constraintAnnot.getRootMessage()/getOptionMessage(), then set
properties.setDescription(combined.trim()) where combined is either the existing
description plus a separator and the constraint description or the constraint
description alone if no existing description exists, ensuring you do not clobber
prior documentation.
In
`@ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/type/ReferenceTypeMapper.java`:
- Around line 86-91: Update isTimeDateType to validate the module org as well as
the module name: inside isTimeDateType(TypeReferenceTypeSymbol typeSymbol)
refine the module.filter predicate to check
module.id().orgName().equals("ballerina") in addition to
module.getName().orElse("").equals("time"), and use the same defensive
string-equals pattern for the type name check (i.e., compare "Date" against
typeSymbol.getName().orElse("")) so the method matches the org-level validation
used in ValidatorUtils/MapperCommonUtils/BallerinaTypeExtensioner.
---
Nitpick comments:
In
`@ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/constraint/ConstraintMapperImpl.java`:
- Around line 468-478: The code duplicates quote-stripping logic: replace the
inline replaceAll in extractFieldValue with a call to the existing
sanitizeStringLiteral(String) helper so both NUMERIC_LITERAL and STRING_LITERAL
paths reuse the same sanitizer; update extractFieldValue to invoke
sanitizeStringLiteral(exprNode.toString().trim()) for STRING_LITERAL and remove
the duplicated regex call so behavior is consistent with sanitizeStringLiteral.
In
`@openapi-cli/src/test/resources/ballerina-to-openapi/constraint/dateConstraint.bal`:
- Around line 20-35: Update the test fixture by adding constraint:Date
annotations that use the FUTURE_OR_PRESENT and PAST_OR_PRESENT options alongside
the existing FUTURE and PAST cases: update the MyDate/type-level annotation or
add a new typedef (e.g., MyDateFutureOrPresent) with option:
constraint:FUTURE_OR_PRESENT and message, and likewise update or add a
field/record-level annotation on RegDate (or a new record field) using option:
constraint:PAST_OR_PRESENT with an appropriate message; ensure you reference the
existing constraint:Date annotations on MyDate and the RegDate.lastLogin field
so the new cases exercise both type-level and field-level mappings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: de91e46d-fa29-43f1-b27c-c0b83357d123
📒 Files selected for processing (10)
ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/constraint/Constants.javaballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/constraint/ConstraintAnnotation.javaballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/constraint/ConstraintMapperImpl.javaballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/type/ReferenceTypeMapper.javaopenapi-cli/src/test/java/io/ballerina/openapi/generators/openapi/ConstraintTests.javaopenapi-cli/src/test/resources/ballerina-to-openapi/constraint/dateConstraint.balopenapi-cli/src/test/resources/ballerina-to-openapi/expected_gen/constraint/expectedDate.yamlopenapi-tool/BalTool.tomlopenapi-tool/Ballerina.tomlopenapi-tool/Dependencies.toml
| String description = ""; | ||
| if (constraintAnnot.getRootMessage().isPresent()) { | ||
| description += constraintAnnot.getRootMessage().get(); | ||
| } | ||
| if (constraintAnnot.getOptionMessage().isPresent()) { | ||
| description += (description.isEmpty() ? "" : " ") + constraintAnnot.getOptionMessage().get(); | ||
| } | ||
| if (!description.isEmpty()) { | ||
| properties.setDescription(description.trim()); |
There was a problem hiding this comment.
Don't clobber existing schema descriptions.
Line 265 replaces any description that was already populated from Ballerina docs. For schemas or fields that have both normal documentation and a date-constraint message, this regresses the generated OpenAPI docs.
🛠️ Suggested fix
- if (!description.isEmpty()) {
- properties.setDescription(description.trim());
- }
+ if (!description.isEmpty()) {
+ String constraintDescription = description.trim();
+ String existingDescription = properties.getDescription();
+ properties.setDescription(existingDescription == null || existingDescription.isBlank()
+ ? constraintDescription
+ : existingDescription + " " + constraintDescription);
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/constraint/ConstraintMapperImpl.java`
around lines 257 - 265, ConstraintMapperImpl currently builds a description from
constraintAnnot.getRootMessage()/getOptionMessage() and then calls
properties.setDescription(...), which overwrites any existing docs-derived
description; change this to preserve and merge with an existing description on
the schema: read the current properties.getDescription() (if any), build the
constraint description string from
constraintAnnot.getRootMessage()/getOptionMessage(), then set
properties.setDescription(combined.trim()) where combined is either the existing
description plus a separator and the constraint description or the constraint
description alone if no existing description exists, ensuring you do not clobber
prior documentation.
| public static boolean isTimeDateType(TypeReferenceTypeSymbol typeSymbol) { | ||
| return typeSymbol.getModule() | ||
| .filter(module -> module.getName().orElse("").equals("time")) | ||
| .isPresent() | ||
| && typeSymbol.getName().orElse("").equals("Date"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify existing module identity-check patterns and possible conflicting time imports.
set -euo pipefail
echo "== Existing Java patterns that check module org/name =="
rg -nP 'orgName\s*\(|getName\(\)\.orElse\(""\)\.equals\("time"\)|module\.getName\(' -g '*.java'
echo
echo "== Ballerina imports of */time modules (potential conflicts) =="
rg -nP '^\s*import\s+[^;]*/time\s*;' -g '*.bal'
echo
echo "== Explicit non-ballerina time imports (if any) =="
rg -nP '^\s*import\s+(?!ballerina/time)[^;]*/time\s*;' -g '*.bal' || trueRepository: ballerina-platform/openapi-tools
Length of output: 4539
🏁 Script executed:
cat -n ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/type/ReferenceTypeMapper.java | head -100Repository: ballerina-platform/openapi-tools
Length of output: 4995
🏁 Script executed:
sed -n '385,395p' ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/utils/MapperCommonUtils.javaRepository: ballerina-platform/openapi-tools
Length of output: 394
🏁 Script executed:
sed -n '175,185p' ballerina-to-openapi/src/main/java/io/ballerina/openapi/validator/ValidatorUtils.javaRepository: ballerina-platform/openapi-tools
Length of output: 204
🏁 Script executed:
sed -n '175,185p' openapi-validator/src/main/java/io/ballerina/openapi/validator/ValidatorUtils.javaRepository: ballerina-platform/openapi-tools
Length of output: 398
🏁 Script executed:
rg -A 3 'getModule\(\)' ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/type/ReferenceTypeMapper.javaRepository: ballerina-platform/openapi-tools
Length of output: 271
🏁 Script executed:
rg -B 5 'getModule\(\)' ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/type/ReferenceTypeMapper.java | head -20Repository: ballerina-platform/openapi-tools
Length of output: 235
🏁 Script executed:
rg 'ModuleSymbol' ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/type/ReferenceTypeMapper.javaRepository: ballerina-platform/openapi-tools
Length of output: 58
Add org-level validation to isTimeDateType to match established codebase patterns.
The method currently validates only module.getName().equals("time") without checking the org name. Update to also validate module.id().orgName().equals("ballerina") to match the pattern used consistently across ValidatorUtils, MapperCommonUtils, and BallerinaTypeExtensioner.
Suggested fix pattern
public static boolean isTimeDateType(TypeReferenceTypeSymbol typeSymbol) {
return typeSymbol.getModule()
.filter(module -> "ballerina".equals(module.id().orgName())
&& "time".equals(module.getName().orElse("")))
.isPresent()
&& "Date".equals(typeSymbol.getName().orElse(""));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ballerina-to-openapi/src/main/java/io/ballerina/openapi/service/mapper/type/ReferenceTypeMapper.java`
around lines 86 - 91, Update isTimeDateType to validate the module org as well
as the module name: inside isTimeDateType(TypeReferenceTypeSymbol typeSymbol)
refine the module.filter predicate to check
module.id().orgName().equals("ballerina") in addition to
module.getName().orElse("").equals("time"), and use the same defensive
string-equals pattern for the type name check (i.e., compare "Date" against
typeSymbol.getName().orElse("")) so the method matches the org-level validation
used in ValidatorUtils/MapperCommonUtils/BallerinaTypeExtensioner.
|
Hi everyone, For this PR, I decided to tackle a feature request that's been open for a while. Since this is a bigger implementation, I'd love to get some eyes on it. I'm still getting familiar with the project's preferred logic patterns and coding styles, so I highly welcome any feedback, suggestions, or corrections. Looking forward to your reviews so I can refine this and get it right! |
|
Hi, @TharmiganK @lnash94 @daneshk @shafreenAnfar As sachin mentioned in the issue I mapped the date fields to type:string with format:date. Then fI map the options and message using the x-ballerina-constraint extension. Finally I created a test case to test with the system. Here following screenshot shows the all the unit test cases with my test case pass when running I know my code has issues in formatting to the current standards in ballerina library. Please can anyone help me to fix those. |
| private final String rootMessage; | ||
| private final String optionMessage; |
There was a problem hiding this comment.
Why do we need these two fields?
| * This util is used to sanitize a string literal by trimming whitespace and removing surrounding quotes. | ||
| */ | ||
| private String sanitizeStringLiteral(String raw) { | ||
| return raw.trim().replaceAll("^\"|\"$", ""); |
There was a problem hiding this comment.
Shall we make this "^\"|\"$" to a constant value?
| @@ -0,0 +1,42 @@ | |||
| // Copyright (c) 2023, WSO2 LLC. (http://www.wso2.org). | |||
There was a problem hiding this comment.
Update the licenses header to 2026
|
Hi @sathindudezoysa, Thank you for your contribution! Sorry for the late response. For this issue, we actually need the mapping of how we are going to represent the constraint annotation information in the generated OpenAPI specification. This requires a design for specification and approval. After the approval we can check on the implementation. Can you please update the issue with your proposed mapping? We already have a specification of how Ballerina constructs are mapped to OpenAPI specification: https://github.com/ballerina-platform/openapi-tools/blob/master/docs/ballerina-to-oas/spec/spec.md. You can refer to this to get some idea. You have to refer to the OpenAPI schema as well to figure out a proper mapping. Once the proposed mapping is approved in the issue, we can review your implementation. |
|
Thanks for the guidance and the resources. That makes perfect sense! Since we need to finalize and approve the design before jumping into the code, I will go ahead and close this PR for now to keep things clean. I will study the specifications, design a proper mapping method, and raise a new issue with my proposal so we can discuss it there. Thanks again for the help! |
3 participants
Resolves #5049
Description
Summary of user stories addressed by this change:
Resolves the issue where Ballerina
@constraint:Dateannotations were not being accurately mapped to the OpenAPI specification during Ballerina to OpenAPI conversion.Brief description of the nature of the feature or bug-fix:
This PR adds support for mapping the
@constraint:Dateannotation into the generated OpenAPI specification. Since OpenAPI does not have a nativeDatetype, this change ensures that date constraints are mapped totype: stringwithformat: date. It also includes logic to handle specific date validation options such asPAST,FUTURE,PAST_OR_PRESENT, andFUTURE_OR_PRESENTby properly formatting the OpenAPI schema and mapping the relevant patterns or constraints.Testing
Code coverage information:
@constraint:Dateannotation mapping.Testing done (Unit, Integration, etc.):
FUTURE,PAST,FUTURE_OR_PRESENT, andPAST_OR_PRESENTconstraint options.Test environments:
Security Check
Summary
This pull request adds support for mapping the Ballerina
@constraint:Dateannotation to OpenAPI format during Ballerina-to-OpenAPI conversion. Since OpenAPI does not have a native date type, date constraints are mapped totype: stringwithformat: date, while date validation options (PAST,FUTURE,PAST_OR_PRESENT,FUTURE_OR_PRESENT) are preserved through an OpenAPI extension (x-ballerina-constraint).Changes
Core Implementation:
ConstraintAnnotationclass to track date-related constraint metadata, including root validation message, option message, and the specific date constraint optionConstraintMapperImplto parse and process date constraint annotations, handling both simple option references and structured constraint configurationsReferenceTypeMapperto detecttime:Datereference types and map them to string schemas with date format instead of treating them as component objectsConfiguration & Versioning:
Ballerina.toml,BalTool.toml, andDependencies.tomlfrom2.4.1to2.4.2-SNAPSHOTor2.4.2Test Coverage:
The implementation ensures that date constraints are properly represented in the generated OpenAPI specification, allowing API documentation to reflect date validation requirements from the Ballerina service definitions.