Skip to content

Conversation

@p123-stack
Copy link
Collaborator

Add Explicit Socket Type Configuration

Overview

This PR enables users to explicitly choose between Socket and Stream connection factories via driver configuration, while maintaining full backwards compatibility with auto-detection as the default behavior.

Problem

Previously, the connection factory type was determined solely by automatic detection via extension_loaded('sockets') at the singleton level. Users had no way to override this behavior, even if they wanted to force a specific connection type for performance, debugging, or compatibility reasons.

Solution

Added a new optional socketType configuration parameter that allows users to explicitly specify which connection factory to use:

  • 'sockets' - Force SocketConnectionFactory (faster, requires PHP sockets extension)
  • 'stream' - Force StreamConnectionFactory (always available, slightly slower)
  • null (default) - Auto-detect based on available extensions (existing behavior)

Changes

  1. DriverConfiguration - Added socketType property with getter/setter methods
  2. SystemWideConnectionFactory - Updated getInstance() to accept preferred socket type parameter
  3. BoltFactory - Updated create() to pass socket type to SystemWideConnectionFactory
  4. ConnectionPool - Updated create() to extract and pass socket type from configuration
  5. Tests - Added comprehensive unit tests covering all socket type scenarios

Usage Example

// Force Stream factory
$config = DriverConfiguration::default()->withSocketType('stream');
$driver = BoltDriver::create('neo4j://localhost', $config);

// Force Socket factory
$config = DriverConfiguration::default()->withSocketType('sockets');
$driver = BoltDriver::create('neo4j://localhost', $config);

// Auto-detect (default)
$config = DriverConfiguration::default();
$driver = BoltDriver::create('neo4j://localhost', $config);### Implementation Details

  • Config-first approach: Driver configuration takes precedence over auto-detection
  • Non-breaking: All new parameters are optional; existing code continues to work unchanged
  • Minimal footprint: ~40 lines of implementation code across 4 files
  • No refactoring: Only additive changes, no architectural modifications

Testing

New test coverage:

  • Socket type override to Stream
  • Socket type override to Socket (when extension available)
  • Configuration stores socket type correctly
  • BoltFactory accepts socket type parameter

Backwards Compatibility

Fully backwards compatible - all new parameters default to null which preserves existing auto-detection behavior.

Copy link
Collaborator

@transistive transistive left a comment

Choose a reason for hiding this comment

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

Great work! We just need a few small changes to get it out the door

public static function getInstance(?string $preferredSocket = null): SystemWideConnectionFactory
{
// If a specific socket type is requested, create a new instance without caching
if ($preferredSocket === 'sockets' && extension_loaded('sockets')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would throw an error if a socket is provided, but no socket extension is loaded. Maybe a DriverConfigurationError with a short message like: Cannot enforce socket extension usage if the extension is not enabled.

* @psalm-suppress InvalidNullableReturnType
*/
public static function getInstance(): SystemWideConnectionFactory
public static function getInstance(?string $preferredSocket = null): SystemWideConnectionFactory
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's use an enum instead of a string for better type safety and built-in documentation

Replace all string-based socket type parameters with the SocketType enum
to provide type safety and prevent configuration typos. Update related
factory classes and tests accordingly.
@transistive transistive merged commit 5e94937 into main Dec 17, 2025
14 checks passed
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.

3 participants