Skip to content

Conversation

@chamilaadhi
Copy link

JDK21 compilation changes

chamilaadhi and others added 25 commits December 18, 2025 13:31
This reverts commit 64e8f25.
Port servlet context related fixes
Port JDK 21 related refactoring improvements
Register CSRFGuard listeners and filter
@coderabbitai
Copy link

coderabbitai bot commented Feb 4, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

  • 🔍 Trigger a full review
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 4.9.x-JDK-21

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ chamilaadhi
❌ tharikaGitHub
You have signed the CLA already but the status is still pending? Let us recheck it.

Comment on lines 206 to 208
@Override
public boolean isSupported(OptionalFeature optionalFeature) {
Util.checkAccess(ownerTenantDomain, ownerTenantId);

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

Suggested change
@Override
public boolean isSupported(OptionalFeature optionalFeature) {
Util.checkAccess(ownerTenantDomain, ownerTenantId);
@Override
public boolean isSupported(OptionalFeature optionalFeature) {
log.debug("Checking if optional feature {} is supported", optionalFeature);
Util.checkAccess(ownerTenantDomain, ownerTenantId);

Comment on lines 56 to 57
//register carbon server confg as an OSGi service
registration = bundleContext.registerService(ServerConfigurationService.class.getName(), carbonServerConfiguration, null);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 3

Suggested change
//register carbon server confg as an OSGi service
registration = bundleContext.registerService(ServerConfigurationService.class.getName(), carbonServerConfiguration, null);
//register carbon server confg as an OSGi service
registration = bundleContext.registerService(ServerConfigurationService.class.getName(), carbonServerConfiguration, null);
if (log.isDebugEnabled()) {
log.debug("Registered ServerConfigurationService as OSGi service");
}

Comment on lines +29 to 31
if (frameworkLauncher == null) {
frameworkLauncher = new EquinoxFrameworkLauncher();
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 4

Suggested change
if (frameworkLauncher == null) {
frameworkLauncher = new EquinoxFrameworkLauncher();
}
if (frameworkLauncher == null) {
log.info("Creating new EquinoxFrameworkLauncher instance");
frameworkLauncher = new EquinoxFrameworkLauncher();
}

Comment on lines 26 to +27
public void run() {
SecurityManager secMan = System.getSecurityManager();
if (secMan != null) {
secMan.checkPermission(new ManagementPermission("control"));
}

// Carbon is running within an AppServer
FrameworkLauncher frameworkLauncher = FrameworkLauncherFactory.getFrameworkLauncher();
frameworkLauncher.stop(); //TODO FIXME There is an Equinox memory leak which causes ChildFirstURLClassloader to remain alive
System.setProperty(FrameworkLauncher.START_TIME, String.valueOf(System.currentTimeMillis()));
frameworkLauncher.deploy();
frameworkLauncher.start();

// Carbon is running within an AppServer

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 5

Suggested change
public void run() {
SecurityManager secMan = System.getSecurityManager();
if (secMan != null) {
secMan.checkPermission(new ManagementPermission("control"));
}
// Carbon is running within an AppServer
FrameworkLauncher frameworkLauncher = FrameworkLauncherFactory.getFrameworkLauncher();
frameworkLauncher.stop(); //TODO FIXME There is an Equinox memory leak which causes ChildFirstURLClassloader to remain alive
System.setProperty(FrameworkLauncher.START_TIME, String.valueOf(System.currentTimeMillis()));
frameworkLauncher.deploy();
frameworkLauncher.start();
// Carbon is running within an AppServer
public void run() {
// Carbon is running within an AppServer
log.info("Initiating framework restart");

Comment on lines +29 to +32
frameworkLauncher.stop(); //TODO FIXME There is an Equinox memory leak which causes ChildFirstURLClassloader to remain alive
System.setProperty(FrameworkLauncher.START_TIME, String.valueOf(System.currentTimeMillis()));
frameworkLauncher.deploy();
frameworkLauncher.start();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 6

Suggested change
frameworkLauncher.stop(); //TODO FIXME There is an Equinox memory leak which causes ChildFirstURLClassloader to remain alive
System.setProperty(FrameworkLauncher.START_TIME, String.valueOf(System.currentTimeMillis()));
frameworkLauncher.deploy();
frameworkLauncher.start();
frameworkLauncher.stop(); //TODO FIXME There is an Equinox memory leak which causes ChildFirstURLClassloader to remain alive
log.debug("Framework stopped successfully");
System.setProperty(FrameworkLauncher.START_TIME, String.valueOf(System.currentTimeMillis()));
frameworkLauncher.deploy();
log.debug("Framework deployed");
frameworkLauncher.start();
log.info("Framework restart completed successfully");

Comment on lines +541 to +542
private void registerCarbonServlet() throws InvalidSyntaxException {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 7

Suggested change
private void registerCarbonServlet() throws InvalidSyntaxException {
private void registerCarbonServlet() throws InvalidSyntaxException {
log.info("Registering CarbonServlet for service path");

carbonServletProperties.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_PATTERN, servicePath + "/*");
carbonServletProperties.put("osgi.http.whiteboard.context.select", "(osgi.http.whiteboard.context.name=carbonContext)");
ServiceRegistration<Servlet> servletServiceRegistration =
bundleContext.registerService(Servlet.class, carbonServlet, carbonServletProperties);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 8

Suggested change
bundleContext.registerService(Servlet.class, carbonServlet, carbonServletProperties);
CarbonCoreDataHolder.getInstance().addServiceRegistration(servletServiceRegistration);
log.info("Successfully registered CarbonServlet at path: " + servicePath + "/*");

@@ -37,11 +37,7 @@ public class CarbonCoreActivator implements BundleActivator {
private CarbonCoreDataHolder dataHolder = CarbonCoreDataHolder.getInstance();

public void start(BundleContext context) throws Exception {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 9

Suggested change
public void start(BundleContext context) throws Exception {
public void start(BundleContext context) throws Exception {
log.info("Initializing WSO2 Carbon Core bundle");

Comment on lines +176 to +179
public void addServiceRegistration(ServiceRegistration<?> serviceRegistration) {

serviceRegistrations.add(serviceRegistration);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 11

Suggested change
public void addServiceRegistration(ServiceRegistration<?> serviceRegistration) {
serviceRegistrations.add(serviceRegistration);
}
public void addServiceRegistration(ServiceRegistration<?> serviceRegistration) {
log.debug("Adding service registration to CarbonCoreDataHolder");
serviceRegistrations.add(serviceRegistration);
}

Comment on lines +181 to +186
public void unregisterServiceRegistrations() {

for (ServiceRegistration<?> serviceRegistration : serviceRegistrations) {
serviceRegistration.unregister();
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 12

Suggested change
public void unregisterServiceRegistrations() {
for (ServiceRegistration<?> serviceRegistration : serviceRegistrations) {
serviceRegistration.unregister();
}
}
public void unregisterServiceRegistrations() {
log.info("Unregistering " + serviceRegistrations.size() + " service registrations from CarbonCoreDataHolder");
for (ServiceRegistration<?> serviceRegistration : serviceRegistrations) {
serviceRegistration.unregister();
}
}

Comment on lines +89 to 90
if (isMeteringEnabled) {
requestDataPersister = new RequestDataPersisterTask();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 13

Suggested change
if (isMeteringEnabled) {
requestDataPersister = new RequestDataPersisterTask();
if (isMeteringEnabled) {
log.info("Metering is enabled. Starting request data persister.");
requestDataPersister = new RequestDataPersisterTask();

Comment on lines 34 to 36
public class DataSourceService {

public List<CarbonDataSource> getAllDataSources() throws DataSourceException {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 14

Suggested change
public class DataSourceService {
public List<CarbonDataSource> getAllDataSources() throws DataSourceException {
public class DataSourceService {
private static final Log log = LogFactory.getLog(DataSourceService.class);
public List<CarbonDataSource> getAllDataSources() throws DataSourceException {

Copy link

@wso2-engineering wso2-engineering bot left a 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

⚠️ Warning: AI-Generated Review Comments

  • 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: 3
#### Log Improvement Suggestion No: 4
#### Log Improvement Suggestion No: 5
#### Log Improvement Suggestion No: 6
#### Log Improvement Suggestion No: 7
#### Log Improvement Suggestion No: 8
#### Log Improvement Suggestion No: 9
#### Log Improvement Suggestion No: 11
#### Log Improvement Suggestion No: 12
#### Log Improvement Suggestion No: 13
#### Log Improvement Suggestion No: 14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants