Skip connector download if already provided by an integration project dependency#521
Merged
chathuranga-jayanath-99 merged 4 commits intowso2:mi-ext-alpha-releasefrom Apr 9, 2026
Conversation
…on project dependency
…oject dependency If the connector already exists in a dependent integration project, adding it again is skipped and an appropriate message is shown.
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
| Comment | Accepted (Y/N) | Reason |
|---|---|---|
| #### Log Improvement Suggestion No: 1 | ||
| #### Log Improvement Suggestion No: 2 | ||
| #### Log Improvement Suggestion No: 5 | ||
| #### Log Improvement Suggestion No: 6 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates dependency download flows to skip connector downloads when the connector is already available via an integration project dependency, and surfaces that outcome back to the caller via structured result objects.
Changes:
- Introduces
ConnectorDependencyDownloadResultand updates connector download APIs to return categorized results (failed vs skipped-from-integration-deps). - Renames/aligns integration-project dependency download result type to
IntegrationProjectDependencyDownloadResultand updates call sites/tests accordingly. - Updates
DependencyDownloadManagerto build aggregated user-facing messages from the new result objects.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/synapse/parser/IntegrationProjectDownloadManagerTest.java | Updates tests to use IntegrationProjectDependencyDownloadResult. |
| org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/synapse/connector/downloader/ConnectorDownloadManagerTest.java | Adds/updates tests for connector skip behavior and new result type. |
| org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/IntegrationProjectDownloadManager.java | Changes return type to IntegrationProjectDependencyDownloadResult. |
| org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/IntegrationProjectDependencyDownloadResult.java | Renames the public class to match the file and clarifies semantics. |
| org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/DependencyDownloadManager.java | Aggregates connector/integration-project results into user-facing status messages. |
| org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/ConnectorDownloadManager.java | Implements skip logic based on connectors loaded from integration project dependencies. |
| org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/ConnectorDependencyDownloadResult.java | New result type for connector dependency download outcomes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chathuranga-jayanath-99
force-pushed
the
connector-dependency-dev
branch
from
daae05b to
2eea2ce
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/ConnectorDependencyDownloadResult.java (1)
27-44: Consider making this class immutable for safer usage.The fields are assigned once in the constructor but are not
final, and the getters return the internal list references directly. If callers modify the returned lists, the object's state changes unexpectedly.♻️ Suggested immutable implementation
public class ConnectorDependencyDownloadResult { - private List<String> failedDependencies; - private List<String> fromIntegrationProjectDependencies; + private final List<String> failedDependencies; + private final List<String> fromIntegrationProjectDependencies; public ConnectorDependencyDownloadResult(List<String> failedDependencies, List<String> fromIntegrationProjectDependencies) { - this.failedDependencies = failedDependencies; - this.fromIntegrationProjectDependencies = fromIntegrationProjectDependencies; + this.failedDependencies = List.copyOf(failedDependencies); + this.fromIntegrationProjectDependencies = List.copyOf(fromIntegrationProjectDependencies); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/ConnectorDependencyDownloadResult.java` around lines 27 - 44, ConnectorDependencyDownloadResult exposes mutable internal lists via getFailedDependencies and getFromIntegrationProjectDependencies and its fields failedDependencies and fromIntegrationProjectDependencies are not final; make the class immutable by declaring those fields final, defensively copying the incoming lists in the constructor (e.g., new ArrayList<>(...)) and storing those copies, and return either unmodifiable views or fresh copies from the getters (e.g., Collections.unmodifiableList(storedList)) so callers cannot mutate internal state; also consider null-checking the constructor parameters in ConnectorDependencyDownloadResult to avoid NPEs.org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/IntegrationProjectDependencyDownloadResult.java (1)
27-51: Consider making this class immutable for consistency withConnectorDependencyDownloadResult.Same observation as the other result class: fields could be
finaland lists could use defensive copies to prevent unintended mutation.♻️ Suggested immutable implementation
public class IntegrationProjectDependencyDownloadResult { - private List<String> failedDependencies; - private List<String> noDescriptorDependencies; - private List<String> versioningTypeMismatchDependencies; + private final List<String> failedDependencies; + private final List<String> noDescriptorDependencies; + private final List<String> versioningTypeMismatchDependencies; public IntegrationProjectDependencyDownloadResult(List<String> failedDependencies, List<String> noDescriptorDependencies, List<String> versioningTypeMismatchDependencies) { - this.failedDependencies = failedDependencies; - this.noDescriptorDependencies = noDescriptorDependencies; - this.versioningTypeMismatchDependencies = versioningTypeMismatchDependencies; + this.failedDependencies = List.copyOf(failedDependencies); + this.noDescriptorDependencies = List.copyOf(noDescriptorDependencies); + this.versioningTypeMismatchDependencies = List.copyOf(versioningTypeMismatchDependencies); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/IntegrationProjectDependencyDownloadResult.java` around lines 27 - 51, Make IntegrationProjectDependencyDownloadResult immutable: mark the three fields failedDependencies, noDescriptorDependencies, and versioningTypeMismatchDependencies as final, copy the incoming Lists inside the constructor to defensive (unmodifiable) copies, and have the getter methods (getFailedDependencies, getNoDescriptorDependencies, getVersioningTypeMismatchDependencies) return those unmodifiable copies; also consider null-checking constructor parameters to avoid NPEs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/synapse/connector/downloader/ConnectorDownloadManagerTest.java`:
- Around line 93-94: The assertion message is misleading: the test checks
result.getFromIntegrationProjectDependencies() for presence of
"mi-connector-http" but the message says "should be marked as failed"; update
the assertion message to reflect the actual expectation (e.g. "Connector from
integration project dependency should be skipped / provided by integration
project dependency") so it matches the check on
getFromIntegrationProjectDependencies() for the variable result in
ConnectorDownloadManagerTest.
---
Nitpick comments:
In
`@org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/ConnectorDependencyDownloadResult.java`:
- Around line 27-44: ConnectorDependencyDownloadResult exposes mutable internal
lists via getFailedDependencies and getFromIntegrationProjectDependencies and
its fields failedDependencies and fromIntegrationProjectDependencies are not
final; make the class immutable by declaring those fields final, defensively
copying the incoming lists in the constructor (e.g., new ArrayList<>(...)) and
storing those copies, and return either unmodifiable views or fresh copies from
the getters (e.g., Collections.unmodifiableList(storedList)) so callers cannot
mutate internal state; also consider null-checking the constructor parameters in
ConnectorDependencyDownloadResult to avoid NPEs.
In
`@org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/IntegrationProjectDependencyDownloadResult.java`:
- Around line 27-51: Make IntegrationProjectDependencyDownloadResult immutable:
mark the three fields failedDependencies, noDescriptorDependencies, and
versioningTypeMismatchDependencies as final, copy the incoming Lists inside the
constructor to defensive (unmodifiable) copies, and have the getter methods
(getFailedDependencies, getNoDescriptorDependencies,
getVersioningTypeMismatchDependencies) return those unmodifiable copies; also
consider null-checking constructor parameters to avoid NPEs.
🪄 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: 0761194b-4cbd-4f70-908d-ed8e51631927
📒 Files selected for processing (7)
org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/ConnectorDependencyDownloadResult.javaorg.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/ConnectorDownloadManager.javaorg.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/DependencyDownloadManager.javaorg.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/IntegrationProjectDependencyDownloadResult.javaorg.eclipse.lemminx/src/main/java/org/eclipse/lemminx/customservice/synapse/parser/IntegrationProjectDownloadManager.javaorg.eclipse.lemminx/src/test/java/org/eclipse/lemminx/synapse/connector/downloader/ConnectorDownloadManagerTest.javaorg.eclipse.lemminx/src/test/java/org/eclipse/lemminx/synapse/parser/IntegrationProjectDownloadManagerTest.java
ChinthakaJ98
approved these changes
Apr 9, 2026
chathuranga-jayanath-99
merged commit 2f6f29d
into
wso2:mi-ext-alpha-release
6 of 10 checks passed
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
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.
Purpose
When adding a new connector, if the connector is already included as part of an integration project dependency, the download is skipped and an appropriate message is shown to the user.
Fixes: wso2/mi-vscode#1442
Summary by CodeRabbit
Release Notes
New Features