Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add ServerSeq into ChangeInfo #179

Merged
merged 1 commit into from
Jun 10, 2024
Merged

Add ServerSeq into ChangeInfo #179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 23 additions & 8 deletions Sources/API/Converter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ extension Converter {
static func fromChangeID(_ pbChangeID: PbChangeID) -> ChangeID {
ChangeID(clientSeq: pbChangeID.clientSeq,
lamport: pbChangeID.lamport,
actor: pbChangeID.actorID.toHexString)
actor: pbChangeID.actorID.toHexString,
serverSeq: pbChangeID.serverSeq)
Comment on lines +193 to +194
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

The function definition of fromChangeID in Sources/API/Converter.swift includes the new serverSeq parameter, and its usage in fromChanges matches this new signature. However, the test case in Tests/Unit/API/V1/ConverterTests.swift does not include the serverSeq parameter.

  • Tests/Unit/API/V1/ConverterTests.swift: Update the test case to include the serverSeq parameter when calling fromChangeID.
Analysis chain

The addition of serverSeq to fromChangeID enhances change tracking capabilities. Ensure all calls to this function are updated accordingly.

The search results indicate multiple occurrences of fromChangeID in Sources/API/Converter.swift and Tests/Unit/API/V1/ConverterTests.swift. We need to verify that each of these occurrences matches the new function signature, which includes the serverSeq parameter.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `fromChangeID` match the new signature.

# Test: Search for the function usage. Expect: Only occurances of the new signature.
rg --type swift $'fromChangeID'

Length of output: 419

Script:

#!/bin/bash
# Description: Verify that all occurrences of `fromChangeID` include the new `serverSeq` parameter.

# Check the specific lines in the files where `fromChangeID` is used.
rg --type swift --context 5 'fromChangeID' Sources/API/Converter.swift Tests/Unit/API/V1/ConverterTests.swift

Length of output: 2640

}
}

Expand Down Expand Up @@ -510,13 +511,27 @@ extension Converter {
executedAt: fromTimeTicket(pbTreeEditOperation.executedAt),
maxCreatedAtMapByActor: createdAtMapByActor)
} else if case let .treeStyle(pbTreeStyleOperation) = pbOperation.body {
return TreeStyleOperation(parentCreatedAt: fromTimeTicket(pbTreeStyleOperation.parentCreatedAt),
fromPos: fromTreePos(pbTreeStyleOperation.from),
toPos: fromTreePos(pbTreeStyleOperation.to),
maxCreatedAtMapByActor: pbTreeStyleOperation.createdAtMapByActor.compactMapValues({ fromTimeTicket($0) }),
attributes: pbTreeStyleOperation.attributes,
attributesToRemove: pbTreeStyleOperation.attributesToRemove,
executedAt: fromTimeTicket(pbTreeStyleOperation.executedAt))
var createdAtMapByActor = [String: TimeTicket]()
pbTreeStyleOperation.createdAtMapByActor.forEach { key, value in
createdAtMapByActor[key] = fromTimeTicket(value)
}
if !pbTreeStyleOperation.attributesToRemove.isEmpty {
return TreeStyleOperation(parentCreatedAt: fromTimeTicket(pbTreeStyleOperation.parentCreatedAt),
fromPos: fromTreePos(pbTreeStyleOperation.from),
toPos: fromTreePos(pbTreeStyleOperation.to),
maxCreatedAtMapByActor: createdAtMapByActor,
attributes: [:],
attributesToRemove: pbTreeStyleOperation.attributesToRemove,
executedAt: fromTimeTicket(pbTreeStyleOperation.executedAt))
} else {
return TreeStyleOperation(parentCreatedAt: fromTimeTicket(pbTreeStyleOperation.parentCreatedAt),
fromPos: fromTreePos(pbTreeStyleOperation.from),
toPos: fromTreePos(pbTreeStyleOperation.to),
maxCreatedAtMapByActor: createdAtMapByActor,
attributes: pbTreeStyleOperation.attributes,
attributesToRemove: [],
executedAt: fromTimeTicket(pbTreeStyleOperation.executedAt))
}
} else {
throw YorkieError.unimplemented(message: "unimplemented operation \(pbOperation)")
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/Document/Change/Change.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public struct Change {
}

self.operations = operations
self.id.setActor(actorID)
self.id = self.id.setActor(actorID)
}

