Improve data service mediator and tryout#1526
Merged
Merged
Conversation
ChinthakaJ98
added
Checks/Run MI UI Tests
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
packages/mi-language-server/org.eclipse.lemminx/src/main/resources/org/eclipse/lemminx/mediators/430/ui-schemas/dataServiceCall.json (1)
85-265: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated
DSSPropertiessub-schema across three tables.The
DSSPropertiesnested table definition (propertyName/propertyValueType/propertyValue/propertyExpression) is repeated verbatim insingleOperations,batchOperations, andrequestBoxoperations. If the schema format supports shared/referenced sub-definitions, extracting this would reduce duplication and future drift risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mi-language-server/org.eclipse.lemminx/src/main/resources/org/eclipse/lemminx/mediators/430/ui-schemas/dataServiceCall.json` around lines 85 - 265, The DSSProperties nested table schema is duplicated across multiple operation tables, so update the dataServiceCall.json schema to reuse a shared sub-definition instead of repeating the same propertyName/propertyValueType/propertyValue/propertyExpression block in singleOperations, batchOperations, and requestBoxoperations. Extract that repeated table into one reusable definition and reference it from each table’s elements so future changes stay in sync; use the existing DSSProperties, singleOperations, and batchOperations symbols to locate the repeated schema sections.packages/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts (1)
5933-5938: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReuse a single proxy server instead of spinning up a new one per call.
Each call starts a new
cors-anywhereserver on a freshly-allocated port that is never closed. This mirrors the existing pattern ingetOpenAPISpec/editOpenAPISpec, but now a third call site duplicates it. Repeated "Try it" clicks over a session will accumulate listening servers. Consider extracting a shared helper that lazily starts and reuses a single proxy instance across all three call sites.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts` around lines 5933 - 5938, The proxy startup logic in the RPC manager creates a fresh cors-anywhere server on every call, which can leak listeners across repeated “Try it” actions. Extract the shared proxy setup used by getOpenAPISpec and editOpenAPISpec into a reusable helper in rpc-manager.ts that lazily initializes the proxy once and returns the same instance/port on subsequent calls. Update this new call site to use that helper instead of directly calling getPortPromise and cors_proxy.createServer.packages/mi-language-server/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/resourceFinder/AbstractResourceFinder.java (1)
700-716: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueJavadoc example doesn't match actual output.
Per WSO2 MI's documented naming convention, the runtime uses
_<method><path>with no separator between method and path (e.g._poststudent). This implementation matches that convention (correctly), but the method's own javadoc example —(path="student/{id}", method="PUT")yields_put_student_id— has a spurious underscore; the actual output would be_putstudent_id. Consider correcting the javadoc example to avoid confusing future maintainers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mi-language-server/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/resourceFinder/AbstractResourceFinder.java` around lines 700 - 716, The Javadoc example in generateRequestName is inconsistent with the actual naming logic and MI’s documented convention. Update the example in AbstractResourceFinder.generateRequestName so it reflects the real output from the current path/method transformation (no extra separator between method and path), keeping the description aligned with the implementation and the runtime naming rule.packages/mi-diagram/src/components/Form/Keylookup/Keylookup.tsx (1)
279-293: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: skip the fetch when no data service is selected yet.
For
filterType === "dsOperation"with nodataServiceNameyet, an RPC round trip is still made (the server-side branch degrades gracefully to an empty/irrelevant lookup). Short-circuiting locally would avoid the unnecessary request while the user hasn't picked a data service.♻️ Proposed optimization
+ if (filterType === "dsOperation" && !props.dataServiceName) { + setItems([]); + return; + } + let resourceType: ResourceType | MultipleResourceType[];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mi-diagram/src/components/Form/Keylookup/Keylookup.tsx` around lines 279 - 293, Short-circuit the lookup in Keylookup before calling getAvailableResources when filterType is "dsOperation" and props.dataServiceName is not set, so no unnecessary RPC request is made. Update the logic around the resourceType selection and the rpcClient.getMiDiagramRpcClient().getAvailableResources call to return early with an empty result (or equivalent no-op state) in this case, while preserving the existing behavior for bindToInbound, array filterType, and all other resource lookups.packages/mi-language-server/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/resourceFinder/pojo/DataServiceOperationResource.java (1)
22-28: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider setting a
typeon the resource.
DataServiceOperationResourceonly setsname; the baseResource.typefield is left null. Callers inKeylookup.tsxuseresource.typeas the display-type for the dropdown item icon/badge, so dsOperation entries may render without a proper type indicator.♻️ Proposed fix
public class DataServiceOperationResource extends Resource { public DataServiceOperationResource(String name) { setName(name); + setType("dsOperation"); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mi-language-server/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/resourceFinder/pojo/DataServiceOperationResource.java` around lines 22 - 28, `DataServiceOperationResource` only initializes the name, so its inherited `Resource.type` remains null and downstream UI in `Keylookup.tsx` cannot render the correct badge/icon. Update the `DataServiceOperationResource` constructor to also assign an appropriate type value for data service operations, using the existing `Resource`/`DataServiceOperationResource` symbols to locate the initialization point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts`:
- Around line 5928-5932: The swagger fetch in getDataServiceOpenAPISpec
currently calls axios.get without a timeout or any error handling, so add a
timeout to the request and wrap the fetch in try/catch. Use the existing
getDataServiceOpenAPISpec method and the swaggerResponse/swaggerUrl flow to
catch network or host resolution failures, then surface a clear error back to
the caller instead of letting the request hang or fail silently.
In
`@packages/mi-language-server/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/resourceFinder/AbstractResourceFinder.java`:
- Around line 647-674: The getDataServiceOperations flow in
AbstractResourceFinder only handles IOException, so unchecked failures from
Utils.getDOMDocument or SyntaxTreeGenerator.buildTree can escape and break
availableResources. Broaden the try/catch in getDataServiceOperations to handle
unexpected parsing/runtime exceptions as well, and on any failure keep the
existing graceful behavior by logging a warning and returning the empty response
instead of propagating. Use the existing symbols Utils.getDOMDocument,
SyntaxTreeGenerator.buildTree, and LOGGER to locate the change.
In `@packages/mi-visualizer/src/RuntimeServicesPanel.tsx`:
- Around line 221-223: The onTryDataService handler in RuntimeServicesPanel
currently awaits getDataServiceOpenAPISpec without handling rejections, which
can surface as an unhandled promise rejection with no user feedback. Update
onTryDataService to catch failures from
rpcClient.getMiDiagramRpcClient().getDataServiceOpenAPISpec({ name }) and handle
them gracefully, using the existing RuntimeServicesPanel/“Try it” flow to show a
failure state or error message when the runtime service is unavailable.
---
Nitpick comments:
In `@packages/mi-diagram/src/components/Form/Keylookup/Keylookup.tsx`:
- Around line 279-293: Short-circuit the lookup in Keylookup before calling
getAvailableResources when filterType is "dsOperation" and props.dataServiceName
is not set, so no unnecessary RPC request is made. Update the logic around the
resourceType selection and the
rpcClient.getMiDiagramRpcClient().getAvailableResources call to return early
with an empty result (or equivalent no-op state) in this case, while preserving
the existing behavior for bindToInbound, array filterType, and all other
resource lookups.
In `@packages/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts`:
- Around line 5933-5938: The proxy startup logic in the RPC manager creates a
fresh cors-anywhere server on every call, which can leak listeners across
repeated “Try it” actions. Extract the shared proxy setup used by getOpenAPISpec
and editOpenAPISpec into a reusable helper in rpc-manager.ts that lazily
initializes the proxy once and returns the same instance/port on subsequent
calls. Update this new call site to use that helper instead of directly calling
getPortPromise and cors_proxy.createServer.
In
`@packages/mi-language-server/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/resourceFinder/AbstractResourceFinder.java`:
- Around line 700-716: The Javadoc example in generateRequestName is
inconsistent with the actual naming logic and MI’s documented convention. Update
the example in AbstractResourceFinder.generateRequestName so it reflects the
real output from the current path/method transformation (no extra separator
between method and path), keeping the description aligned with the
implementation and the runtime naming rule.
In
`@packages/mi-language-server/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/resourceFinder/pojo/DataServiceOperationResource.java`:
- Around line 22-28: `DataServiceOperationResource` only initializes the name,
so its inherited `Resource.type` remains null and downstream UI in
`Keylookup.tsx` cannot render the correct badge/icon. Update the
`DataServiceOperationResource` constructor to also assign an appropriate type
value for data service operations, using the existing
`Resource`/`DataServiceOperationResource` symbols to locate the initialization
point.
In
`@packages/mi-language-server/org.eclipse.lemminx/src/main/resources/org/eclipse/lemminx/mediators/430/ui-schemas/dataServiceCall.json`:
- Around line 85-265: The DSSProperties nested table schema is duplicated across
multiple operation tables, so update the dataServiceCall.json schema to reuse a
shared sub-definition instead of repeating the same
propertyName/propertyValueType/propertyValue/propertyExpression block in
singleOperations, batchOperations, and requestBoxoperations. Extract that
repeated table into one reusable definition and reference it from each table’s
elements so future changes stay in sync; use the existing DSSProperties,
singleOperations, and batchOperations symbols to locate the repeated schema
sections.
🪄 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: 1281521f-7195-4c08-9c28-fa0376c2bccd
📒 Files selected for processing (20)
packages/mi-core/src/rpc-types/mi-diagram/index.tspackages/mi-core/src/rpc-types/mi-diagram/rpc-type.tspackages/mi-core/src/rpc-types/mi-diagram/types.tspackages/mi-diagram/src/components/Form/FormGenerator.tsxpackages/mi-diagram/src/components/Form/GigaParamManager/ParameterManager.tsxpackages/mi-diagram/src/components/Form/Keylookup/Keylookup.tsxpackages/mi-extension/src/lang-client/ExtendedLanguageClient.tspackages/mi-extension/src/rpc-managers/mi-diagram/rpc-handler.tspackages/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.tspackages/mi-language-server/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/SynapseLanguageService.javapackages/mi-language-server/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/mediatorService/mediators/DataServiceCallMediator.javapackages/mi-language-server/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/resourceFinder/AbstractResourceFinder.javapackages/mi-language-server/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/resourceFinder/pojo/DataServiceOperationResource.javapackages/mi-language-server/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/resourceFinder/pojo/ResourceParam.javapackages/mi-language-server/org.eclipse.lemminx/src/main/resources/org/eclipse/lemminx/mediators/430/templates/dataServiceCall.mustachepackages/mi-language-server/org.eclipse.lemminx/src/main/resources/org/eclipse/lemminx/mediators/430/ui-schemas/dataServiceCall.jsonpackages/mi-language-server/org.eclipse.lemminx/src/main/resources/org/eclipse/lemminx/mediators/440/templates/dataServiceCall.mustachepackages/mi-language-server/org.eclipse.lemminx/src/main/resources/org/eclipse/lemminx/mediators/440/ui-schemas/dataServiceCall.jsonpackages/mi-rpc-client/src/rpc-clients/mi-diagram/rpc-client.tspackages/mi-visualizer/src/RuntimeServicesPanel.tsx
ChinthakaJ98
force-pushed
the
dss-improvements
branch
from
July 7, 2026 05:02
8cad79b to
10662fb
Compare
arunans23
reviewed
Jul 7, 2026
arunans23
reviewed
Jul 7, 2026
arunans23
reviewed
Jul 7, 2026
Member
|
Shall we add screenshots to the PR description to show the rendering? |
ChinthakaJ98
removed
Checks/Run MI UI Tests
ChinthakaJ98
force-pushed
the
dss-improvements
branch
from
July 14, 2026 07:26
10662fb to
5528d13
Compare
arunans23
reviewed
Jul 14, 2026
arunans23
reviewed
Jul 14, 2026
ChinthakaJ98
force-pushed
the
dss-improvements
branch
from
July 15, 2026 07:23
5528d13 to
e7fdf9f
Compare
arunans23
approved these changes
Jul 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR will address the below mentioned issues.
Screen.Recording.2026-07-14.at.12.44.46.mov