Skip to content

feat: add StructureException handling for invalid datetime values in …#854

Merged
abnegate merged 2 commits intomainfrom
datetime-exception
Apr 10, 2026
Merged

feat: add StructureException handling for invalid datetime values in …#854
abnegate merged 2 commits intomainfrom
datetime-exception

Conversation

@ArnabChatterjee20k
Copy link
Copy Markdown
Contributor

@ArnabChatterjee20k ArnabChatterjee20k commented Apr 10, 2026

…Mongo adapter and corresponding tests

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling for datetime validation: invalid non-UTC datetime inputs now raise clear structure validation errors that identify the affected attribute and reason.
  • Tests

    • Added end-to-end tests covering invalid datetime values for created/updated timestamps to ensure structured errors are thrown and preserved.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 95761114-e61b-48f9-81d7-aa9dc4f45d03

📥 Commits

Reviewing files that changed from the base of the PR and between 615a530 and 81d3b85.

📒 Files selected for processing (1)
  • tests/e2e/Adapter/Scopes/DocumentTests.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/e2e/Adapter/Scopes/DocumentTests.php

📝 Walkthrough

Walkthrough

The MongoDB adapter now wraps DateTime conversion failures in castingBefore() by catching conversion \Throwable and rethrowing as StructureException. An e2e test was added to assert invalid $createdAt and $updatedAt values raise StructureException.

Changes

Cohort / File(s) Summary
MongoDB Adapter change
src/Database/Adapter/Mongo.php
Added StructureException import; updated castingBefore() to catch \Throwable when converting non-UTCDateTime values and rethrow as StructureException with an attribute-specific message.
End-to-end tests
tests/e2e/Adapter/Scopes/DocumentTests.php
Added testInvalidCreatedAndUpdatedAtThrowStructureException() to verify invalid datetime strings for $createdAt and $updatedAt throw StructureException; test runs conditionally and cleans up created collection.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • abnegate
  • fogelito

Poem

🐰 I hopped through timestamps, one by one,
Found dates that shattered in the sun,
I caught the crash and gave a shout,
Structure's warning sorts them out,
Now creation and updates stay well-run.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding StructureException handling for invalid datetime values in the Mongo adapter.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 datetime-exception

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 10, 2026

Greptile Summary

This PR wraps new \DateTime($node) in Mongo's castingBefore with a try/catch that re-throws as StructureException, and adds a shared-trait test for invalid $createdAt/$updatedAt values. The adapter change is a safe defensive guard, but it is only reachable when the structure validator is bypassed — in the normal call flow (createDocument, updateDocument, createDocuments), Structure::isValid() fires before castingBefore and throws first, leaving the new catch block dead code under default settings. The accompanying test confirms StructureException is raised, but it does so via the pre-existing validation layer, not through the new Mongo code path.

Confidence Score: 3/5

The adapter change is safe to merge on its own, but the test provides no evidence that the new castingBefore guard actually works.

The try/catch in castingBefore is dead code under normal (validation-enabled) operation, and the test never disables validation, so it exercises only the pre-existing Structure validator. Without a test that bypasses validation (e.g. via disableValidation()) and confirms the Mongo adapter itself raises StructureException, there is no proof the new code path works correctly. This is a P1 test-coverage gap on the changed code.

tests/e2e/Adapter/Scopes/DocumentTests.php — test does not reach the new Mongo castingBefore code

Important Files Changed

Filename Overview
src/Database/Adapter/Mongo.php Wraps new \DateTime($node) in castingBefore with try/catch and re-throws as StructureException; defensive but only reachable when validation is disabled (structure validation fires first).
tests/e2e/Adapter/Scopes/DocumentTests.php Adds testInvalidCreatedAndUpdatedAtThrowStructureException in the shared trait; the StructureException is produced by the pre-existing structure validator (runs before castingBefore), so the new Mongo adapter code path is never exercised.

Reviews (2): Last reviewed commit: "updated tests" | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/e2e/Adapter/Scopes/DocumentTests.php`:
- Line 5826: The test builds $id as "$createdAt-{$index}" which can produce ids
starting with '$' and trigger a StructureException before the intended datetime
validation for $createdAt/$updatedAt; change the id generation (the $id variable
where '$id' => $attribute . '-' . $index is set) to produce a valid id that
cannot start with '$' (e.g., prefix with a neutral string or use the index only)
so the datetime path is exercised, and update the failing assertion to also
check the thrown StructureException message contains the target attribute name
($createdAt or $updatedAt) to ensure the failure source is the datetime
validation rather than an invalid id.
🪄 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.yaml

Review profile: CHILL

Plan: Pro

Run ID: cb62c484-0d61-4ac7-9dd8-6eac75e22055

📥 Commits

Reviewing files that changed from the base of the PR and between fad8e6b and 615a530.

📒 Files selected for processing (2)
  • src/Database/Adapter/Mongo.php
  • tests/e2e/Adapter/Scopes/DocumentTests.php

@abnegate abnegate merged commit ee2d7d4 into main Apr 10, 2026
33 of 34 checks passed
@abnegate abnegate deleted the datetime-exception branch April 10, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants