Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
package io.github.kamiazya.scopes.devicesync.domain.entity

import io.github.kamiazya.scopes.devicesync.domain.valueobject.ConflictId
import io.github.kamiazya.scopes.devicesync.domain.valueobject.ConflictType
import io.github.kamiazya.scopes.devicesync.domain.valueobject.ResolutionAction
import io.github.kamiazya.scopes.devicesync.domain.valueobject.VectorClock
import kotlinx.datetime.Instant
import org.jmolecules.ddd.types.Entity

/**
* Represents a synchronization conflict with rich domain logic for resolution.
*
* This entity encapsulates the business rules for conflict detection, analysis,
* and resolution strategies.
*
*/
data class SyncConflict(
private val _id: ConflictId,
val localEventId: String,
val remoteEventId: String,
val aggregateId: String,
Expand All @@ -23,7 +27,13 @@ data class SyncConflict(
val detectedAt: Instant,
val resolvedAt: Instant? = null,
val resolution: ResolutionAction? = null,
) {
) : Entity<SyncState, ConflictId> {

/**
* Use getId() to access the conflict ID.
*/
override fun getId(): ConflictId = _id

init {
require(localEventId.isNotBlank()) { "Local event ID cannot be blank" }
require(remoteEventId.isNotBlank()) { "Remote event ID cannot be blank" }
Expand Down Expand Up @@ -180,6 +190,7 @@ data class SyncConflict(
require(remoteVersion >= 0) { "Remote version must be non-negative" }

return SyncConflict(
_id = ConflictId.generate(),
localEventId = localEventId,
remoteEventId = remoteEventId,
aggregateId = aggregateId,
Expand All @@ -191,6 +202,37 @@ data class SyncConflict(
detectedAt = detectedAt,
)
}

/**
* Create a new SyncConflict for testing purposes.
* Auto-generates an ID.
*/
fun create(
localEventId: String,
remoteEventId: String,
aggregateId: String,
localVersion: Long,
remoteVersion: Long,
localVectorClock: VectorClock,
remoteVectorClock: VectorClock,
conflictType: ConflictType,
detectedAt: Instant,
resolvedAt: Instant? = null,
resolution: ResolutionAction? = null,
): SyncConflict = SyncConflict(
_id = ConflictId.generate(),
localEventId = localEventId,
remoteEventId = remoteEventId,
aggregateId = aggregateId,
localVersion = localVersion,
remoteVersion = remoteVersion,
localVectorClock = localVectorClock,
remoteVectorClock = remoteVectorClock,
conflictType = conflictType,
detectedAt = detectedAt,
resolvedAt = resolvedAt,
resolution = resolution,
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,36 @@ package io.github.kamiazya.scopes.devicesync.domain.entity
import io.github.kamiazya.scopes.devicesync.domain.valueobject.DeviceId
import io.github.kamiazya.scopes.devicesync.domain.valueobject.VectorClock
import kotlinx.datetime.Instant
import org.jmolecules.ddd.types.AggregateRoot
import kotlin.time.Duration

/**
* Represents the synchronization state between this device and another device.
*
* This entity encapsulates the business logic for managing synchronization state,
* including state transitions, sync readiness checks, and error handling.
*
* Each SyncState is uniquely identified by the remote DeviceId it syncs with.
*/
data class SyncState(
val deviceId: DeviceId,
private val _deviceId: DeviceId,
val lastSyncAt: Instant?,
val remoteVectorClock: VectorClock,
val lastSuccessfulPush: Instant?,
val lastSuccessfulPull: Instant?,
val syncStatus: SyncStatus,
val pendingChanges: Int = 0,
) {
) : AggregateRoot<SyncState, DeviceId> {
Comment on lines 14 to +25
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add @ConsistentCopyVisibility annotation for consistency.

Similar to AspectDefinition.kt (line 23), this data class uses a private backing field (_deviceId) for identity but lacks the @ConsistentCopyVisibility annotation. Without it, the generated copy() method may expose the private constructor parameter, potentially allowing external code to modify the aggregate's identity.

Apply this diff:

+@ConsistentCopyVisibility
 data class SyncState(
     private val _deviceId: DeviceId,
📝 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.

Suggested change
data class SyncState(
val deviceId: DeviceId,
private val _deviceId: DeviceId,
val lastSyncAt: Instant?,
val remoteVectorClock: VectorClock,
val lastSuccessfulPush: Instant?,
val lastSuccessfulPull: Instant?,
val syncStatus: SyncStatus,
val pendingChanges: Int = 0,
) {
) : AggregateRoot<SyncState, DeviceId> {
@ConsistentCopyVisibility
data class SyncState(
private val _deviceId: DeviceId,
val lastSyncAt: Instant?,
val remoteVectorClock: VectorClock,
val lastSuccessfulPush: Instant?,
val lastSuccessfulPull: Instant?,
val syncStatus: SyncStatus,
val pendingChanges: Int = 0,
) : AggregateRoot<SyncState, DeviceId> {
//
}
🤖 Prompt for AI Agents
In
contexts/device-synchronization/domain/src/main/kotlin/io/github/kamiazya/scopes/devicesync/domain/entity/SyncState.kt
around lines 17 to 25, the data class uses a private backing field `_deviceId`
for identity but is missing the @ConsistentCopyVisibility annotation; add the
@ConsistentCopyVisibility annotation immediately above the data class
declaration (and import it if needed) so the generated copy() keeps the same
visibility for the private constructor parameter and prevents external code from
altering the aggregate identity.


/**
*/
override fun getId(): DeviceId = _deviceId
Comment on lines +27 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The KDoc for the overridden getId() method is empty. While it's an override, providing clear documentation improves code clarity and maintainability, especially for a core concept like an aggregate's ID. Please add a KDoc explaining what this ID represents for a SyncState.

Suggested change
/**
*/
override fun getId(): DeviceId = _deviceId
/**
* The unique identifier for this sync state, which corresponds to the remote device's ID.
*/
override fun getId(): DeviceId = _deviceId


/**
* Public accessor for deviceId.
*/
val deviceId: DeviceId
get() = _deviceId
init {
require(pendingChanges >= 0) { "Pending changes cannot be negative" }
lastSuccessfulPush?.let { push ->
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
package io.github.kamiazya.scopes.devicesync.domain.valueobject

import io.github.kamiazya.scopes.platform.commons.id.ULID
import org.jmolecules.ddd.types.Identifier

/**
* Represents a unique identifier for a device in the multi-device synchronization system.
*
* Each device participating in synchronization has a unique ID that is used to track
* which events originated from which device and to maintain vector clocks.
*
*/
@JvmInline
value class DeviceId(val value: String) {
value class DeviceId(val value: String) : Identifier {
init {
require(value.isNotBlank()) { "Device ID cannot be blank" }
require(value.length <= 64) { "Device ID cannot exceed 64 characters" }
Expand Down
Loading
Loading