fix: encode RFC 3339 UTC offset as "Z" instead of "+0000"#27
Conversation
RFC3339Strategy used the single "Z" date-format pattern, which emits an
RFC 822 style offset ("+0000") for UTC. RFC 3339 requires the offset to be
"Z" or "+00:00", so strict parsers such as Go's time.RFC3339 (behind
gRPC-gateway) reject the value with HTTP 400.
Switch the pattern to "ZZZZZ", which emits "Z" for UTC and "±HH:MM" for
other offsets. Decoding is unaffected: "ZZZZZ" parses "-08:00", "-0800",
"+0000", and "Z" identically to the previous pattern, so there is no
backward-compatibility loss. Add an encoding regression test.
Ref: IOS-4838
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe RFC 3339 date formatter now uses a colon-separated timezone pattern. Tests cover UTC encoding, UTC and non-UTC offset decoding, and malformed input errors. ChangesRFC 3339 timezone handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Sources/KarrotCodableKit/BetterCodable/DateValue/Strategy/RFC3339Strategy.swift`:
- Line 23: Update the RFC3339 date strategy’s formatter handling so encoding
continues using the `ZZZZZ` format while decoding also accepts legacy `+0000`
offsets, using a separate decode formatter or fallback. Add coverage for
`+0000`, `+00:00`, and `Z` inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 175bff6c-757c-4fa4-9fb5-683e305900fe
📒 Files selected for processing (2)
Sources/KarrotCodableKit/BetterCodable/DateValue/Strategy/RFC3339Strategy.swiftTests/KarrotCodableKitTests/BetterCodable/DateValue/RFC3339StrategyEncodingTests.swift
The "ZZZZZ" pattern only changes encoding output; DateFormatter still parses every offset form the previous "Z" pattern accepted, including the colon-less RFC 822 style "+0000" (verified with isLenient at both false and true). Add explicit decode coverage to guard backward compatibility, and rename the file to RFC3339StrategyTests since it now covers both encode and decode. Addresses CodeRabbit review on PR #27.
f0b0de7 to
c8a4720
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Tests/KarrotCodableKitTests/BetterCodable/DateValue/RFC3339StrategyTests.swift (1)
13-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a non-UTC encoding test and an invalid-input error case.
The encoding test only verifies UTC produces
"Z". Since the format string changed from"Z"to"ZZZZZ"(which also affects non-UTC output), adding a test that encodes a date with a non-zero offset and asserts the"±HH:MM"form would close the coverage gap on the encoding side of the change.Additionally, per path instructions, error handling and edge case coverage is [HIGH] priority — a test asserting that
RFC3339Strategy.decodethrows for an invalid string would round out the suite.As per path instructions: "Validate error handling and edge case coverage" [HIGH].
✨ Suggested additional tests
`@Test`("UTC 날짜를 인코딩하면 오프셋을 표준 형식(Z)으로 만들어요") func encodesUTCDateWithStandardOffset() { // given let date = Date(timeIntervalSince1970: 1715082540) // 2024-05-07T11:49:00 UTC // when let result = RFC3339Strategy.encode(date) // then // RFC 3339 requires the UTC offset to be "Z" (or "+00:00"), not the colon-less "+0000". `#expect`(result == "2024-05-07T11:49:00Z") } + + `@Test`("UTC 외 날짜를 인코딩하면 오프셋을 ±HH:MM 형식으로 만들어요") + func encodesNonUTCDateWithColonOffset() { + // given + // 2024-05-07T11:49:00 UTC → in a non-UTC strategy the formatter is pinned to UTC, + // so this verifies the formatter's timezone (UTC) produces "Z". + // Note: RFC3339Strategy.dateFormatter.timeZone is always UTC, so encode + // will always produce "Z". This test documents that behavior. + let date = Date(timeIntervalSince1970: 1715082540) + + // when + let result = RFC3339Strategy.encode(date) + + // then + `#expect`(result == "2024-05-07T11:49:00Z") + } + + `@Test`("잘못된 날짜 문자열을 받으면 디코딩에 실패해요") + func throwsOnInvalidDateString() { + // given + let invalid = "not-a-date" + + // then + `#expect`(throws: DecodingError.self) { + try RFC3339Strategy.decode(invalid) + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/KarrotCodableKitTests/BetterCodable/DateValue/RFC3339StrategyTests.swift` around lines 13 - 59, Add coverage in RFC3339StrategyTests for non-UTC encoding by using a date with a non-zero timezone offset and asserting the output uses the ±HH:MM format. Also add an invalid-input case for RFC3339Strategy.decode and assert that decoding throws, while preserving the existing valid-input tests.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@Tests/KarrotCodableKitTests/BetterCodable/DateValue/RFC3339StrategyTests.swift`:
- Around line 13-59: Add coverage in RFC3339StrategyTests for non-UTC encoding
by using a date with a non-zero timezone offset and asserting the output uses
the ±HH:MM format. Also add an invalid-input case for RFC3339Strategy.decode and
assert that decoding throws, while preserving the existing valid-input tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8a98474a-5499-48a9-abf0-8aa4eb927580
📒 Files selected for processing (1)
Tests/KarrotCodableKitTests/BetterCodable/DateValue/RFC3339StrategyTests.swift
c8a4720 to
5727b7f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Tests/KarrotCodableKitTests/BetterCodable/DateValue/RFC3339StrategyTests.swift`:
- Around line 42-52: Add a negative test alongside
decodesNonUTCOffsetsRegardlessOfColon that passes a malformed date string to
RFC3339Strategy.decode and asserts the thrown error is
DecodingError.dataCorrupted. Keep the existing valid-offset coverage unchanged
and verify the strategy’s failure path specifically reports dataCorrupted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ed7f8597-a4fd-4274-9a52-fc503cc2668f
📒 Files selected for processing (1)
Tests/KarrotCodableKitTests/BetterCodable/DateValue/RFC3339StrategyTests.swift
Assert that decoding a string which does not match the RFC 3339 pattern throws a DecodingError (the strategy throws .dataCorrupted). Addresses a CodeRabbit review suggestion on PR #27.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Tests/KarrotCodableKitTests/BetterCodable/DateValue/RFC3339StrategyTests.swift`:
- Around line 54-60: Update throwsDecodingErrorForMalformedString to assert that
RFC3339Strategy.decode("not-a-valid-date") throws the specific
DecodingError.dataCorrupted case, rather than accepting any DecodingError.
Preserve the existing malformed-input scenario and failure expectation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 809eab00-707f-4ca9-b774-3d2bde959e95
📒 Files selected for processing (1)
Tests/KarrotCodableKitTests/BetterCodable/DateValue/RFC3339StrategyTests.swift
Restructure RFC3339StrategyTests into @Suite-grouped behaviors (encoding, decoding, decoding failures) with given/when/then bodies. Add real-world coverage that serializes @DATEvalue<RFC3339Strategy> via JSONEncoder and asserts a "Z" offset (not "+0000"), parameterized decode of every offset form, and a malformed-string case asserting DecodingError.dataCorrupted.
b4e846c to
18808b4
Compare
1 participant
Summary
RFC3339Strategyencoded UTC dates with a colon-less+0000offset, which violates RFC 3339. Strict parsers such as Go'stime.RFC3339(behind gRPC-gateway) reject+0000and respond with HTTP 400.Root cause: the date-format pattern used a single
Z("yyyy-MM-dd'T'HH:mm:ssZ"). InDateFormatter, a singleZis an RFC 822 style offset, so UTC is emitted as+0000. RFC 3339 requires the offset to beZor+00:00.Fix: switch the pattern to
ZZZZZ("yyyy-MM-dd'T'HH:mm:ssZZZZZ"), the ISO 8601 extended offset, which emitsZfor UTC and±HH:MMfor other offsets.Encoding behavior
Z(before)2024-05-07T11:49:00+0000ZZZZZ(after)2024-05-07T11:49:00ZBackward compatibility
Decoding is unaffected.
ZZZZZparses-08:00,-0800,+0000, andZidentically to the previous single-Zpattern (verified empirically and by the existing decode tests).RFC3339NanoStrategyalready usedZZZZZfor encoding; its decode-only single-Zfallback is left untouched.Tests
RFC3339StrategyEncodingTests, which asserts that a UTC date encodes to...Z. It fails before the fix (+0000) and passes after.Ref: IOS-4838
Summary by CodeRabbit
Zdesignator.Z,+00:00,+0000), non-UTC offsets (-08:00,-0800), and malformed date strings.