feat(core): decode DOS-packed date/time fields on Fat12Entry#8
Open
evanofficial wants to merge 2 commits into
Open
feat(core): decode DOS-packed date/time fields on Fat12Entry#8evanofficial wants to merge 2 commits into
evanofficial wants to merge 2 commits into
Conversation
Add pure helpers that decode the raw DOS-packed 16-bit date/time integers carried on Fat12Entry (createdDate, modifiedDate, modifiedTime) into java.time types: - decodeFatDate(packed): LocalDate? — null for the 0 sentinel and any bit pattern that is not a valid calendar date - decodeFatTime(packed): LocalTime — 2-second resolution - Fat12Entry.createdDateOrNull() / modifiedDateOrNull() / modifiedDateTimeOrNull() KDoc cites the FAT bit layout. The raw Int fields on Fat12Entry are unchanged (additive only). Adds FatDateTimeTest with hand-computed fixtures (2026-06-18, the engine DEFAULT_DOS_DATE/TIME, year-1980 offset 0), a full round-trip over the representable time space, and edge cases (zero date -> null, invalid month -> null, even-seconds, high-bit masking). Fixes #2
|
Warning
|
| Layer / File(s) | Summary |
|---|---|
Decode functions and Fat12Entry accessors core/src/main/kotlin/com/ams/fat12ex/core/FatDateTime.kt |
decodeFatDate masks to 16 bits, returns null for 0x0000 and invalid dates, and decodeFatTime extracts FAT bitfields with 2-second resolution. Fat12Entry extensions decode created and modified dates, plus modified date-time with null handling for missing or invalid values. |
Test suite core/src/test/kotlin/com/ams/fat12ex/core/FatDateTimeTest.kt |
Covers decodeFatDate, decodeFatTime, and Fat12Entry accessor behavior for valid packed values, zero sentinels, invalid components, high-bit masking, and corrupted modified-time handling. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
🐇 I hopped through bits of date and time,
and turned old FAT fields into rhyme.
A nibble here, a shift there too,
now dates and clocks come shining through!
Thump-thump, the bunny approves ✨
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 36.84% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and concisely describes the main change: decoding DOS-packed FAT date/time fields on Fat12Entry. |
| Linked Issues check | ✅ Passed | The PR adds the requested pure decode helpers, additive entry accessors, KDoc, and tests for the specified edge cases. |
| Out of Scope Changes check | ✅ Passed | All changes are directly related to FAT date/time decoding and its tests; no unrelated scope is evident. |
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feat/fat-datetime-helpers
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/src/test/kotlin/com/ams/fat12ex/core/FatDateTimeTest.kt (1)
102-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for corrupt
modifiedTimevalues.The suite never exercises the branch that differentiates
decodeFatTime()frommodifiedDateTimeOrNull(): an invalid packed time should throw in the decoder but yieldnullin the entry accessor. A case likemodifiedTime = 0x001E(seconds-field 30 → 60s) would pin that contract down.Proposed test
`@Test` fun entryAccessors_zeroDateFieldsAreNull() { val entry = Fat12Entry( name = "EMPTY.TXT", shortName = "EMPTY TXT", isDirectory = false, size = 0, firstCluster = 0, attributes = 0, createdDate = 0, modifiedDate = 0, modifiedTime = 0, ) assertNull(entry.createdDateOrNull()) assertNull(entry.modifiedDateOrNull()) assertNull(entry.modifiedDateTimeOrNull()) } + + `@Test` + fun entryAccessors_invalidModifiedTimeIsNull() { + val entry = Fat12Entry( + name = "BROKEN.TXT", + shortName = "BROKEN TXT", + isDirectory = false, + size = 1, + firstCluster = 2, + attributes = 0x20, + createdDate = 0x5CD2, + modifiedDate = 0x58C1, + modifiedTime = 0x001E, + ) + + assertNull(entry.modifiedDateTimeOrNull()) + }🤖 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 `@core/src/test/kotlin/com/ams/fat12ex/core/FatDateTimeTest.kt` around lines 102 - 139, Add a test in FatDateTimeTest that covers a corrupt packed modifiedTime value and verifies the contract between decodeFatTime() and Fat12Entry.modifiedDateTimeOrNull(). Use a Fat12Entry with a valid date but an invalid modifiedTime such as 0x001E, assert that the low-level decoder rejects it, and assert that modifiedDateTimeOrNull() returns null for the same entry.
🤖 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.
Nitpick comments:
In `@core/src/test/kotlin/com/ams/fat12ex/core/FatDateTimeTest.kt`:
- Around line 102-139: Add a test in FatDateTimeTest that covers a corrupt
packed modifiedTime value and verifies the contract between decodeFatTime() and
Fat12Entry.modifiedDateTimeOrNull(). Use a Fat12Entry with a valid date but an
invalid modifiedTime such as 0x001E, assert that the low-level decoder rejects
it, and assert that modifiedDateTimeOrNull() returns null for the same entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3301914f-011e-4a13-b6e2-47bf918bd854
📒 Files selected for processing (2)
core/src/main/kotlin/com/ams/fat12ex/core/FatDateTime.ktcore/src/test/kotlin/com/ams/fat12ex/core/FatDateTimeTest.kt
Add the case that distinguishes decodeFatTime() from modifiedDateTimeOrNull(): a packed time of 0x001E (seconds-field 30 -> 60s) is rejected by the low-level decoder (DateTimeException) but yields null from the entry-level accessor. Pins down that contract. Addresses CodeRabbit review feedback on this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2.
What
Fat12Entryexposes three raw DOS-packed integers —createdDate,modifiedDate,modifiedTime— straight from the on-disk directory entry. This adds pure helpers to decode them intojava.timetypes, so callers no longer need to know the FAT bit-packing.Decisions worth a look
decodeFatDatereturnsnullfor the0x0000sentinel and for any bit pattern that isn't a valid calendar date (corrupt month/day) — directory listings shouldn't throw on a bad entry.decodeFatTimeis strict: a corrupt field (e.g. a seconds-field of 30/31 → 60/62s) throwsDateTimeException, documented in the KDoc. The compositemodifiedDateTimeOrNull()swallows that intonullso the entry-level accessor never throws.…OrNull()suffix to match Kotlin convention and avoid colliding with the rawIntproperties. Open to other names.Changes
FatDateTime.kt(free functions +Fat12Entryextensions), KDoc citing the bit layout.Intfields onFat12Entryunchanged (additive only).FatDateTimeTest: hand-computed fixtures (2026-06-18, the engineDEFAULT_DOS_DATE/TIME, year-1980 offset 0), a full round-trip over the representable time space, and edge cases (zero date → null, invalid month → null, even-seconds, high-bit masking).Testing
./gradlew :core:testgreen.Summary by CodeRabbit
New Features
Tests