Skip to content

Extended RestOptions with dateDecodingStrategy so for each request th… - #19

Merged
tkausch merged 1 commit into
mainfrom
MakeDateEncodingAndDecodingConfigurable
Nov 17, 2025
Merged

Extended RestOptions with dateDecodingStrategy so for each request th…#19
tkausch merged 1 commit into
mainfrom
MakeDateEncodingAndDecodingConfigurable

Conversation

@tkausch

@tkausch tkausch commented Nov 11, 2025

Copy link
Copy Markdown
Owner

User description

…is can be set by default using .iso8601


PR Type

Enhancement


Description

  • Add configurable JSONDecoder to Deserializer protocol for date decoding strategy

  • Extend RestOptions with dateDecodingStrategy property defaulting to ISO-8601

  • Apply date decoding strategy from RestOptions to deserializer's JSON decoder

  • Remove platform-specific logging bootstrap code from test setup


Diagram Walkthrough

flowchart LR
  RestOptions["RestOptions<br/>dateDecodingStrategy"] -- "applies strategy" --> Deserializer["Deserializer<br/>jsonDecoder"]
  Deserializer -- "decodes dates" --> JSONDecoder["JSONDecoder<br/>with strategy"]
Loading

File Walkthrough

Relevant files
Enhancement
Deserializer.swift
Add configurable JSONDecoder to deserializers                       

Sources/SwiftRestRequests/Deserializer.swift

  • Add optional jsonDecoder property to Deserializer protocol
  • Implement jsonDecoder in DecodableDeserializer with default
    JSONDecoder() instance
  • Set jsonDecoder to nil in VoidDeserializer and DataDeserializer
  • Update deserialization to use instance decoder instead of creating new
    one
+9/-1     
RestApiCaller.swift
Apply date decoding strategy to deserializer                         

Sources/SwiftRestRequests/RestApiCaller.swift

  • Apply dateDecodingStrategy from RestOptions to deserializer's JSON
    decoder before making request
  • Add comment explaining date decoding strategy configuration
+3/-0     
RestOptions.swift
Add configurable date decoding strategy option                     

Sources/SwiftRestRequests/RestOptions.swift

  • Add dateDecodingStrategy property to RestOptions struct
  • Default to JSONDecoder.DateDecodingStrategy.iso8601 for consistent
    date handling
  • Include documentation comment explaining ISO-8601 RFC 3339 format
+3/-0     
Miscellaneous
AbstractRestApiCallerTests.swift
Remove platform-specific logging bootstrap code                   

Tests/SwiftRestRequestsTests/AbstractRestApiCallerTests.swift

  • Remove platform-specific conditional compilation block for logging
    bootstrap
  • Simplify test setup by removing #if os(Linux) and OSLogHandler
    initialization
+0/-7     

@qodo-code-review

qodo-code-review Bot commented Nov 11, 2025

Copy link
Copy Markdown

PR Compliance Guide 🔍

(Compliance updated until commit 2fe4c23)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing auditing: The new behavior that sets a date decoding strategy is not accompanied by any audit
logging of critical actions, but it is unclear whether such logging is handled elsewhere
in the codebase.

Referred Code
// for deserializer with decoder set date decoding strategy
responseDeserializer.jsonDecoder?.dateDecodingStrategy = options.dateDecodingStrategy

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Force unwrap risk: The deserializer force-unwraps jsonDecoder during decode which may crash if nil and lacks
contextual error handling.

