Skip to content

Implement an Arithmetic Request supporting increment and decrement operations #29

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.

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

Merged
Merged
Show file tree
Hide file tree
Changes from 7 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
16 changes: 16 additions & 0 deletions Sources/SwiftMemcache/Extensions/ByteBuffer+SwiftMemcache.swift
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,22 @@ extension ByteBuffer {
self.writeInteger(UInt8.R)
}
}

if let arithmeticMode = flags.arithmeticMode {
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you can't specify a storage mode and arithmetic mode. Can we add a precondition somewhere to check for this

self.writeInteger(UInt8.whitespace)
self.writeInteger(UInt8.M)
switch arithmeticMode {
case .decrement:
self.writeInteger(UInt8.decrement)
case .increment:
self.writeInteger(UInt8.increment)
}
if let arithmeticDelta = flags.arithmeticDelta {
self.writeInteger(UInt8.whitespace)
self.writeInteger(UInt8.D)
self.writeIntegerAsASCII(arithmeticDelta)
}
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions Sources/SwiftMemcache/Extensions/UInt8+Characters.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@ extension UInt8 {
static var s: UInt8 = .init(ascii: "s")
static var g: UInt8 = .init(ascii: "g")
static var d: UInt8 = .init(ascii: "d")
static var a: UInt8 = .init(ascii: "a")
static var v: UInt8 = .init(ascii: "v")
static var T: UInt8 = .init(ascii: "T")
static var M: UInt8 = .init(ascii: "M")
static var P: UInt8 = .init(ascii: "P")
static var A: UInt8 = .init(ascii: "A")
static var E: UInt8 = .init(ascii: "E")
static var R: UInt8 = .init(ascii: "R")
static var D: UInt8 = .init(ascii: "D")
static var zero: UInt8 = .init(ascii: "0")
static var nine: UInt8 = .init(ascii: "9")
static var increment: UInt8 = .init(ascii: "+")
static var decrement: UInt8 = .init(ascii: "-")
}
54 changes: 54 additions & 0 deletions Sources/SwiftMemcache/MemcachedConnection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -424,4 +424,58 @@ public actor MemcachedConnection {
throw MemcachedConnectionError.connectionShutdown
}
}

// MARK: - Increment a Value

/// Increment the value for an existing key in the Memcache server by a specified amount.
///
/// - Parameters:
/// - key: The key for the value to increment.
/// - amount: The `UInt64` amount to increment the value by.
/// - Throws: A `MemcachedConnectionError` if the connection to the Memcached server is shut down.
public func increment(_ key: String, amount: UInt64) async throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use Int here and precondition that it is larger than 0

switch self.state {
case .initial(_, _, _, _),
.running:

var flags = MemcachedFlags()
flags.arithmeticMode = .increment
flags.arithmeticDelta = amount

let command = MemcachedRequest.ArithmeticCommand(key: key, flags: flags)
let request = MemcachedRequest.arithmetic(command)

_ = try await self.sendRequest(request)

case .finished:
throw MemcachedConnectionError.connectionShutdown
}
}

// MARK: - Decrement a Value

/// Decrement the value for an existing key in the Memcache server by a specified amount.
///
/// - Parameters:
/// - key: The key for the value to decrement.
/// - amount: The `UInt64` amount to decrement the value by.
/// - Throws: A `MemcachedConnectionError` if the connection to the Memcached server is shut down.
public func decrement(_ key: String, amount: UInt64) async throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here and adjust the doc comment as well please

switch self.state {
case .initial(_, _, _, _),
.running:

var flags = MemcachedFlags()
flags.arithmeticMode = .decrement
flags.arithmeticDelta = amount

let command = MemcachedRequest.ArithmeticCommand(key: key, flags: flags)
let request = MemcachedRequest.arithmetic(command)

_ = try await self.sendRequest(request)

case .finished:
throw MemcachedConnectionError.connectionShutdown
}
}
}
18 changes: 18 additions & 0 deletions Sources/SwiftMemcache/MemcachedFlags.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ struct MemcachedFlags {
/// The default mode is 'set'.
var storageMode: StorageMode?

/// Flag 'M' for the 'ma' (meta arithmetic) command.
///
/// Represents the mode of the 'ma' command, which determines the behavior of the arithmetic operation.
var arithmeticMode: ArithmeticMode?

/// Flag 'D' for the 'ma' (meta arithmetic) command.
///
/// Represents the delta to apply to the 'ma' command. The default value is 1.
var arithmeticDelta: UInt64?
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make this an associated value of the arithmetic mode enum please


init() {}
}

Expand All @@ -60,4 +70,12 @@ enum StorageMode: Equatable, Hashable {
case replace
}

/// Enum representing the mode for the 'ma' (meta arithmetic) command in Memcached (corresponding to the 'M' flag).
enum ArithmeticMode: Equatable, Hashable {
/// 'increment' command. If applied, it increases the numerical value of the item.
case increment
/// 'decrement' command. If applied, it decreases the numerical value of the item.
case decrement
}

extension MemcachedFlags: Hashable {}
6 changes: 6 additions & 0 deletions Sources/SwiftMemcache/MemcachedRequest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ enum MemcachedRequest {
let key: String
}

struct ArithmeticCommand {
let key: String
var flags: MemcachedFlags
}

