Skip to content

Conversation

@lohanidamodar
Copy link

@lohanidamodar lohanidamodar commented Dec 30, 2025

  • Enhance DSN to support partial DSN strings
  • Add relevant unit tests

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved DSN connection string parsing to better handle cases with missing or incomplete host information.
    • Enhanced validation logic to more comprehensively detect missing connection information across multiple components.
  • Tests

    • Expanded test coverage for edge cases and partial DSN component scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 30, 2025

Walkthrough

The changes introduce a new protected parseDsn() method in the DSN class that normalizes DSN parsing to handle cases where host information is missing. The method replaces direct parse_url() calls by using a placeholder when the host is absent. The constructor now uses this new method, and validation logic has been expanded to check for the presence of authority, path, or query instead of only enforcing a host requirement. The host defaults to an empty string when missing. Additionally, a comprehensive test method was added to verify parsing of partial DSNs with various missing components.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Enhance DSN constructor to handle partial DSN strings' directly and accurately describes the main change: support for partial DSN strings in the constructor, which is the primary objective of this changeset.
✨ Finishing touches
  • 📝 Generate docstrings

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.

Copy link

@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: 0

🧹 Nitpick comments (1)
src/DSN/DSN.php (1)

79-83: Consider documenting the placeholder fallback strategy.

The docblock accurately describes the method's purpose. For maintainability, consider briefly noting the placeholder injection strategy used when parse_url fails, so future maintainers understand why __utopia_dsn_placeholder__ exists in the code.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 42ee37a and cf567b3.

📒 Files selected for processing (2)
  • src/DSN/DSN.php
  • tests/DSN/DSNTest.php
🔇 Additional comments (3)
tests/DSN/DSNTest.php (1)

141-176: Good test coverage for partial DSN scenarios.

The test method covers the key partial DSN cases well: user-only, password-only, path-only, and query-only DSNs. The use of assertSame('', ...) for empty host validation ensures strict type checking.

Consider adding edge case tests for combined partial components (e.g., redis://user@/path or redis://:pass@?query=1) and ensuring malformed DSNs throw appropriately.

src/DSN/DSN.php (2)

56-77: Constructor validation logic is well-designed.

The new validation approach correctly allows partial DSNs by checking for the presence of authority, path, or query components rather than strictly requiring a host. The logic on lines 62-64 properly identifies authority presence by checking any of its subcomponents.

The change on line 73 where host defaults to empty string is a behavioral change — consumers expecting a non-empty host should be aware that getHost() may now return '' for partial DSNs.


84-128: Clever placeholder strategy for handling partial DSNs.

The implementation correctly addresses PHP's parse_url limitations with DSNs lacking a host. The logic flow is sound:

  1. Attempts normal parsing first (fast path)
  2. Falls back to regex extraction and placeholder injection
  3. Cleans up the placeholder from the result

The regex on line 92 correctly validates URI scheme syntax per RFC 3986.

One minor observation: rtrim($authority, '@') on line 108 would strip multiple trailing @ characters if present in malformed input, but such input is already invalid, so this is acceptable behavior.

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