-
Notifications
You must be signed in to change notification settings - Fork 268
Provide JDK21 compilation support #4555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughLarge-scale refactoring transitioning from SCR plugins to OSGi service declarations via Provide-Capability entries, updating Eclipse OSGi dependencies, removing unused servlet dependencies, migrating tests from PowerMock to Mockito, removing Declarative Services annotations, and increasing Java compilation target to version 21 across the codebase. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
| protected void activate(ComponentContext ctxt) { | ||
|
|
||
| bindTransactionManagerWithJNDI(); | ||
| log.debug("Transaction Manager bundle is activated "); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Log Improvement Suggestion No: 1
| protected void activate(ComponentContext ctxt) { | |
| bindTransactionManagerWithJNDI(); | |
| log.debug("Transaction Manager bundle is activated "); | |
| } | |
| protected void activate(ComponentContext ctxt) { | |
| bindTransactionManagerWithJNDI(); | |
| log.info("Transaction Manager component activated successfully"); | |
| log.debug("Transaction Manager bundle is activated "); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/org.wso2.micro.integrator.core/src/main/java/org/wso2/micro/integrator/transaction/manager/TransactionManagerComponent.java (1)
36-135: Critical: Component is completely non-functional—marked as incomplete work in code.The removal of Declarative Services annotations has left TransactionManagerComponent broken:
Missing all DS annotations: No
@Component,@Activate,@Reference, or@Deactivateannotations are present in the class (verified across all 136 lines).No activation pathway: The
activate()method (line 49) is never invoked, sobindTransactionManagerWithJNDI()is never called.Services never injected:
setTransactionManager()andsetUserTransaction()methods are never called without@Referenceannotations, leavingtxManageranduserTransactionstatic fields null.Dead code: The class retains all lifecycle and binding logic but has no way to execute it. No external code instantiates or calls this component.
Acknowledged as unfinished: The file header contains a TODO comment stating "need a way to solve javax.transaction service dependency"—confirming this is abandoned, incomplete refactoring.
The codebase has moved on to use
DBUtils.getContainerTransactionManager()instead, making TransactionManagerComponent entirely superseded and non-functional.pom.xml (1)
1758-1758: Remove unused PowerMock dependencies from pom.xml.PowerMock 2.0.7 is incompatible with Mockito 5.3.1 and JDK21, but verification shows PowerMock is not actually used anywhere in the codebase—no imports or test annotations found. The dependencies declared in pom.xml (lines 1364-1367 and 1370-1373) are unused and should be removed.
🧹 Nitpick comments (3)
components/org.wso2.micro.integrator.ndatasource.common/pom.xml (1)
47-48: Consider aligning with related modules by specifying explicit imports.The empty
<Import-Package>element disables automatic import calculation by the Maven Bundle Plugin. While<DynamicImport-Package>*</DynamicImport-Package>on line 49 provides runtime fallback, this skips build-time dependency validation. All related ndatasource modules (core,rdbms,capp.deployer) explicitly declare their imports while also using dynamic imports as fallback. Consider adding explicit imports for known dependencies to this common module for consistency, or document if the intentionally minimal approach is required.pom.xml (1)
1800-1802: Remove commented-out code.The commented-out
carbon.kernel.versionproperty appears to be leftover from development. Clean this up.🔎 Proposed fix
-<!-- <carbon.kernel.version>4.10.108-SNAPSHOT</carbon.kernel.version>--> - <carbon.kernel.version>4.9.34-SNAPSHOT</carbon.kernel.version> + <carbon.kernel.version>4.9.34-SNAPSHOT</carbon.kernel.version>features/mediation-features/data-publisher-features/org.wso2.micro.integrator.observability.feature/pom.xml (1)
98-102: Use a version property instead of hardcoded version.The
jakarta.annotation-apiversion is hardcoded as3.0.0. Define a property in the root POM for consistency and easier maintenance.🔎 Proposed fix
In root
pom.xml, add property:<jakarta.annotation-api.version>3.0.0</jakarta.annotation-api.version>Then update this dependency:
<dependency> <groupId>jakarta.annotation</groupId> <artifactId>jakarta.annotation-api</artifactId> - <version>3.0.0</version> + <version>${jakarta.annotation-api.version}</version> </dependency>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
components/data/data-services/org.wso2.micro.integrator.dataservices.capp.deployer/pom.xmlcomponents/data/data-services/org.wso2.micro.integrator.dataservices.common/pom.xmlcomponents/data/data-services/org.wso2.micro.integrator.dataservices.core/pom.xmlcomponents/data/data-services/org.wso2.micro.integrator.dataservices.core/src/main/java/org/wso2/micro/integrator/dataservices/core/odata/serializer/ODataImpl.javacomponents/data/data-services/org.wso2.micro.integrator.dataservices.core/src/test/java/org/wso2/micro/integrator/dataservices/core/test/util/DSComponentExtension.javacomponents/data/data-services/org.wso2.micro.integrator.dataservices.odata.endpoint/pom.xmlcomponents/data/data-services/org.wso2.micro.integrator.dataservices.sql.driver/pom.xmlcomponents/javax.cache/src/main/java/org/wso2/carbon/caching/impl/DataHolder.javacomponents/javax.cache/src/main/java/org/wso2/carbon/caching/impl/internal/CachingServiceComponent.javacomponents/mediation/data-publishers/org.wso2.micro.integrator.observability/pom.xmlcomponents/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint.osgi/pom.xmlcomponents/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint.persistence/pom.xmlcomponents/mediation/mediators/cache-mediator/org.wso2.carbon.mediator.cache/pom.xmlcomponents/mediation/tasks/org.wso2.micro.integrator.ntask.core/pom.xmlcomponents/org.wso2.micro.integrator.core/pom.xmlcomponents/org.wso2.micro.integrator.core/src/main/java/org/wso2/micro/integrator/transaction/manager/TransactionManagerComponent.javacomponents/org.wso2.micro.integrator.extensions/org.wso2.micro.integrator.management.apis/pom.xmlcomponents/org.wso2.micro.integrator.extensions/org.wso2.micro.integrator.management.apis/src/test/java/org/wso2/micro/integrator/management/apis/ManagementApiParserTest.javacomponents/org.wso2.micro.integrator.initializer/pom.xmlcomponents/org.wso2.micro.integrator.log4j2.plugins/pom.xmlcomponents/org.wso2.micro.integrator.ndatasource.capp.deployer/pom.xmlcomponents/org.wso2.micro.integrator.ndatasource.common/pom.xmlcomponents/org.wso2.micro.integrator.ndatasource.core/pom.xmlcomponents/org.wso2.micro.integrator.security/pom.xmlfeatures/mediation-features/data-publisher-features/org.wso2.micro.integrator.observability.feature/pom.xmlfeatures/org.wso2.micro.integrator.server.feature/pom.xmlintegration/pom.xmlp2-profile/carbon.productp2-profile/pom.xmlpom.xml
💤 Files with no reviewable changes (7)
- components/data/data-services/org.wso2.micro.integrator.dataservices.odata.endpoint/pom.xml
- components/org.wso2.micro.integrator.extensions/org.wso2.micro.integrator.management.apis/src/test/java/org/wso2/micro/integrator/management/apis/ManagementApiParserTest.java
- components/data/data-services/org.wso2.micro.integrator.dataservices.capp.deployer/pom.xml
- components/org.wso2.micro.integrator.log4j2.plugins/pom.xml
- components/data/data-services/org.wso2.micro.integrator.dataservices.core/pom.xml
- components/org.wso2.micro.integrator.ndatasource.capp.deployer/pom.xml
- components/data/data-services/org.wso2.micro.integrator.dataservices.core/src/test/java/org/wso2/micro/integrator/dataservices/core/test/util/DSComponentExtension.java
🔇 Additional comments (25)
components/data/data-services/org.wso2.micro.integrator.dataservices.core/src/main/java/org/wso2/micro/integrator/dataservices/core/odata/serializer/ODataImpl.java (1)
30-30: No syntax error to fix. Line 30 contains correct import syntax with a single semicolon. TheUriHelperimport is properly declared and used in the file (as a return type in thecreateUriHelper()method at line 68).Likely an incorrect or invalid review comment.
components/org.wso2.micro.integrator.extensions/org.wso2.micro.integrator.management.apis/pom.xml (2)
59-63: LGTM: Modern test dependency migration.Migrating from PowerMock to Mockito is a good practice. PowerMock has limited support for newer Java versions and Mockito provides better Java 21 compatibility.
84-84: Property is properly defined. Theosgi.service.cm.versionproperty is defined in the root pom.xml at line 1781 with value1.6.1. No action required.p2-profile/pom.xml (1)
40-40: LGTM: Standard extension configuration for P2 plugin.Adding
<extensions>true</extensions>enables the carbon-p2-plugin to properly participate in the Maven lifecycle as an extension, which is standard practice for P2 packaging plugins.p2-profile/carbon.product (1)
5-5: LGTM: Version alignment with development snapshot.The version updates from 4.9.30 to 4.9.34.SNAPSHOT correctly align the product descriptor with the current development cycle.
Also applies to: 17-17
components/javax.cache/src/main/java/org/wso2/carbon/caching/impl/DataHolder.java (1)
23-23: Import relocation verified as complete.The import update from
org.wso2.carbon.base.api.ServerConfigurationServicetoorg.wso2.micro.integrator.core.services.ServerConfigurationServiceis properly isolated with no remaining references to the old package in the codebase.components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint.osgi/pom.xml (1)
63-71: Dependency migration is consistent across the codebase.The migration from
org.eclipse.osgi:org.eclipse.osgi.servicestoorg.eclipse.platform:org.eclipse.osgi.serviceswith thejetty-servletexclusion is applied uniformly across all five dependencies in the codebase. No legacy references remain.components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint.persistence/pom.xml (1)
43-50: LGTM - Dependency migration to Eclipse Platform.The change from
org.eclipse.osgitoorg.eclipse.platformgroupId with thejetty-servletexclusion is consistent with the project-wide dependency consolidation for JDK21 compatibility. The exclusion prevents servlet API version conflicts.components/data/data-services/org.wso2.micro.integrator.dataservices.sql.driver/pom.xml (2)
84-91: LGTM - Dependency migration consistent with project pattern.The change from
org.eclipse.osgitoorg.eclipse.platform:org.eclipse.osgi.serviceswithjetty-servletexclusion is consistent across the codebase for JDK21 compatibility.
113-159: No action required. This module does not provide OSGi services—it only consumes services through@Referenceannotations. TheSQLDriverDSComponentclass lacks a@Componentannotation, so it is not registering any services with the OSGi framework.Provide-Capabilityis only necessary for modules that export/provide OSGi services; since this module provides none, the removal ofmaven-scr-plugindoes not create a discoverability issue.components/org.wso2.micro.integrator.ndatasource.core/pom.xml (2)
34-41: LGTM - Consistent dependency migration.The change to
org.eclipse.platform:org.eclipse.osgi.serviceswithjetty-servletexclusion follows the established pattern.
79-81: LGTM - DataSourceService capability declaration.The
Provide-Capabilityentry properly replaces the removed SCR plugin functionality, ensuring theDataSourceServiceremains discoverable in the OSGi runtime.components/org.wso2.micro.integrator.core/pom.xml (2)
151-156: LGTM - Core service capabilities declaration.The
Provide-Capabilityblock declares four essential core services:
Axis2ConfigurationContextServiceUserStoreTemporaryServiceCarbonServerConfigurationServiceServerConfigurationServiceThis properly exposes these services for OSGi capability-based dependency resolution, replacing SCR-based service registration.
59-61: Verify functional equivalence of javax.servlet as replacement for org.eclipse.equinox.http.helper.The dependency change from
org.eclipse.equinox.http.helpertoorg.eclipse.equinox:javax.servletinvolves a significant semantic shift:
http.helperprovides OSGi-specific HTTP integration with dynamic servlet/filter registration via HttpService and extension pointsjavax.servletprovides the standard servlet API only, without OSGi extensionsWhile
org.eclipse.equinox:javax.servletis compatible with JDK21 (Equinox bundles declared for JavaSE-17 run on newer JDKs), ensure this change does not break code that depends on OSGi HttpService registration or Equinox HTTP helpers.components/org.wso2.micro.integrator.initializer/pom.xml (1)
145-149: LGTM - Synapse service capabilities declaration.The
Provide-Capabilityblock correctly declares the three Synapse-related services. Notably,SynapseEnvironmentServiceis also referenced in theCAPP_MANAGER-RequiredServicesheader (line 134), showing good consistency between the capability declarations and service requirements.components/mediation/tasks/org.wso2.micro.integrator.ntask.core/pom.xml (1)
76-78: OSGi service capability declaration is correct.The
Provide-Capabilityentry properly declares theTaskServiceas an OSGi service capability. The service interface exists at the declared location, and the syntax follows OSGi R5+ specification.components/org.wso2.micro.integrator.security/pom.xml (1)
164-166: The Provide-Capability manifest entry is correctly declared, but review characterization is inaccurate.The
Provide-Capabilityin pom.xml correctly describes the OSGi service exposed by this bundle. However, the service is not registered via a "SCR-based approach" replacement. The actual registration occurs imperatively inActivator.javathroughbundleContext.registerService(), maintaining the traditional BundleActivator pattern rather than Declarative Services.The manifest declaration accurately reflects what Activator.java registers (RealmService and UserRealmService), and the service interface exists with a proper DefaultRealmService implementation. No issues with the manifest entry itself, but clarify that the registration mechanism remains imperative, not declarative.
Likely an incorrect or invalid review comment.
features/org.wso2.micro.integrator.server.feature/pom.xml (1)
40-99: Verify necessity of all OSGi service bundles.A broad set of OSGi service dependencies is added (upnp, device, wireadmin, provisioning, useradmin, etc.). Some of these appear to be specialized services not typically required for a Micro Integrator. Confirm these are actually needed for JDK21 compatibility or the Felix SCR runtime, as unnecessary bundles increase footprint and startup time.
components/mediation/data-publishers/org.wso2.micro.integrator.observability/pom.xml (1)
103-103: LGTM - Contract suppression for embedded packages.The
<_contract>!*</_contract>directive correctly disables OSGi contract requirements. This is appropriate since the bundle embeds Prometheus packages privately (lines 108-114), avoiding contract conflicts at runtime.pom.xml (3)
2272-2273: LGTM - Java 21 compilation target.The source and target are correctly set to 21 for JDK21 support.
1798-1809: Multiple SNAPSHOT dependencies may cause build instability.Several core dependencies use SNAPSHOT versions:
carbon.p2.plugin.version: 5.4.12-SNAPSHOTcarbon.kernel.version: 4.9.34-SNAPSHOTsynapse.version: 4.0.0-wso2v300-SNAPSHOTcarbon.mediation.version: 4.7.269-SNAPSHOTSNAPSHOT dependencies can change unexpectedly, leading to non-reproducible builds. Ensure these are stabilized before release.
56-68: Verify Jetty exclusions don't break HTTP functionality.The Jetty servlet exclusions from
org.eclipse.osgi.servicesmay impact HTTP-related functionality if any OSGi HTTP services depend on Jetty. Ensure integration tests cover HTTP endpoints.features/mediation-features/data-publisher-features/org.wso2.micro.integrator.observability.feature/pom.xml (2)
125-134: Clarify intent of commented-out bundleDefs.All Prometheus and Jakarta annotation bundleDefs are commented out. If these bundles are embedded privately within the observability bundle (via
Private-Packagein the component POM), this is correct. However, if they should be separate OSGi bundles, this needs to be uncommented.Please confirm this is intentional based on the embedding strategy in
org.wso2.micro.integrator.observability/pom.xml.
45-61: LGTM - Supporting dependencies for observability feature.The added dependencies provide necessary runtime support for the observability feature.
components/javax.cache/src/main/java/org/wso2/carbon/caching/impl/internal/CachingServiceComponent.java (1)
35-35: Verify interface compatibility and OSGi service availability.The import has been relocated from
org.wso2.carbon.base.apitoorg.wso2.micro.integrator.core.services. Since this service hasMANDATORYcardinality (line 69), ensure that:
- The
ServerConfigurationServiceinterface is properly defined and exported from the core module (note: onlyCarbonServerConfigurationServiceimplementation is visible in source code)- The service is correctly registered at runtime as
org.wso2.micro.integrator.core.services.ServerConfigurationServiceand available for injection- The module's POM already depends on the new core module (verified), but confirm the OSGi service wiring is complete
| <Import-Package> | ||
| </Import-Package> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty Import-Package may cause missing dependencies.
The empty <Import-Package> block prevents automatic import calculation by the Maven Bundle Plugin. Unlike components/org.wso2.micro.integrator.ndatasource.common/pom.xml, this POM lacks a <DynamicImport-Package> fallback, which may lead to ClassNotFoundException at runtime.
Consider one of the following:
- Remove the empty Import-Package block to enable automatic calculation
- Add explicit package imports
- Add
<DynamicImport-Package>*</DynamicImport-Package>as a fallback (though this reduces build-time validation)
#!/bin/bash
# Description: Verify if the exported packages have external dependencies
# Find Java files in the source directory and check their imports
fd -e java . components/data/data-services/org.wso2.micro.integrator.dataservices.common/src/main/java/ \
--exec grep -h '^import ' {} \; | \
sort -u | \
grep -v 'org.wso2.micro.integrator.dataservices.common'🤖 Prompt for AI Agents
In
components/data/data-services/org.wso2.micro.integrator.dataservices.common/pom.xml
around lines 47-48 there is an empty <Import-Package> block which prevents the
Maven Bundle Plugin from calculating imports and may cause missing dependencies
at runtime; remove the empty <Import-Package> block to let imports be
auto-calculated, or alternatively replace it with explicit package import
entries listing required packages, or add a fallback
<DynamicImport-Package>*</DynamicImport-Package> if you need a broad runtime
import allowance—pick one of these fixes and update the POM accordingly.
| <Import-Package> | ||
| </Import-Package> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if other fragment bundles in the project use similar patterns
rg -A3 "Fragment-Host" --type xml -g "pom.xml" | rg -B1 -A2 "Import-Package"Repository: wso2/product-micro-integrator
Length of output: 633
🏁 Script executed:
#!/bin/bash
# First, view the cache-mediator pom.xml around lines 104-105
echo "=== Cache-mediator pom.xml around lines 104-105 ==="
sed -n '100,110p' components/mediation/mediators/cache-mediator/org.wso2.carbon.mediator.cache/pom.xml
# Find all fragment bundles and their Import-Package patterns
echo -e "\n=== All fragment bundles with their instructions sections ==="
rg -B5 -A15 "Fragment-Host" --type xml -g "pom.xml" | grep -A15 "Fragment-Host"Repository: wso2/product-micro-integrator
Length of output: 14876
Remove the explicitly empty <Import-Package> element or populate it with required packages.
The empty <Import-Package> element is inconsistent with other fragment bundles in the project. Most fragment bundles either omit Import-Package entirely (allowing bnd to auto-generate imports based on bytecode analysis) or explicitly list required packages. Combined with DynamicImport-Package:*, this configuration defers all package resolution to runtime, masking missing dependencies that would normally be caught at build time. Either remove the empty element to allow auto-generation, or explicitly list the packages this fragment actually imports.
🤖 Prompt for AI Agents
In
components/mediation/mediators/cache-mediator/org.wso2.carbon.mediator.cache/pom.xml
around lines 104-105, an explicitly empty <Import-Package> element is present;
remove this empty element (or alternatively replace it with an explicit
comma-separated list of the packages this fragment requires) so bnd can
auto-generate imports or so build-time package resolution is explicit; prefer
removing the empty element to match other fragments unless you know and can list
exact imports required.
|
|
||
| @Component(name = "org.wso2.micro.integrator.transaction.manager.TransactionManagerComponent", | ||
| immediate = true) | ||
| //TODO: need a way to solve javax.transaction service dependency |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: TODO indicates incomplete refactoring.
The TODO comment acknowledges an unresolved dependency issue for javax.transaction services. This placeholder suggests the refactoring is incomplete and the transaction manager wiring needs to be addressed before merging.
| <dependency> | ||
| <groupId>org.osgi</groupId> | ||
| <artifactId>org.osgi.service.cm</artifactId> | ||
| </dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate dependency declaration.
org.osgi:org.osgi.service.cm is already declared at lines 52-55. Remove this duplicate to avoid confusion.
🔎 Proposed fix
- <dependency>
- <groupId>org.osgi</groupId>
- <artifactId>org.osgi.service.cm</artifactId>
- </dependency>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <dependency> | |
| <groupId>org.osgi</groupId> | |
| <artifactId>org.osgi.service.cm</artifactId> | |
| </dependency> |
🤖 Prompt for AI Agents
In features/org.wso2.micro.integrator.server.feature/pom.xml around lines 92 to
95, there's a duplicate dependency declaration for org.osgi:org.osgi.service.cm
already declared at lines 52-55; remove the redundant <dependency> block at
lines 92-95 so the artifact is declared only once, then run mvn validate (or
build) to confirm no other references require duplication.
| <bundleDef>org.osgi:org.osgi.service.cm:${osgi.service.cm.version}</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.service.coordinator:${osgi.service.coordinator.version}</bundleDef> | ||
| <bundleDef>org.apache.felix:org.apache.felix.scr:${felix.scr.version}</bundleDef> | ||
| <bundleDef>org.eclipse.platform:org.eclipse.osgi:${version.equinox.platform.osgi}</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.service.component</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.util.promise</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.util.function</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.service.cm</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.service.upnp</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.service.device</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.service.event</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.service.wireadmin</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.service.provisioning</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.service.useradmin</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.service.metatype</bundleDef> | ||
| <bundleDef>org.osgi:org.osgi.service.permissionadmin</bundleDef> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate bundleDef and inconsistent version specifications.
org.osgi:org.osgi.service.cmis declared twice (lines 350 and 357)- Lines 350-353 specify versions via properties, but lines 354-365 omit versions, relying on dependency management. This inconsistency may cause resolution issues.
🔎 Proposed fix - remove duplicate and add consistent versions
<bundleDef>org.osgi:org.osgi.service.cm:${osgi.service.cm.version}</bundleDef>
<bundleDef>org.osgi:org.osgi.service.coordinator:${osgi.service.coordinator.version}</bundleDef>
<bundleDef>org.apache.felix:org.apache.felix.scr:${felix.scr.version}</bundleDef>
<bundleDef>org.eclipse.platform:org.eclipse.osgi:${version.equinox.platform.osgi}</bundleDef>
-<bundleDef>org.osgi:org.osgi.service.component</bundleDef>
-<bundleDef>org.osgi:org.osgi.util.promise</bundleDef>
-<bundleDef>org.osgi:org.osgi.util.function</bundleDef>
-<bundleDef>org.osgi:org.osgi.service.cm</bundleDef>
-<bundleDef>org.osgi:org.osgi.service.upnp</bundleDef>
-<bundleDef>org.osgi:org.osgi.service.device</bundleDef>
-<bundleDef>org.osgi:org.osgi.service.event</bundleDef>
-<bundleDef>org.osgi:org.osgi.service.wireadmin</bundleDef>
-<bundleDef>org.osgi:org.osgi.service.provisioning</bundleDef>
-<bundleDef>org.osgi:org.osgi.service.useradmin</bundleDef>
-<bundleDef>org.osgi:org.osgi.service.metatype</bundleDef>
-<bundleDef>org.osgi:org.osgi.service.permissionadmin</bundleDef>
+<bundleDef>org.osgi:org.osgi.service.component:${osgi.service.component.version}</bundleDef>
+<bundleDef>org.osgi:org.osgi.util.promise:${osgi.util.promise.version}</bundleDef>
+<bundleDef>org.osgi:org.osgi.util.function:${osgi.util.function.version}</bundleDef>
+<bundleDef>org.osgi:org.osgi.service.upnp:${osgi.service.upnp.version}</bundleDef>
+<bundleDef>org.osgi:org.osgi.service.device:${osgi.service.device.version}</bundleDef>
+<bundleDef>org.osgi:org.osgi.service.event:${osgi.service.event.version}</bundleDef>
+<bundleDef>org.osgi:org.osgi.service.wireadmin:${osgi.service.wireadmin.version}</bundleDef>
+<bundleDef>org.osgi:org.osgi.service.provisioning:${osgi.service.provisioning.version}</bundleDef>
+<bundleDef>org.osgi:org.osgi.service.useradmin:${osgi.service.useradmin.version}</bundleDef>
+<bundleDef>org.osgi:org.osgi.service.metatype:${osgi.service.metatype.version}</bundleDef>
+<bundleDef>org.osgi:org.osgi.service.permissionadmin:${osgi.service.permissionadmin.version}</bundleDef>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <bundleDef>org.osgi:org.osgi.service.cm:${osgi.service.cm.version}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.coordinator:${osgi.service.coordinator.version}</bundleDef> | |
| <bundleDef>org.apache.felix:org.apache.felix.scr:${felix.scr.version}</bundleDef> | |
| <bundleDef>org.eclipse.platform:org.eclipse.osgi:${version.equinox.platform.osgi}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.component</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.util.promise</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.util.function</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.cm</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.upnp</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.device</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.event</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.wireadmin</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.provisioning</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.useradmin</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.metatype</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.permissionadmin</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.cm:${osgi.service.cm.version}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.coordinator:${osgi.service.coordinator.version}</bundleDef> | |
| <bundleDef>org.apache.felix:org.apache.felix.scr:${felix.scr.version}</bundleDef> | |
| <bundleDef>org.eclipse.platform:org.eclipse.osgi:${version.equinox.platform.osgi}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.component:${osgi.service.component.version}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.util.promise:${osgi.util.promise.version}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.util.function:${osgi.util.function.version}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.upnp:${osgi.service.upnp.version}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.device:${osgi.service.device.version}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.event:${osgi.service.event.version}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.wireadmin:${osgi.service.wireadmin.version}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.provisioning:${osgi.service.provisioning.version}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.useradmin:${osgi.service.useradmin.version}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.metatype:${osgi.service.metatype.version}</bundleDef> | |
| <bundleDef>org.osgi:org.osgi.service.permissionadmin:${osgi.service.permissionadmin.version}</bundleDef> |
🤖 Prompt for AI Agents
In features/org.wso2.micro.integrator.server.feature/pom.xml around lines 350 to
365, there's a duplicate bundleDef for org.osgi:org.osgi.service.cm and several
bundleDef entries omit explicit versions while nearby entries use property-based
versions; remove the duplicate org.osgi:org.osgi.service.cm entry (the later
one) and make all bundleDef lines consistent by adding the appropriate version
properties (or the same version strings) used elsewhere (e.g., use the existing
${osgi.*.version} properties) so every bundleDef either consistently references
a version property or intentionally relies on dependencyManagement.
| <jersey-client.version>1.8</jersey-client.version> | ||
| <awaitility.version>3.1.2</awaitility.version> | ||
| <jacoco.agent.version>0.8.8</jacoco.agent.version> | ||
| <jacoco.agent.version>0.8.12</jacoco.agent.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
JaCoCo 0.8.12 Java 21 support compatibility
💡 Result:
Short answer: JaCoCo 0.8.12 is nominally compatible with Java 21 (official Java‑21 support was added in 0.8.11 and 0.8.12 continues that support), but there have been user‑reported issues (analysis exceptions and some JVM crashes) when running with certain JDK21 builds. [1][2][3][4]
References:
- JaCoCo changelog / releases: official Java 21 support added in 0.8.11; 0.8.12 release notes. [1][2]
- GitHub issue reporting JVM SIGSEGV with JDK 21 + JaCoCo 0.8.12. [3]
- JaCoCo user group reports of "unsupported class file major version" / analysis errors with JDK21 and 0.8.12. [4]
If you plan to run JaCoCo with Java 21: use 0.8.11+ (0.8.12), but test on your exact JDK build and watch for the linked issues; if you hit problems, try different JDK21 builds or follow the GitHub thread for workarounds/fixes.
🏁 Script executed:
# Check the pom.xml file to verify the context and previous version
cat integration/pom.xml | grep -A 5 -B 5 "jacoco.agent.version"Repository: wso2/product-micro-integrator
Length of output: 202
JaCoCo 0.8.12 supports Java 21 bytecode instrumentation but requires testing on your specific JDK build.
The version bump to 0.8.12 aligns with the JDK21 compilation support objective. Official Java 21 support was added in version 0.8.11 and is continued in 0.8.12. However, there are user-reported issues including analysis exceptions and JVM crashes (SIGSEGV) with certain JDK21 builds. Verify compatibility by testing this version against your exact JDK21 distribution and monitor for known issues before full deployment.
🤖 Prompt for AI Agents
In integration/pom.xml around line 829 the jacoco.agent.version property was
bumped to 0.8.12; validate this change by running the full test suite (unit,
integration, and any instrumentation tests) on the exact JDK21 distribution(s)
you will use in CI/prod to check for analysis exceptions or JVM crashes, add a
JDK21 CI job that exercises JaCoCo instrumentation, and if failures appear
either pin to the last-known-good version (e.g., 0.8.11) behind a Maven profile
or add a conditional profile/fallback and document the required JDK build(s) for
safe deployment.
Purpose
$subject
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.