Skip to content

fix(wallet-api): remove core-wallet classpath collision#1654

Open
szijpeter wants to merge 1 commit intowalt-id:mainfrom
szijpeter:fix/wallet-api-corewallet-classpath
Open

fix(wallet-api): remove core-wallet classpath collision#1654
szijpeter wants to merge 1 commit intowalt-id:mainfrom
szijpeter:fix/wallet-api-corewallet-classpath

Conversation

@szijpeter
Copy link
Copy Markdown

@szijpeter szijpeter commented Apr 13, 2026

Summary

  • remove waltid-core-wallet from wallet-api runtime dependencies
  • add local UuidSerializer in wallet-api
  • switch wallet-api auth controller to use local serializer

Why

wallet-api and waltid-core-wallet both publish classes under id.walt.webwallet.usecase.exchange.* with different constructor signatures. At runtime this can load the wrong class and crash WalletServiceManager init (NoSuchMethodError -> NoClassDefFoundError), breaking registration flow.

Verification

  • ./gradlew :waltid-services:waltid-wallet-api:compileKotlin
  • ./gradlew :waltid-services:waltid-wallet-api:dependencies --configuration runtimeClasspath | rg "waltid-core-wallet" (no matches)

Context

Isolated hotfix for #1608

Summary by CodeRabbit

  • Refactor
    • Updated internal module dependencies and serialization utilities for improved code organization and maintenance.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 13, 2026

📝 Walkthrough

Walkthrough

The PR removes the waltid-core-wallet dependency from the waltid-wallet-api module and introduces a local UuidSerializer implementation to replace the previously imported one. The AuthController is updated to reference the new local serializer instead.

Changes

Cohort / File(s) Summary
Build Configuration
waltid-services/waltid-wallet-api/build.gradle.kts
Removed the project(":waltid-libraries:waltid-core-wallet") dependency from the module's dependency declarations.
UUID Serialization
waltid-services/waltid-wallet-api/src/main/kotlin/id/walt/webwallet/utils/UuidSerializer.kt
Added new UuidSerializer object implementing KSerializer<Uuid> with string-based serialization/deserialization logic for UUID values. Marked as temporary pending kotlinx.serialization update.
Controller Update
waltid-services/waltid-wallet-api/src/main/kotlin/id/walt/webwallet/web/controllers/auth/AuthController.kt
Updated LoginResponseData to use the local UuidSerializer instead of the one from the removed dependency.

Possibly related issues

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description provides a clear summary, explains the root cause of the issue, and includes verification steps, but does not follow the provided template structure with Type of Change and Checklist sections. Consider filling out the template sections (Type of Change: bug fix, and Checklist items) to ensure consistency with repository standards.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: removing a classpath collision caused by the core-wallet dependency.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
waltid-services/waltid-wallet-api/src/main/kotlin/id/walt/webwallet/utils/UuidSerializer.kt (1)

16-16: Consider making this temporary serializer internal.

This keeps the workaround module-local and avoids accidental use as a long-lived public API.

♻️ Suggested visibility tightening
- object UuidSerializer : KSerializer<Uuid> {
+ internal object UuidSerializer : KSerializer<Uuid> {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@waltid-services/waltid-wallet-api/src/main/kotlin/id/walt/webwallet/utils/UuidSerializer.kt`
at line 16, The UuidSerializer object is currently public; make it module-local
by changing its visibility to internal (i.e., declare internal object
UuidSerializer : KSerializer<Uuid>) so the temporary workaround isn't exposed as
a public API; update any usages in the same module if necessary to ensure they
still resolve.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@waltid-services/waltid-wallet-api/src/main/kotlin/id/walt/webwallet/utils/UuidSerializer.kt`:
- Line 16: The UuidSerializer object is currently public; make it module-local
by changing its visibility to internal (i.e., declare internal object
UuidSerializer : KSerializer<Uuid>) so the temporary workaround isn't exposed as
a public API; update any usages in the same module if necessary to ensure they
still resolve.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fb3f17a7-8a01-4dbb-88ef-edbe39d7eab2

📥 Commits

Reviewing files that changed from the base of the PR and between 3a0ab00 and dccfcb6.

📒 Files selected for processing (3)
  • waltid-services/waltid-wallet-api/build.gradle.kts
  • waltid-services/waltid-wallet-api/src/main/kotlin/id/walt/webwallet/utils/UuidSerializer.kt
  • waltid-services/waltid-wallet-api/src/main/kotlin/id/walt/webwallet/web/controllers/auth/AuthController.kt
💤 Files with no reviewable changes (1)
  • waltid-services/waltid-wallet-api/build.gradle.kts

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.

1 participant