case set(SetCommand)
case get(GetCommand)
case delete(DeleteCommand)
case arithmetic(ArithmeticCommand)
}
16 changes: 16 additions & 0 deletions Sources/SwiftMemcache/MemcachedRequestEncoder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,22 @@ struct MemcachedRequestEncoder: MessageToByteEncoder {
out.writeInteger(UInt8.whitespace)
out.writeBytes(command.key.utf8)

// write separator
out.writeInteger(UInt8.carriageReturn)
out.writeInteger(UInt8.newline)

case .arithmetic(let command):
precondition(!command.key.isEmpty, "Key must not be empty")

// write command and key
out.writeInteger(UInt8.m)
out.writeInteger(UInt8.a)
out.writeInteger(UInt8.whitespace)
out.writeBytes(command.key.utf8)

// write flags if there are any
out.writeMemcachedFlags(flags: command.flags)

// write separator
out.writeInteger(UInt8.carriageReturn)
out.writeInteger(UInt8.newline)
Expand Down
5 changes: 3 additions & 2 deletions Sources/SwiftMemcache/MemcachedValue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,15 @@ extension MemcachedValue where Self: FixedWidthInteger {
///
/// - Parameter buffer: The ByteBuffer to which the integer should be written.
public func writeToBuffer(_ buffer: inout ByteBuffer) {
buffer.writeInteger(self)
buffer.writeString(String(self))
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems wrong. If you want to store an integer we shouldn't convert it to a string. Is this the problem you encountered where you set a key and then want to increment it but the type in memcache was wrong?

Copy link
Contributor Author

@dkz2 dkz2 Aug 8, 2023

Choose a reason for hiding this comment

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

yes, for example when I called set with a value of say 100 the request that was being written looked like ms decrement 8 ... when it should look something like ms decrement 3 ... when I went to increment the value I would get this error CLIENT_ERROR cannot increment or decrement non-numeric value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could also make use of writeIntegerAsASCII here.

Copy link
Contributor

Choose a reason for hiding this comment

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

So the root problem here is that memcache is operating on the values interpreted as ASCII and not the raw byte values right? So decrement expects that the value for a key is an ASCII string representing an integer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that is correct, since the protocol is text based its wanting to receive the values as ASCII strings. we can fall back on making use of writeIntegerAsASCII and then implement one of the possible solutions we outlined here #9 to improve the performance.

}

/// Reads a FixedWidthInteger from a ByteBuffer.
///
/// - Parameter buffer: The ByteBuffer from which the value should be read.
public static func readFromBuffer(_ buffer: inout ByteBuffer) -> Self? {
return buffer.readInteger()
guard let string = buffer.readString(length: buffer.readableBytes)?.trimmingCharacters(in: .whitespacesAndNewlines) else { return nil }
return Self(string)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,62 @@ final class MemcachedIntegrationTest: XCTestCase {
}
}

func testIncrementValue() async throws {
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer {
XCTAssertNoThrow(try! group.syncShutdownGracefully())
}
let memcachedConnection = MemcachedConnection(host: "memcached", port: 11211, eventLoopGroup: group)

try await withThrowingTaskGroup(of: Void.self) { group in
group.addTask { try await memcachedConnection.run() }

// Set key and initial value
let initialValue: UInt64 = 1
try await memcachedConnection.set("increment", value: initialValue)

// Increment value
let incrementAmount: UInt64 = 100
try await memcachedConnection.increment("increment", amount: incrementAmount)

// Get new value
let newValue: UInt64? = try await memcachedConnection.get("increment")

// Check if new value is equal to initial value plus increment amount
XCTAssertEqual(newValue, initialValue + incrementAmount, "Incremented value is incorrect")

group.cancelAll()
}
}

func testDecrementValue() async throws {
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer {
XCTAssertNoThrow(try! group.syncShutdownGracefully())
}
let memcachedConnection = MemcachedConnection(host: "memcached", port: 11211, eventLoopGroup: group)

try await withThrowingTaskGroup(of: Void.self) { group in
group.addTask { try await memcachedConnection.run() }

// Set key and initial value
let initialValue: UInt64 = 100
try await memcachedConnection.set("decrement", value: initialValue)

// Increment value
let decrementAmount: UInt64 = 10
try await memcachedConnection.decrement("decrement", amount: decrementAmount)

// Get new value
let newValue: UInt64? = try await memcachedConnection.get("decrement")

// Check if new value is equal to initial value plus increment amount
XCTAssertEqual(newValue, initialValue - decrementAmount, "Incremented value is incorrect")

group.cancelAll()
}
}

func testMemcachedConnectionWithUInt() async throws {
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,4 +169,34 @@ final class MemcachedRequestEncoderTests: XCTestCase {
let expectedEncodedData = "md foo\r\n"
XCTAssertEqual(outBuffer.getString(at: 0, length: outBuffer.readableBytes), expectedEncodedData)
}

func testEncodeIncrementRequest() {
// Prepare a MemcachedRequest
var flags = MemcachedFlags()
flags.arithmeticMode = .increment
flags.arithmeticDelta = 100
let command = MemcachedRequest.ArithmeticCommand(key: "foo", flags: flags)
let request = MemcachedRequest.arithmetic(command)

// pass our request through the encoder
let outBuffer = self.encodeRequest(request)

let expectedEncodedData = "ma foo M+ D100\r\n"
XCTAssertEqual(outBuffer.getString(at: 0, length: outBuffer.readableBytes), expectedEncodedData)
}

func testEncodeDecrementRequest() {
// Prepare a MemcachedRequest
var flags = MemcachedFlags()
flags.arithmeticMode = .decrement
flags.arithmeticDelta = 100
let command = MemcachedRequest.ArithmeticCommand(key: "foo", flags: flags)
let request = MemcachedRequest.arithmetic(command)

// pass our request through the encoder
let outBuffer = self.encodeRequest(request)

let expectedEncodedData = "ma foo M- D100\r\n"
XCTAssertEqual(outBuffer.getString(at: 0, length: outBuffer.readableBytes), expectedEncodedData)
}
}