/**
Expand Down
18 changes: 15 additions & 3 deletions Sources/Document/Change/ChangeID.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ struct ChangeID {
private var lamport: Int64
private var actor: ActorID

init(clientSeq: UInt32, lamport: Int64, actor: ActorID) {
init(clientSeq: UInt32, lamport: Int64, actor: ActorID, serverSeq: Int64? = nil) {
self.clientSeq = clientSeq
self.lamport = lamport
self.actor = actor
self.serverSeq = serverSeq
}

/**
Expand Down Expand Up @@ -69,8 +70,8 @@ struct ChangeID {
/**
* `setActor` sets the given actor.
*/
mutating func setActor(_ actorID: ActorID) {
self.actor = actorID
func setActor(_ actorID: ActorID) -> ChangeID {
ChangeID(clientSeq: self.clientSeq, lamport: self.lamport, actor: actorID, serverSeq: self.serverSeq)
}

/**
Expand All @@ -80,6 +81,17 @@ struct ChangeID {
return self.clientSeq
}

/**
* `getServerSeq` returns the server sequence of this ID.
*/
func getServerSeq() -> String {
if let serverSeq {
return String(serverSeq)
} else {
return ""
}
}
Comment on lines +84 to +93
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider returning an optional String? from getServerSeq instead of an empty string to allow explicit handling of absent values.


/**
* `getLamport` returns the lamport clock of this ID.
*/
Expand Down
2 changes: 2 additions & 0 deletions Sources/Document/DocEvent.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ public struct ChangeInfo {
public let message: String
public let operations: [any OperationInfo]
public let actorID: ActorID?
public let clientSeq: UInt32
public let serverSeq: String
}

/**
Expand Down
18 changes: 14 additions & 4 deletions Sources/Document/Document.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,9 @@ public actor Document {
if !opInfos.isEmpty {
let changeInfo = ChangeInfo(message: change.message ?? "",
operations: opInfos,
actorID: actorID)
actorID: actorID,
clientSeq: change.id.getClientSeq(),
serverSeq: change.id.getServerSeq())
let changeEvent = LocalChangeEvent(value: changeInfo)
self.publish(changeEvent)
}
Expand Down Expand Up @@ -322,7 +324,7 @@ public actor Document {

self.localChanges = changes

self.changeID.setActor(actorID)
self.changeID = self.changeID.setActor(actorID)

// TODOs also apply into root.
}
Expand Down Expand Up @@ -484,7 +486,11 @@ public actor Document {
let opInfos = try change.execute(root: self.root, presences: &self.presences)

if change.hasOperations {
changeInfo = ChangeInfo(message: change.message ?? "", operations: opInfos, actorID: actorID)
changeInfo = ChangeInfo(message: change.message ?? "",
operations: opInfos,
actorID: actorID,
clientSeq: change.id.getClientSeq(),
serverSeq: change.id.getServerSeq())
}

// DocEvent should be emitted synchronously with applying changes.
Expand Down Expand Up @@ -650,7 +656,11 @@ public actor Document {
}

for (key, value) in operations {
let info = ChangeInfo(message: event.value.message, operations: value, actorID: event.value.actorID)
let info = ChangeInfo(message: event.value.message,
operations: value,
actorID: event.value.actorID,
clientSeq: event.value.clientSeq,
serverSeq: event.value.serverSeq)

if let callback = self.subscribeCallbacks[key] {
callback(event.type == .localChange ? LocalChangeEvent(value: info) : RemoteChangeEvent(value: info), self)
Expand Down