review feedback

This commit is contained in:
Konrad `ktoso` Malawski 2020-09-11 11:59:06 +09:00
parent 920644b013
commit a8bbc79d4c
4 changed files with 25 additions and 26 deletions

View File

@ -22,7 +22,7 @@
/// typealias Value = String
/// }
///
/// var context = BaggageContext.empty
/// var context = BaggageContext.background
/// // set a new value
/// context[TestIDKey.self] = "abc"
/// // retrieve a stored value
@ -49,8 +49,6 @@ public struct BaggageContext: BaggageContextProtocol {
/// Internal on purpose, please use `TODO` or `.background` to create an "empty" context,
/// which carries more meaning to other developers why an empty context was used.
///
/// Frameworks and libraries which create a new baggage to populate it right away can start
init() {}
public subscript<Key: BaggageContextKey>(_ key: Key.Type) -> Key.Value? {
@ -179,16 +177,17 @@ extension BaggageContextProtocol {
/// request headers.
///
/// ### Usage in applications
/// More often than not application code should never have to create an empty context.
/// Application code should never have to create an empty context during the processing lifetime of any request,
/// and only should create contexts if some processing is performed in the background - thus the naming of this property.
///
/// Usually, a framework such as an HTTP server or similar "request handler" would already provide users
/// with a context to be passed along through subsequent calls.
///
/// If unsure where to obtain a context from, prefer using `TODO("Not sure where I should get a context from here?")`,
/// If unsure where to obtain a context from, prefer using `.TODO("Not sure where I should get a context from here?")`,
/// such that other developers are informed that the lack of context was not done on purpose, but rather because either
/// not being sure where to obtain a context from, or other framework limitations -- e.g. the outer framework not being
/// context aware just yet.
public static var empty: BaggageContext {
public static var background: BaggageContext {
return BaggageContext()
}
}
@ -206,7 +205,7 @@ extension BaggageContextProtocol {
/// ### Crashing on TO-DO context creation
/// You may set the `BAGGAGE_CRASH_TODOS` variable while compiling a project in order to make calls to this function crash
/// with a fatal error, indicating where a to-do baggage context was used. This comes in handy when wanting to ensure that
/// a project never ends up using with code which initially was written as "was lazy, did not pass context", yet the
/// a project never ends up using with code initially was written as "was lazy, did not pass context", yet the
/// project requires context passing to be done correctly throughout the application. Similar checks can be performed
/// at compile time easily using linters (not yet implemented), since it is always valid enough to detect a to-do context
/// being passed as illegal and warn or error when spotted.
@ -214,10 +213,10 @@ extension BaggageContextProtocol {
/// - Parameters:
/// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one,
/// - Returns: Empty "to-do" baggage context which should be eventually replaced with a carried through one, or `background`.
public static func TODO(_ reason: String, function: String = #function, file: String = #file, line: UInt = #line) -> BaggageContext {
var context = BaggageContext.empty
public static func TODO(_ reason: StaticString? = "", function: String = #function, file: String = #file, line: UInt = #line) -> BaggageContext {
var context = BaggageContext.background
#if BAGGAGE_CRASH_TODOS
fatalError("BAGGAGE_CRASH_TODOS: at \(file):\(line) (function \(function))")
fatalError("BAGGAGE_CRASH_TODOS: at \(file):\(line) (function \(function)), reason: \(reason)")
#else
context[TODOKey.self] = .init(file: file, line: line)
return context
@ -228,7 +227,7 @@ extension BaggageContextProtocol {
internal enum TODOKey: BaggageContextKey {
typealias Value = TODOLocation
static var name: String? {
return "todo-loc"
return "todo"
}
}

View File

@ -21,7 +21,7 @@ public let BaggagePassingBenchmarks: [BenchmarkInfo] = [
BenchmarkInfo(
name: "BaggagePassingBenchmarks.pass_async_empty_100_000 ",
runFunction: { _ in
let context = BaggageContext.empty
let context = BaggageContext.background
pass_async(context: context, times: 100_000)
},
tags: [],
@ -31,7 +31,7 @@ public let BaggagePassingBenchmarks: [BenchmarkInfo] = [
BenchmarkInfo(
name: "BaggagePassingBenchmarks.pass_async_smol_100_000 ",
runFunction: { _ in
var context = BaggageContext.empty
var context = BaggageContext.background
context.k1 = "one"
context.k2 = "two"
context.k3 = "three"
@ -45,7 +45,7 @@ public let BaggagePassingBenchmarks: [BenchmarkInfo] = [
BenchmarkInfo(
name: "BaggagePassingBenchmarks.pass_async_small_nonconst_100_000",
runFunction: { _ in
var context = BaggageContext.empty
var context = BaggageContext.background
context.k1 = "\(Int.random(in: 1 ... Int.max))"
context.k2 = "\(Int.random(in: 1 ... Int.max))"
context.k3 = "\(Int.random(in: 1 ... Int.max))"
@ -65,7 +65,7 @@ public let BaggagePassingBenchmarks: [BenchmarkInfo] = [
BenchmarkInfo(
name: "BaggagePassingBenchmarks.pass_mut_async_small_100_000 ",
runFunction: { _ in
var context = BaggageContext.empty
var context = BaggageContext.background
context.k1 = "\(Int.random(in: 1 ... Int.max))"
context.k2 = "\(Int.random(in: 1 ... Int.max))"
context.k3 = "\(Int.random(in: 1 ... Int.max))"

View File

@ -18,7 +18,7 @@ import XCTest
final class LoggingBaggageContextCarrierTests: XCTestCase {
func test_ContextWithLogger_dumpBaggage() throws {
let baggage = BaggageContext.empty
let baggage = BaggageContext.background
let logger = Logger(label: "TheLogger")
var context: LoggingBaggageContextCarrier = ExampleFrameworkContext(context: baggage, logger: logger)
@ -43,7 +43,7 @@ final class LoggingBaggageContextCarrierTests: XCTestCase {
}
func test_ContextWithLogger_log_withBaggage() throws {
let baggage = BaggageContext.empty
let baggage = BaggageContext.background
let logging = TestLogging()
let logger = Logger(label: "TheLogger", factory: { label in logging.make(label: label) })
@ -66,7 +66,7 @@ final class LoggingBaggageContextCarrierTests: XCTestCase {
}
func test_ContextWithLogger_log_prefersBaggageContextOverExistingLoggerMetadata() {
let baggage = BaggageContext.empty
let baggage = BaggageContext.background
let logging = TestLogging()
var logger = Logger(label: "TheLogger", factory: { label in logging.make(label: label) })
logger[metadataKey: "secondIDExplicitlyNamed"] = "set on logger"
@ -103,7 +103,7 @@ struct CoolFrameworkContext: LoggingBaggageContextCarrier {
return self._logger.with(context: self.baggage)
}
var baggage: BaggageContext = .empty
var baggage: BaggageContext = .background
// framework context defines other values as well
let frameworkField: String = ""

View File

@ -18,7 +18,7 @@ final class BaggageContextTests: XCTestCase {
func testSubscriptAccess() {
let testID = 42
var baggage = BaggageContext.empty
var baggage = BaggageContext.background
XCTAssertNil(baggage[TestIDKey.self])
baggage[TestIDKey.self] = testID
@ -31,7 +31,7 @@ final class BaggageContextTests: XCTestCase {
func testRecommendedConvenienceExtension() {
let testID = 42
var baggage = BaggageContext.empty
var baggage = BaggageContext.background
XCTAssertNil(baggage.testID)
baggage.testID = testID
@ -42,18 +42,18 @@ final class BaggageContextTests: XCTestCase {
}
func testEmptyBaggageDescription() {
XCTAssertEqual(String(describing: BaggageContext.empty), "BaggageContext(keys: [])")
XCTAssertEqual(String(describing: BaggageContext.background), "BaggageContext(keys: [])")
}
func testSingleKeyBaggageDescription() {
var baggage = BaggageContext.empty
var baggage = BaggageContext.background
baggage.testID = 42
XCTAssertEqual(String(describing: baggage), #"BaggageContext(keys: ["TestIDKey"])"#)
}
func testMultiKeysBaggageDescription() {
var baggage = BaggageContext.empty
var baggage = BaggageContext.background
baggage.testID = 42
baggage[SecondTestIDKey.self] = "test"
@ -80,7 +80,7 @@ final class BaggageContextTests: XCTestCase {
}
func test_todo_empty() {
let context = BaggageContext.empty
let context = BaggageContext.background
_ = context // avoid "not used" warning
// TODO: Can't work with protocols; re-consider the entire carrier approach... Context being a Baggage + Logger, and a specific type.
@ -88,7 +88,7 @@ final class BaggageContextTests: XCTestCase {
// func take(context: BaggageContextProtocol) {
// _ = context // ignore
// }
// take(context: .empty)
// take(context: .background)
}
}