Skip to content

Conversation

@sntiwari1
Copy link

@sntiwari1 sntiwari1 commented Oct 13, 2025

This PR upgrades the sunbird-dial-service repository from Play Framework 2.4.6 + Akka + Scala 2.11.12 to Play Framework 2.8.22 + Apache Pekko 1.0.3 + Scala 2.13.12.

The upgrade ensures license compliance, security, and modernization, while maintaining API compatibility and preserving existing functionality.

Issue Fixed: N/A (general upgrade and maintenance)

Motivation and Context:

  • License Compliance: Akka switched to Business Source License 1.1; Pekko remains Apache 2.0
  • Security: Old Play and Akka versions no longer receive updates
  • Modernization: Access to Play 2.8 features, Scala 2.13 improvements, and performance optimizations

Dependencies:

  • Play Framework 2.8.22
  • Apache Pekko 1.0.3
  • Scala 2.13.12
  • Jackson 2.11.4
  • Guice 5.1.0
  • SLF4J 2.0.9
  • Logback 1.4.14
  • Netty 4.1.93.Final
  • Commons Lang3 3.12.0

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature / Upgrade (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Built all modules: mvn clean install -Dmaven.test.skip=true
  • Built with test compilation: mvn clean install -DskipTests
  • Packaged distribution: mvn play2:dist
  • Development server runs successfully: mvn play2:run

Test Configuration:

  • Software versions: Java 11/17, Play 2.8.22, Scala 2.13.12, Pekko 1.0.3
  • Hardware versions: 2 CPU / 4GB RAM

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my upgrade works
  • New and existing unit tests pass locally
  • Any dependent changes have been merged and published in downstream modules

Key Changes / Files Modified

Maven POMs

  • Updated all POM files with Play 2.8, Pekko 1.0.3, Scala 2.13, and supporting libraries
  • Added Scala 2.13 exclusions to prevent conflicts with Scala 2.11

Source Code

  • Migrated Akka imports to Pekko equivalents (akka.*org.apache.pekko.*)
  • Controllers updated to accept Http.Request parameters
  • Promise API converted to CompletionStage
  • Header API updated to use Optional pattern

Configuration

  • application.conf: Updated akka namespaces to pekko
  • Actor system, serialization, dispatcher references updated

Global.java Removal

  • Replaced deprecated Global.java with modules/StartModule.java using Guice
  • Preserved all startup functionality (Netty system property, telemetry, health checks)
  • Provides better lifecycle management via dependency injection

Play 2.8 API Updates

  • StartModule implements AbstractModule for Guice-based initialization
  • Filters updated to return List<EssentialFilter>
  • Routes file updated for request injection

Migration Impact

  • Business Logic: No changes; functionality preserved
  • API Compatibility: Maintained (Pekko API-compatible with Akka 2.6)
  • Code Changes: Primarily package name updates (akkapekko)
  • License: Fully compliant with Apache 2.0

Phase 2

  • Upgrade from Play Framework 2.8.22 to Play Framework 3.0.5 (Pending)

Copilot AI and others added 24 commits October 10, 2025 11:26
- Updated pom.xml with Play 3.0.5, Scala 2.13.12, and Pekko 1.0.3
- Migrated all Akka imports to Pekko (3 files)
- Updated configuration files (akka -> pekko)
- Added upgrade documentation (UPGRADE_README.md)

Co-authored-by: sntiwari1 <[email protected]>
- Updated pom.xml to Play 2.8.22 (Maven plugin compatible)
- Migrated Akka to Pekko imports (3 files)
- Updated Promise to CompletionStage API
- Updated configuration (akka -> pekko)
- Added commons-lang3 dependency
- Updated Scala filter for Play 2.8
- Build partially working - controller updates in progress

Co-authored-by: sntiwari1 <[email protected]>
…eckFilter

- Updated Global.java to extend DefaultHttpRequestHandler instead of GlobalSettings
- Fixed CompletionStage usage and request parameter handling
- Updated Filters.java to implement getFilters() method correctly
- Fixed HealthCheckFilter.scala to use akka.stream.Materializer (Play 2.8 uses Akka streams)
- Fixed all header() calls to use Optional pattern (.orElse(null))
- Added commons-lang3 dependency

Co-authored-by: sntiwari1 <[email protected]>
- Fixed Filters.java to return List<EssentialFilter> (Play 2.8 requirement)
- Simplified Global.java to basic initialization class
- Updated all controller methods to accept Http.Request parameter
- Added ThreadLocal request() helper in BaseController for backward compatibility
- Updated routes file to pass request parameter to controller methods
- Fixed all header() API calls to use Optional pattern
- Build now completes successfully with mvn clean install -DskipTests

Co-authored-by: sntiwari1 <[email protected]>
- Removed deprecated Global.java class
- Created modules/StartModule.java for Guice-based initialization
- Updated application.conf to use play.modules.enabled instead of application.global
- Fixed NullPointerException during startup by using proper module initialization
- Build and startup now work correctly with Play 2.8.22

Co-authored-by: sntiwari1 <[email protected]>
- Created .mvn/jvm.config with --add-opens flags for Java 17 compatibility
- Downgraded Jackson from 2.14.3 to 2.11.4 (Play 2.8.22 compatibility)
- Updated UPGRADE_README.md with complete build instructions
- Application now starts successfully with mvn play2:run
- Server listening on port 9000

Co-authored-by: sntiwari1 <[email protected]>
- Added detailed explanation of Global.java to StartModule.java migration
- Clarified that all functionality was preserved (no missing features)
- Explained Guice-based initialization replaces deprecated GlobalSettings
- Cleaned up README format per user requirements
- Removed version-specific details to focus on key changes

Co-authored-by: sntiwari1 <[email protected]>
Updated upgrade documentation to reflect Phase 1 changes and removed testing recommendations and known issues.
…d Production Ready (Maven Compatible - Play 3.0 Not Supported)

Upgrade to Play Framework 2.8.22 and Apache Pekko 1.0.3 - Complete and Production Ready (Maven Compatible - Play 3.0 Not Supported)
@sntiwari1 sntiwari1 changed the base branch from release-8.0.0 to release-8.1.0 October 13, 2025 11:17
@sntiwari1 sntiwari1 marked this pull request as ready for review November 4, 2025 05:59
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR upgrades the sunbird-dial-service from Play Framework 2.4.6 with Akka to Play Framework 3.0.5 with Apache Pekko 1.0.3, addressing licensing concerns and security vulnerabilities. The upgrade is executed in two phases: first to Play 2.8.22, then to 3.0.5.

Key Changes:

  • Migrated from Akka to Apache Pekko 1.0.3 (Apache 2.0 licensed)
  • Upgraded Play Framework from 2.4.6 to 3.0.5
  • Updated Scala from 2.11.12 to 2.13.12 and Java runtime from 11 to 17
  • Migrated from javax.* to jakarta.* namespaces for Jakarta EE compliance
  • Replaced deprecated Global.java with modules/StartModule.java using Guice dependency injection
  • Updated API signatures to use Http.Request parameters and CompletionStage instead of Promise

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
pom.xml Updated all dependency versions including Play Framework to 3.0.5, Pekko 1.0.3, Scala 2.13.12, Jackson 2.14.3, Guice 6.0.0, and various other libraries; added Scala library exclusions to prevent version conflicts
devops/roles/dial-service-deploy/templates/dial-service.conf.j2 Updated Akka configuration blocks to Pekko with renamed namespaces
conf/routes Added Http.Request parameter to controller method signatures for Play 3.0 compatibility
conf/application.conf Migrated from Akka to Pekko configuration, added GuiceApplicationLoader, and added secret key configuration
build/Dockerfile Updated base image from Java 11 to Java 17
app/modules/StartModule.java New Guice module replacing deprecated Global.java for application initialization
app/managers/DialcodeManager.java Updated Akka imports to Pekko
app/filters/HealthCheckFilter.scala Migrated to jakarta.inject, updated to use Pekko, and modernized Play API usage
app/elasticsearch/SearchProcessor.java Updated Akka imports to Pekko
app/elasticsearch/ElasticSearchUtil.java Updated Akka imports to Pekko
app/controllers/health/HealthCheckController.java Replaced Promise with CompletionStage return types
app/controllers/dialcode/DialcodeV4Controller.java Added Http.Request parameters and updated to CompletionStage
app/controllers/dialcode/DialcodeV3Controller.java Added Http.Request parameters and updated to CompletionStage
app/controllers/BaseController.java Added ThreadLocal for request context, updated header access to Optional pattern, and migrated from Promise to CompletionStage
app/Global.java Removed deprecated GlobalSettings class
app/Filters.java Updated to jakarta.inject, changed return type from array to List, and updated API usage
UPGRADE_README.md Added comprehensive upgrade documentation
.github/workflows/push-image.yml Updated Java version from 11 to 17
.circleci/config.yml Updated Java Docker image from 14 to 17
Comments suppressed due to low confidence (3)

conf/application.conf:65

  • The 'libraryDependencies' syntax is for SBT build files (build.sbt), not application.conf. These lines should be removed from the configuration file as they have no effect and may cause confusion. Dependencies should be declared in pom.xml for Maven projects.
libraryDependencies += javaWs

conf/application.conf:74

  • The 'libraryDependencies' syntax is for SBT build files (build.sbt), not application.conf. These lines should be removed from the configuration file as they have no effect and may cause confusion. Dependencies should be declared in pom.xml for Maven projects.
libraryDependencies += cache

pom.xml:342

  • The Maven compiler is configured for Java 11, but the documentation and Dockerfile specify Java 17 as the minimum requirement for Play 3.0.5. Update the compiler configuration to '17' to match the runtime requirement and ensure compatibility with Play Framework 3.0.5.
                    <release>11</release>

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 24 to +31

public class BaseController extends Controller {
private static ObjectMapper mapper = new ObjectMapper();

// Thread-local to store current request in Play 2.8
private static ThreadLocal<Http.Request> currentRequest = new ThreadLocal<>();

Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

ThreadLocal variables should be cleaned up to prevent memory leaks in application servers. Consider adding a cleanup mechanism or using try-finally blocks to call currentRequest.remove() after request processing completes. In Play Framework 3.0, request objects are injected directly, making ThreadLocal unnecessary and potentially problematic.

Suggested change
public class BaseController extends Controller {
private static ObjectMapper mapper = new ObjectMapper();
// Thread-local to store current request in Play 2.8
private static ThreadLocal<Http.Request> currentRequest = new ThreadLocal<>();
import java.util.function.Supplier;
public class BaseController extends Controller {
private static ObjectMapper mapper = new ObjectMapper();
// Thread-local to store current request in Play 2.8
private static ThreadLocal<Http.Request> currentRequest = new ThreadLocal<>();
/**
* Utility method to set the current request in ThreadLocal, execute the action, and ensure cleanup.
* Usage: return withRequestContext(request, () -> { ... });
*/
protected CompletionStage<Result> withRequestContext(Http.Request req, Supplier<CompletionStage<Result>> action) {
setCurrentRequest(req);
try {
return action.get();
} finally {
currentRequest.remove();
}
}

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

@Deeksha1502 Can you look into this?

Comment on lines +15 to +16
class HealthCheckFilter @Inject()(implicit val mat: Materializer, ec: ExecutionContext) extends Filter {
val baseController :BaseController = new BaseController()
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

Creating a new instance of BaseController directly violates dependency injection principles and the BaseController may not be properly initialized. Consider injecting BaseController through the constructor or using a static utility method that doesn't rely on controller instance state.

Suggested change
class HealthCheckFilter @Inject()(implicit val mat: Materializer, ec: ExecutionContext) extends Filter {
val baseController :BaseController = new BaseController()
class HealthCheckFilter @Inject()(baseController: BaseController)(implicit val mat: Materializer, ec: ExecutionContext) extends Filter {

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

@Deeksha1502 Can you look into this?

sntiwari1 and others added 2 commits November 4, 2025 11:51
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 4, 2025

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.

2 participants