Skip to content

Commit

Permalink
Fixes an issue where empty values were treated as null values and vic…
Browse files Browse the repository at this point in the history
…a versa (#144)
  • Loading branch information
fabianfett authored Feb 26, 2021
1 parent cdb18d1 commit fa93d76
Show file tree
Hide file tree
Showing 11 changed files with 74 additions and 20 deletions.
2 changes: 1 addition & 1 deletion Sources/PostgresNIO/New/Data/Array+PSQLCodable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ extension Array: PSQLEncodable where Element: PSQLArrayElement {
buffer.writeInteger(1, as: Int32.self)

try self.forEach { element in
try element._encode(into: &buffer, context: context)
try element.encodeRaw(into: &buffer, context: context)
}
}
}
Expand Down
8 changes: 6 additions & 2 deletions Sources/PostgresNIO/New/Data/Optional+PSQLCodable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@ extension Optional: PSQLEncodable where Wrapped: PSQLEncodable {
}

func encode(into byteBuffer: inout ByteBuffer, context: PSQLEncodingContext) throws {
preconditionFailure("Should never be hit, since `encodeRaw` is implemented.")
}

func encodeRaw(into byteBuffer: inout ByteBuffer, context: PSQLEncodingContext) throws {
switch self {
case .none:
return
byteBuffer.writeInteger(-1, as: Int32.self)
case .some(let value):
try value.encode(into: &byteBuffer, context: context)
try value.encodeRaw(into: &byteBuffer, context: context)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/PostgresNIO/New/Messages/Bind.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ extension PSQLFrontendMessage {
let context = PSQLEncodingContext(jsonEncoder: jsonEncoder)

try self.parameters.forEach { parameter in
try parameter._encode(into: &buffer, context: context)
try parameter.encodeRaw(into: &buffer, context: context)
}

// The number of result-column format codes that follow (denoted R below). This can be
Expand Down
2 changes: 1 addition & 1 deletion Sources/PostgresNIO/New/Messages/DataRow.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ extension PSQLBackendMessage {
try PSQLBackendMessage.ensureAtLeastNBytesRemaining(2, in: buffer)
let bufferLength = Int(buffer.readInteger(as: Int32.self)!)

guard bufferLength > 0 else {
guard bufferLength >= 0 else {
result.append(nil)
continue
}
Expand Down
10 changes: 8 additions & 2 deletions Sources/PostgresNIO/New/PSQLCodable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@ protocol PSQLEncodable {
/// identifies the data type that we will encode into `byteBuffer` in `encode`
var psqlType: PSQLDataType { get }

/// encoding the entity into the `byteBuffer` in postgres binary format
/// Encode the entity into the `byteBuffer` in Postgres binary format, without setting
/// the byte count. This method is called from the default `encodeRaw` implementation.
func encode(into byteBuffer: inout ByteBuffer, context: PSQLEncodingContext) throws

/// Encode the entity into the `byteBuffer` in Postgres binary format including its
/// leading byte count. This method has a default implementation and may be overriden
/// only for special cases, like `Optional`s.
func encodeRaw(into byteBuffer: inout ByteBuffer, context: PSQLEncodingContext) throws
}

/// A type that can decode itself from a postgres wire binary representation.
Expand All @@ -18,7 +24,7 @@ protocol PSQLDecodable {
protocol PSQLCodable: PSQLEncodable, PSQLDecodable {}

extension PSQLEncodable {
func _encode(into buffer: inout ByteBuffer, context: PSQLEncodingContext) throws {
func encodeRaw(into buffer: inout ByteBuffer, context: PSQLEncodingContext) throws {
// The length of the parameter value, in bytes (this count does not include
// itself). Can be zero.
let lengthIndex = buffer.writerIndex
Expand Down
15 changes: 11 additions & 4 deletions Sources/PostgresNIO/Postgres+PSQLCompat.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,19 @@ extension PostgresData: PSQLEncodable {
PSQLDataType(Int32(self.type.rawValue))
}

// encoding
func encode(into byteBuffer: inout ByteBuffer, context: PSQLEncodingContext) throws {
guard var selfBuffer = self.value else {
return
preconditionFailure("Should never be hit, since `encodeRaw` is implemented.")
}

// encoding
func encodeRaw(into byteBuffer: inout ByteBuffer, context: PSQLEncodingContext) throws {
switch self.value {
case .none:
byteBuffer.writeInteger(-1, as: Int32.self)
case .some(var input):
byteBuffer.writeInteger(Int32(input.readableBytes))
byteBuffer.writeBuffer(&input)
}
byteBuffer.writeBuffer(&selfBuffer)
}
}

Expand Down
11 changes: 6 additions & 5 deletions Tests/PostgresNIOTests/New/Data/Optional+PSQLCodableTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ class Optional_PSQLCodableTests: XCTestCase {

var buffer = ByteBuffer()
XCTAssertEqual(encodable.psqlType, .uuid)
XCTAssertNoThrow(try encodable.encode(into: &buffer, context: .forTests()))
XCTAssertEqual(buffer.readableBytes, 16)

XCTAssertNoThrow(try encodable.encodeRaw(into: &buffer, context: .forTests()))
XCTAssertEqual(buffer.readableBytes, 20)
XCTAssertEqual(buffer.readInteger(as: Int32.self), 16)
let data = PSQLData(bytes: buffer, dataType: .uuid)

var result: UUID?
Expand All @@ -53,8 +53,9 @@ class Optional_PSQLCodableTests: XCTestCase {

var buffer = ByteBuffer()
XCTAssertEqual(encodable.psqlType, .null)
XCTAssertNoThrow(try encodable.encode(into: &buffer, context: .forTests()))
XCTAssertEqual(buffer.readableBytes, 0)
XCTAssertNoThrow(try encodable.encodeRaw(into: &buffer, context: .forTests()))
XCTAssertEqual(buffer.readableBytes, 4)
XCTAssertEqual(buffer.readInteger(as: Int32.self), -1)

let data = PSQLData(bytes: nil, dataType: .uuid)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ extension ConnectionStateMachine.ConnectionAction: Equatable {
let encodingContext = PSQLEncodingContext(jsonEncoder: JSONEncoder())

do {
try lhs._encode(into: &lhsbuffer, context: encodingContext)
try rhs._encode(into: &rhsbuffer, context: encodingContext)
try lhs.encodeRaw(into: &lhsbuffer, context: encodingContext)
try rhs.encodeRaw(into: &rhsbuffer, context: encodingContext)
} catch {
return false
}
Expand Down
11 changes: 10 additions & 1 deletion Tests/PostgresNIOTests/New/Messages/DataRowTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,23 @@ import XCTest
class DataRowTests: XCTestCase {
func testDecode() {
let buffer = ByteBuffer.backendMessage(id: .dataRow) { buffer in
buffer.writeInteger(2, as: Int16.self)
// the data row has 3 columns
buffer.writeInteger(3, as: Int16.self)

// this is a null value
buffer.writeInteger(-1, as: Int32.self)

// this is an empty value. for example a empty string
buffer.writeInteger(0, as: Int32.self)

// this is a column with ten bytes
buffer.writeInteger(10, as: Int32.self)
buffer.writeBytes([UInt8](repeating: 5, count: 10))
}

let expectedColumns: [ByteBuffer?] = [
nil,
ByteBuffer(),
ByteBuffer(bytes: [UInt8](repeating: 5, count: 10))
]

Expand Down
27 changes: 27 additions & 0 deletions Tests/PostgresNIOTests/PostgresNIOTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,33 @@ final class PostgresNIOTests: XCTestCase {
]).wait())
XCTAssertEqual(rows?.first?.column("array")?.array(of: Int.self), [])
}

// https://github.com/vapor/postgres-nio/issues/143
func testEmptyStringFromNonNullColumn() {
var conn: PostgresConnection?
XCTAssertNoThrow(conn = try PostgresConnection.test(on: eventLoop).wait())
defer { XCTAssertNoThrow( try conn?.close().wait() ) }

XCTAssertNoThrow(_ = try conn?.simpleQuery(#"DROP TABLE IF EXISTS "non_null_empty_strings""#).wait())
XCTAssertNoThrow(_ = try conn?.simpleQuery("""
CREATE TABLE non_null_empty_strings (
"id" SERIAL,
"nonNullString" text NOT NULL,
PRIMARY KEY ("id")
);
""").wait())
defer { XCTAssertNoThrow(_ = try conn?.simpleQuery(#"DROP TABLE "non_null_empty_strings""#).wait()) }

XCTAssertNoThrow(_ = try conn?.simpleQuery("""
INSERT INTO non_null_empty_strings ("nonNullString") VALUES ('')
""").wait())

var rows: [PostgresRow]?
XCTAssertNoThrow(rows = try conn?.simpleQuery(#"SELECT * FROM "non_null_empty_strings""#).wait())
XCTAssertEqual(rows?.count, 1)
XCTAssertEqual(rows?.first?.column("nonNullString")?.string, "") // <--- this fails
}


func testBoolSerialize() {
var conn: PostgresConnection?
Expand Down
2 changes: 1 addition & 1 deletion Tests/PostgresNIOTests/Utilities.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ extension PostgresConnection {
}
}

static func test(on eventLoop: EventLoop, logLevel: Logger.Level = .trace) -> EventLoopFuture<PostgresConnection> {
static func test(on eventLoop: EventLoop, logLevel: Logger.Level = .info) -> EventLoopFuture<PostgresConnection> {
return testUnauthenticated(on: eventLoop, logLevel: logLevel).flatMap { conn in
return conn.authenticate(
username: env("POSTGRES_USER") ?? "vapor_username",
Expand Down

0 comments on commit fa93d76

Please sign in to comment.