feat: Support local file metadata for OData clients#210
feat: Support local file metadata for OData clients#210dionesiusap wants to merge 6 commits intomendixlabs:mainfrom
Conversation
Extends CREATE ODATA CLIENT MetadataUrl to accept local file paths in addition to HTTP(S) URLs, enabling offline development and reproducible testing with metadata snapshots. Supported formats: - https://... or http://... (existing HTTP fetch) - file:///abs/path (local absolute path) - ./path or path/file.xml (local relative path) Path resolution: - With project loaded: relative to .mpr directory - Without project: relative to cwd Implementation: - Created internal/pathutil package with URIToPath() helper - Refactored cmd/mxcli/lsp_helpers.go to use shared utility - Updated fetchODataMetadata() to detect and handle local files - Added comprehensive test coverage Closes mendixlabs#206 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Updated OData-related skill files to comprehensively document all three MetadataUrl formats with complete examples. Changes to odata-data-sharing.md: - Added new "MetadataUrl Formats" section with table and use cases - Updated Step 4 examples to show all formats: * HTTP(S) URL (production) * Relative path with ./ (offline development) * Relative path without ./ * Absolute file:// URI - Updated "Folder Organization" section with all formats - Updated checklist with detailed MetadataUrl options Changes to browse-integrations.md: - Added note about local file support with all three formats - Documented path resolution behavior - Listed use cases for local metadata files Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
AI Code ReviewRecommendation: ApproveThe PR thoroughly implements local file metadata support for OData clients following all requirements. It properly handles:
All checklist items are satisfied with only minor harmless whitespace changes in some AST files. The feature is well-scoped, follows MDL design guidelines, and maintains full-stack consistency through executor-level changes (no new syntax elements were added requiring grammar/AST/visitor updates). The breaking change (ServiceUrl validation) is properly documented and enforces existing Mendix best practices. Automated review via OpenRouter (Nemotron Super 120B) — workflow source |
ReviewOverall: good feature, well-tested, but the diff is noisier than it should be and has two concrete bugs worth fixing. The core design (support What I like
Concerns
Minor
VerdictApprove after (1) Windows URL fix, (2) deduplicate fetcher resolution logic, (3) error message improvement, (4) |
Fixes four issues identified by @ako in code review: 1. Windows file:// URL RFC 8089 compliance - Add leading slash for Windows paths: file:///C:/path - Previously: file://C:/path (incorrect, treats C: as host) - Now: file:///C:/path (correct per RFC 8089) 2. Deduplicate path resolution in fetchODataMetadata - Remove redundant file:// extraction and relative path resolution - Now relies on NormalizeURL() already called in createODataClient - Simplify to: PathFromURL() + os.ReadFile() - Remove unused mprDir parameter 3. Improve ServiceUrl validation error message - Add migration hint explaining the breaking change - Show how to create constant: CREATE CONSTANT Module.ApiLocation - Explain this enforces Mendix best practice 4. Fix unsafe os.Chdir in tests - Replace TestFetchODataMetadata_RelativePathWithoutProject - New test uses NormalizeURL() to generate file:// URL - No process-global state mutation - Tests now expect normalized URLs (matching production flow) All tests pass. Addresses review comments without changing functionality. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
3497b65 to
dbf3943
Compare
AI Code ReviewCritical Issues
Moderate Issues
Minor Issues
What Looks Good
RecommendationApprove - The PR fully implements the requested feature following all architectural guidelines, includes proper testing and documentation, and maintains backward compatibility while adding valuable functionality for offline development and testing scenarios. Automated review via OpenRouter (Nemotron Super 120B) — workflow source |
Two improvements for Studio Pro compatibility:
1. **Normalize relative paths to absolute file:// URLs**
- Relative paths (./path or path/file.xml) are automatically converted
to absolute file:// URLs in the Mendix model
- Ensures Studio Pro can detect local file vs HTTP metadata sources
- Example: './metadata.xml' → 'file:///absolute/path/to/project/metadata.xml'
2. **Enforce ServiceUrl as constant reference**
- ServiceUrl must always start with '@' (e.g., '@Module.ConstantName')
- Direct URLs are rejected with clear error message
- Enforces Mendix best practice of externalizing configuration
Implementation:
- Added normalizeMetadataUrl() function with path resolution logic
- Added validation in createODataClient() for ServiceUrl format
- Updated all documentation and examples
- Added comprehensive test coverage
- Added test MPK file with real Studio Pro-created services
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Extracted URL normalization logic into internal/pathutil for reuse across components (OData clients, REST clients with OpenAPI, etc.). Changes: - Added pathutil.NormalizeURL() - converts relative paths to absolute file:// URLs - Added pathutil.PathFromURL() - extracts filesystem path from file:// URLs - Removed duplicate normalizeMetadataUrl() from cmd_odata.go - Updated executor to use pathutil.NormalizeURL() - Added comprehensive tests for new functions Benefits: - Single source of truth for URL normalization logic - Reusable for REST clients with OpenAPI contracts - Better test coverage - Consistent behavior across all components Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fixes four issues identified by @ako in code review: 1. Windows file:// URL RFC 8089 compliance - Add leading slash for Windows paths: file:///C:/path - Previously: file://C:/path (incorrect, treats C: as host) - Now: file:///C:/path (correct per RFC 8089) 2. Deduplicate path resolution in fetchODataMetadata - Remove redundant file:// extraction and relative path resolution - Now relies on NormalizeURL() already called in createODataClient - Simplify to: PathFromURL() + os.ReadFile() - Remove unused mprDir parameter 3. Improve ServiceUrl validation error message - Add migration hint explaining the breaking change - Show how to create constant: CREATE CONSTANT Module.ApiLocation - Explain this enforces Mendix best practice 4. Fix unsafe os.Chdir in tests - Replace TestFetchODataMetadata_RelativePathWithoutProject - New test uses NormalizeURL() to generate file:// URL - No process-global state mutation - Tests now expect normalized URLs (matching production flow) All tests pass. Addresses review comments without changing functionality. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
dbf3943 to
a2057f8
Compare
AI Code ReviewWhat Looks Good
Automated review via OpenRouter (Nemotron Super 120B) — workflow source |
|
Thanks for the detailed review @ako! I've addressed all the concerns: Changes Made1. Windows file:// URL bug (RFC 8089 compliance) ✅Fixed in: Added leading slash logic for Windows paths to ensure RFC 8089 compliance: // RFC 8089 requires three slashes: file:///path or file:///C:/path
slashed := filepath.ToSlash(absPath)
if !strings.HasPrefix(slashed, "/") {
// Windows path like C:/Users/x needs leading slash: file:///C:/Users/x
slashed = "/" + slashed
}
return "file://" + slashed, nilWindows paths now correctly format as 2. Duplicated path resolution ✅Fixed in: Removed the duplicated // At this point, metadataUrl is already normalized by NormalizeURL() in createODataClient:
// - Relative paths have been converted to absolute file:// URLs
// - HTTP(S) URLs are unchanged
// So we only need to distinguish file:// vs HTTP(S)
filePath := pathutil.PathFromURL(metadataUrl)
if filePath != "" {
// Local file - read directly (path is already absolute)
body, err = os.ReadFile(filePath)
// ...
}Also removed the unused 3. ServiceUrl validation error message ✅Fixed in: Improved the error message with migration hint: return fmt.Errorf(`ServiceUrl must now be a constant reference (e.g., '@Module.ApiLocation').
Previously literal URLs were allowed; this enforces the Mendix best practice of externalizing configuration.
Create a constant first:
CREATE CONSTANT Module.ApiLocation TYPE String DEFAULT 'https://api.example.com/';
Then reference it:
ServiceUrl: '@Module.ApiLocation'
Got: %s`, stmt.ServiceUrl)4. Unsafe os.Chdir in tests ✅Fixed in: Replaced 5. go fmt noise ✅Fixed: Rebased to remove commit The commit that reformatted 14 unrelated files has been removed from the branch history. The PR now only contains feature changes, making it much easier to review. 6. Binary fixture in git history ✅Fixed: Removed The 19MB mpk file has been completely removed from all commits. It now lives at 7. Security note about file access 📝Acknowledged: File access is already sandboxed Good point about the security implication. The That said, we should document this in the skills file to make it clear that All commits have been cleaned and force-pushed. The branch is ready for another review round. |
Support local file metadata for OData clients
Closes #206
Summary
Adds support for local
file://URLs and relative file paths inCREATE ODATA CLIENTMetadataUrlparameter, enabling offline development, reproducible testing, and version-pinned contracts.Changes
1. Local File Support
https://api.example.com/$metadata(existing)file:///absolute/path/to/metadata.xml./metadata/service.xmlormetadata/service.xml2. Path Normalization for Studio Pro Compatibility
Relative paths are automatically converted to absolute
file://URLs in the Mendix model. This ensures Studio Pro's UI can properly detect local file vs HTTP metadata sources.Example:
3. ServiceUrl Validation
ServiceUrlmust now be a constant reference (prefixed with@) to enforce Mendix best practice of externalizing configuration.Example:
4. Shared Utility Package
Extracted URL normalization logic into
internal/pathutilfor reuse across components:pathutil.NormalizeURL()- converts relative paths to absolute file:// URLspathutil.URIToPath()- converts file:// URLs to filesystem pathspathutil.PathFromURL()- extracts path from file:// URLsBenefits:
Use Cases
Implementation Details
Path Resolution
-pflag or REPL): relative paths resolved against.mprdirectorycwdMetadata Caching
Local files are read and cached identically to HTTP-fetched metadata:
MetadatafieldExamples
Testing
mx-test-projects/ODataProject.mpk)Validation
make buildpassesmake testpasses (all tests)make lintpassesAgentic Code Testing
.claude/skills/mendix/odata-data-sharing.mdandbrowse-integrations.md)Documentation
docs/01-project/MDL_QUICK_REFERENCE.md.claude/skills/mendix/odata-data-sharing.md.claude/skills/mendix/browse-integrations.mdmdl-examples/odata-local-metadata/Breaking Changes
@Module.ConstantName).This enforces existing Mendix best practice and should only affect scripts that weren't following the recommended pattern.
Commits
064b511- feat: support file:// URLs and local paths for OData metadata4ad2cfe- docs: update skills with comprehensive local metadata examples856a5e2- feat: normalize relative paths and enforce ServiceUrl as constantc508570- refactor: move URL normalization to shared pathutil package