Referred Code
public func deserialize(_ data: Data) throws -> T {
    return try jsonDecoder!.decode(T.self, from: data)
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Uncontrolled errors: Errors thrown by JSON decoding are passed through without sanitization, and it's
unclear if user-facing layers mask internal details.

Referred Code
public func deserialize(_ data: Data) throws -> T {
    return try jsonDecoder!.decode(T.self, from: data)
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Verbose test logs: Test setup enables trace-level logging which may risk exposing sensitive data during tests
if payloads include PII, though actual log contents are not shown here.

Referred Code
Logger.SwiftRestRequests.security.logLevel = .trace
Logger.SwiftRestRequests.interceptor.logLevel = .trace
Logger.SwiftRestRequests.apiCaller.logLevel = .trace

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input validation gap: The code applies a date decoding strategy to external response data without visible
validation or safeguards against malformed content, though such validation may exist
elsewhere.

Referred Code
// for deserializer with decoder set date decoding strategy
responseDeserializer.jsonDecoder?.dateDecodingStrategy = options.dateDecodingStrategy

let (data, httpResponse) = try await dataTask(relativePath: relativePath, httpMethod: httpMethod.rawValue, accept: responseDeserializer.acceptHeader, payload: payload, options: options)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit 2fe4c23
Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing Auditing: The new behavior of configuring the deserializer’s date decoding strategy is not
accompanied by any audit logging of critical actions, but it is unclear whether such
logging is handled elsewhere in the system.

Referred Code
// for deserializer with decoder set date decoding strategy
responseDeserializer.jsonDecoder?.dateDecodingStrategy = options.dateDecodingStrategy

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Optional Force Use: The code sets dateDecodingStrategy on an optional decoder without verifying decoding
success paths or handling decode failures, relying on external error handling not shown in
the diff.

Referred Code
// for deserializer with decoder set date decoding strategy
responseDeserializer.jsonDecoder?.dateDecodingStrategy = options.dateDecodingStrategy

let (data, httpResponse) = try await dataTask(relativePath: relativePath, httpMethod: httpMethod.rawValue, accept: responseDeserializer.acceptHeader, payload: payload, options: options)

Learn more about managing compliance generic rules or creating your own custom rules

@qodo-code-review

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
State mutation introduces a race condition

The mutation of dateDecodingStrategy on a shared jsonDecoder in RestApiCaller
creates a race condition for concurrent requests. To fix this, avoid mutation
and instead create a new, properly configured JSONDecoder for each
deserialization operation.

Examples:

Sources/SwiftRestRequests/RestApiCaller.swift [276]
        responseDeserializer.jsonDecoder?.dateDecodingStrategy = options.dateDecodingStrategy
Sources/SwiftRestRequests/Deserializer.swift [56]
        return try jsonDecoder!.decode(T.self, from: data)

Solution Walkthrough:

Before:

// File: Sources/SwiftRestRequests/RestApiCaller.swift

private func makeCall<T: Deserializer>(..., responseDeserializer: T, options: RestOptions) async throws -> ... {
    
    // This mutates a shared object, causing a race condition
    responseDeserializer.jsonDecoder?.dateDecodingStrategy = options.dateDecodingStrategy
    
    let (data, httpResponse) = try await dataTask(...)
    
    // ...
    
    let result = try responseDeserializer.deserialize(data)
    return (result, httpStatus)
}

// File: Sources/SwiftRestRequests/Deserializer.swift

public func deserialize(_ data: Data) throws -> T {
    // Uses the potentially mutated decoder
    return try jsonDecoder!.decode(T.self, from: data)
}

After:

// File: Sources/SwiftRestRequests/RestApiCaller.swift

private func makeCall<T: Deserializer>(..., responseDeserializer: T, options: RestOptions) async throws -> ... {
    
    let (data, httpResponse) = try await dataTask(...)
    
    // ...
    
    // Pass options to the deserializer to handle decoding locally
    let result = try responseDeserializer.deserialize(data, options: options)
    return (result, httpStatus)
}

// File: Sources/SwiftRestRequests/Deserializer.swift

// Signature changed to accept options
public func deserialize(_ data: Data, options: RestOptions) throws -> T {
    let decoder = JSONDecoder()
    decoder.dateDecodingStrategy = options.dateDecodingStrategy
    return try decoder.decode(T.self, from: data)
}
Suggestion importance[1-10]: 10

__

Why: This suggestion correctly identifies a critical race condition bug introduced by mutating a shared JSONDecoder instance in a concurrent context, which could lead to incorrect data deserialization.

High
Possible issue
Avoid force-unwrapping an optional property

Replace the force-unwrapped optional jsonDecoder with a guard statement to
safely unwrap it and throw an error if it is nil, preventing a potential runtime
crash.

Sources/SwiftRestRequests/Deserializer.swift [55-57]

 public func deserialize(_ data: Data) throws -> T {
-    return try jsonDecoder!.decode(T.self, from: data)
+    struct DeserializerError: Error, LocalizedError {
+        let errorDescription: String? = "JSONDecoder is not available for DecodableDeserializer."
+    }
+    guard let decoder = jsonDecoder else {
+        throw DeserializerError()
+    }
+    return try decoder.decode(T.self, from: data)
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential runtime crash from force-unwrapping a public optional property and proposes a robust solution using a guard statement, improving code safety.

Medium
  • More

@tkausch
tkausch merged commit 8d9face into main Nov 17, 2025
@tkausch
tkausch deleted the MakeDateEncodingAndDecodingConfigurable branch November 17, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant