Conversation
Bumps [guzzlehttp/guzzle](https://github.com/guzzle/guzzle) from 7.15.1 to 8.0.0. - [Release notes](https://github.com/guzzle/guzzle/releases) - [Changelog](https://github.com/guzzle/guzzle/blob/8.0/CHANGELOG.md) - [Commits](guzzle/guzzle@7.15.1...8.0.0) --- updated-dependencies: - dependency-name: guzzlehttp/guzzle dependency-version: 8.0.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
chore(release): Prepare v2.0.0
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
github-actions
Bot
added
documentation
📝 WalkthroughWalkthroughThis release updates the package to Guzzle 8 and native MongoDB logging, adds JSON/upload and response helpers, introduces builder-native HTTP fakes, refactors middleware handling, updates CI with MongoDB services, and expands documentation and test coverage. Changes2.0 runtime and release alignment
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #33 +/- ##
=============================================
+ Coverage 98.32% 98.72% +0.40%
- Complexity 729 940 +211
=============================================
Files 61 71 +10
Lines 1732 2198 +466
=============================================
+ Hits 1703 2170 +467
+ Misses 29 28 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
docs/00-architecture/04-modules-and-domains.md (1)
380-391: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument
schema_versionin the MongoDB event structure.The supplied
MongoDbLoggerimplementation writesschema_versionalongsidelogged_at, but the structure listed here omits it. Add the field so consumers can implement the documented schema contract correctly.🤖 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 `@docs/00-architecture/04-modules-and-domains.md` around lines 380 - 391, Update the MongoDB event “Document Structure” list to include the schema_version field alongside logged_at, matching the fields written by MongoDbLogger and documenting its schema-contract value.docs/00-architecture/05-data-flow.md (1)
587-593: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUpdate the remaining MongoDB configuration fields.
After Line [587] changes storage to native
MongoDB\Collectiondocuments, Lines [590-593] still describe aconnectionname. Documenturi,database, andcollection(or an injected native collection) instead, matching the 2.0 API.🤖 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 `@docs/00-architecture/05-data-flow.md` around lines 587 - 593, Update the MongoDB Configuration section to remove the connection-name field and document uri, database, and collection, including support for an injected native MongoDB.Collection, consistent with the 2.0 API. Keep the existing body truncation limits and header redaction rules entries.tests/Unit/Adapters/GuzzleHttpClientAdapterTest.php (2)
88-94: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
TransferExceptiontakes noRequestInterfaceargument here
GuzzleHttp\Exception\TransferExceptionuses the defaultRuntimeExceptionconstructor, so passing$requestas the second arg will throw aTypeErrorbefore the adapter is exercised.🐛 Proposed fix
- new \GuzzleHttp\Exception\TransferException('Transfer error', $request), + new \GuzzleHttp\Exception\TransferException('Transfer error'),🤖 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/Unit/Adapters/GuzzleHttpClientAdapterTest.php` around lines 88 - 94, Update test_throws_ClientException_on_non_bad_guzzle_exception to construct TransferException with only its supported message argument, removing the $request parameter; keep the request for the adapter invocation so the test reaches and verifies the intended exception-handling behavior.
1-1: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winDrop the
RequestInterfaceargument here and intests/Unit/Adapters/GuzzleHttpClientAdapterAsyncTest.php.GuzzleHttp\Exception\TransferExceptionuses the base exception constructor, so the second argument is treated as the exception code; passing$requesttriggers aTypeErrorbefore theClientExceptionpath is exercised.🤖 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/Unit/Adapters/GuzzleHttpClientAdapterTest.php` at line 1, Remove the RequestInterface argument from TransferException construction in GuzzleHttpClientAdapterTest and GuzzleHttpClientAdapterAsyncTest, leaving only arguments supported by the base exception constructor so the ClientException path is exercised without a TypeError.tests/Unit/Adapters/GuzzleHttpClientAdapterAsyncTest.php (1)
77-90: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPass only the message to
TransferExceptionThe second argument is the exception code, so this throws aTypeErrorbefore the async wrapper path runs. Change it tonew \GuzzleHttp\Exception\TransferException('Transfer error').🤖 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/Unit/Adapters/GuzzleHttpClientAdapterAsyncTest.php` around lines 77 - 90, Update test_wraps_generic_guzzle_exception_in_async_promise to construct TransferException with only the error message, removing the Request argument so the async rejection reaches the intended ClientException wrapper path.
🤖 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 `@composer.json`:
- Around line 39-40: Move the ext-mongodb entry from the suggest section into
Composer's require section, preserving its existing constraint and removing the
duplicate suggestion entry. Leave the other dependency declarations unchanged.
In `@docs/00-architecture/01-project-overview.md`:
- Around line 15-17: Update the **Name** field in the project overview to use
the canonical product name “JOOservices Client”, while retaining
“jooservices/client” only for the Composer package identifier.
In `@docs/00-architecture/04-modules-and-domains.md`:
- Around line 393-395: Update the logging contract near the confirmed MongoDB
logger behavior to avoid guaranteeing persistence when MongoDbLogger::log()
swallows writer failures. Replace the “one document is written for each logger
call” wording with an accurate statement that one write is attempted or that a
document is written only after successful persistence, while retaining the
sanitization, immutability, timeout, and failure-isolation guarantees.
In `@docs/01-getting-started/basic-concepts.md`:
- Line 210: Update the surrounding MongoDB documentation to remove legacy claims
that MongoDB-Laravel is a dependency or that Laravel application integration is
implemented. Ensure the 2.0+ guidance states native mongodb/mongodb:^2.0 usage,
the required ext-mongodb extension, and the absence of Laravel application
integration.
In `@docs/05-maintenance/BACKLOG-POST-2.0.md`:
- Line 41: Update the user-facing architecture page 04-modules-and-domains.md to
remove or correct outdated “Features NOT Implemented” entries for middleware
capabilities already present in the codebase and changelog, including rate
limiting, signing, metrics, and tracing. Keep the M7 backlog entry only for any
remaining cleanup after the page is corrected.
In `@README.md`:
- Around line 87-99: Update both fake-client examples in README.md to include
imports for ClientBuilder, RetryConfig, and Tests\TestCase so each snippet is
self-contained and works when copied independently.
- Around line 68-70: Update the README JSON client example to configure a base
URI via ClientBuilder::withBaseUri before calling postJson('/users', ...), or
change the request to an absolute URL so the example resolves correctly.
- Around line 213-220: Update the README examples around ClientBuilder and
MongoDbLogConfig to read getenv('MONGODB_URI') once, validate that it returns a
string before use, and reuse the validated URI in both configuration paths.
Preserve the existing example behavior while preventing false from being passed
to withMongoUriLogging() or MongoDbLogConfig::__construct().
In `@src/Client/ClientBuilder.php`:
- Around line 510-512: Preserve caller-defined resilience middleware order by
recording the insertion position when configuring rate limiting in
ClientBuilder’s rate-limit method (src/Client/ClientBuilder.php:510-512) and
retry in its retry method (src/Client/ClientBuilder.php:552-554), rather than
only storing configuration. Update the middleware materialization logic at
src/Client/ClientBuilder.php:816-830 to insert each deferred entry at its
recorded position, while retaining client-specific retry and resilience handling
where it intentionally differs from DTO behavior.
In `@src/Support/CurlExporter.php`:
- Around line 51-54: Replace the hand-rolled apostrophe escaping in
CurlExporter::quote() with PHP’s escapeshellarg() so values containing single
quotes produce valid shell arguments. Add a regression test covering a value
such as “it's” and verify the exported curl command preserves it correctly.
In `@src/Testing/InteractsWithHttpClient.php`:
- Around line 19-23: Update the tearDown method to wrap parent::tearDown() in a
finally block so ClientBuilder::clearFake() always executes, including when
parent teardown throws.
In `@tests/Feature/Logging/MongoDbLoggingTest.php`:
- Around line 124-126: Update the assertions in the MongoDB logging test to
correlate both checks to the same test run: retrieve one document using the
unique $marker and assert its response_payload equals 'YYYYYYYYYY'. Avoid
separate collection-wide queries that could match unrelated stale documents.
In `@tests/Unit/Client/ClientBuilderExtendedTest.php`:
- Around line 141-151: Update
test_withSleeper_overrides_the_default_sleeper_even_when_not_faked to exercise
the retry path: configure a mocked handler to return 503 followed by 200,
execute a request through the built client, and assert the injected
SleeperInterface mock receives exactly one sleep call. Keep the test focused on
verifying withSleeper() is honored during retry handling.
In `@tests/Unit/Logging/MongoDbLoggerFactoriesTest.php`:
- Around line 27-65: The unit tests in
tests/Unit/Logging/MongoDbLoggerFactoriesTest.php lines 27-65 and
tests/Unit/Client/ClientBuilderMongoLoggingTest.php lines 19-54 must stop
invoking real MongoDB writes against 127.0.0.1:1. Replace the MongoDB writer in
the affected factory and client-builder tests with a writer double while
preserving assertions for configuration wiring and swallowed write failures;
leave live insertOne coverage to integration tests.
In `@tests/Unit/Response/ResponseWrapperTest.php`:
- Around line 148-155: Update tests/Unit/Response/ResponseWrapperTest.php lines
148-155 to seek the response body to a nonzero offset before calling body(),
then assert the underlying stream tell() remains unchanged. Update
tests/Unit/Testing/RecordedRequestTest.php lines 34-40 similarly: seek the
request body before capture() and assert its tell() position is preserved.
In `@tests/Unit/Support/NullSleeperTest.php`:
- Around line 23-32: Remove the hrtime-based elapsedMs measurement and
assertLessThan wall-clock check from
test_sleep_returns_immediately_regardless_of_duration in NullSleeperTest. Keep
the NullSleeper::sleep(500) invocation so the interface behavior remains covered
by the existing invocation test.
---
Outside diff comments:
In `@docs/00-architecture/04-modules-and-domains.md`:
- Around line 380-391: Update the MongoDB event “Document Structure” list to
include the schema_version field alongside logged_at, matching the fields
written by MongoDbLogger and documenting its schema-contract value.
In `@docs/00-architecture/05-data-flow.md`:
- Around line 587-593: Update the MongoDB Configuration section to remove the
connection-name field and document uri, database, and collection, including
support for an injected native MongoDB.Collection, consistent with the 2.0 API.
Keep the existing body truncation limits and header redaction rules entries.
In `@tests/Unit/Adapters/GuzzleHttpClientAdapterAsyncTest.php`:
- Around line 77-90: Update test_wraps_generic_guzzle_exception_in_async_promise
to construct TransferException with only the error message, removing the Request
argument so the async rejection reaches the intended ClientException wrapper
path.
In `@tests/Unit/Adapters/GuzzleHttpClientAdapterTest.php`:
- Around line 88-94: Update
test_throws_ClientException_on_non_bad_guzzle_exception to construct
TransferException with only its supported message argument, removing the
$request parameter; keep the request for the adapter invocation so the test
reaches and verifies the intended exception-handling behavior.
- Line 1: Remove the RequestInterface argument from TransferException
construction in GuzzleHttpClientAdapterTest and
GuzzleHttpClientAdapterAsyncTest, leaving only arguments supported by the base
exception constructor so the ClientException path is exercised without a
TypeError.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99d5d9d7-39fb-48ec-9c5e-8dfd13b01482
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (75)
.github/workflows/ci.yml.github/workflows/release.ymlAGENTS.mdCHANGELOG.mdDockerfileREADME.mdUPGRADE-2.0.mdai/skills/http-logging.mdai/skills/mongodb-config.mdcomposer.jsondocs/00-architecture/01-project-overview.mddocs/00-architecture/03-tech-stack.mddocs/00-architecture/04-modules-and-domains.mddocs/00-architecture/05-data-flow.mddocs/00-architecture/business-context-and-goals.mddocs/01-getting-started/basic-concepts.mddocs/01-getting-started/installation.mddocs/02-user-guide/api-reference.mddocs/04-development/confidence-levels.mddocs/04-development/documentation-standards.mddocs/04-development/glossary.mddocs/04-development/templates-and-writing-rules.mddocs/05-maintenance/BACKLOG-POST-2.0.mddocs/05-maintenance/PR1-REVIEW-VERIFICATION.mddocs/05-maintenance/README.mdphpstan-tests-baseline.neonphpstan.neonsrc/Adapters/Guzzle/GuzzleHttpClientAdapter.phpsrc/Client/ClientBuilder.phpsrc/Client/HttpClient.phpsrc/Contracts/AsyncHttpClientInterface.phpsrc/Contracts/HttpClientInterface.phpsrc/Contracts/ResponseWrapperInterface.phpsrc/Contracts/WanIpProviderInterface.phpsrc/Logging/MongoDbLogConfig.phpsrc/Logging/MongoDbLogger.phpsrc/Middleware/MiddlewarePipeline.phpsrc/Models/Mongo/ClientRequestLog.phpsrc/Response/ResponseWrapper.phpsrc/Support/CurlExporter.phpsrc/Support/GuzzleOptionShape.phpsrc/Support/NullSleeper.phpsrc/Testing/AssertionException.phpsrc/Testing/FakeHttpClient.phpsrc/Testing/HttpFake.phpsrc/Testing/HttpFakeRegistry.phpsrc/Testing/InteractsWithHttpClient.phpsrc/Testing/RecordedRequest.phpsrc/Testing/TestResponse.phpsrc/Testing/TestResponseSequence.phptests/Feature/Logging/MongoDbLoggingTest.phptests/TestCase.phptests/Unit/Adapters/GuzzleHttpClientAdapterAsyncTest.phptests/Unit/Adapters/GuzzleHttpClientAdapterTest.phptests/Unit/Client/ClientBuilderExtendedTest.phptests/Unit/Client/ClientBuilderFakeTest.phptests/Unit/Client/ClientBuilderMongoLoggingTest.phptests/Unit/Client/HttpClientAsyncTest.phptests/Unit/Client/HttpClientJsonAndUploadTest.phptests/Unit/Logging/MongoDbLogConfigTest.phptests/Unit/Logging/MongoDbLoggerDefaultWriterTest.phptests/Unit/Logging/MongoDbLoggerFactoriesTest.phptests/Unit/Logging/MongoDbLoggerTest.phptests/Unit/Middleware/MiddlewarePipelineCoverageTest.phptests/Unit/Response/ResponseWrapperTest.phptests/Unit/Response/ResponseWrapperToDtoTest.phptests/Unit/Support/CurlExporterTest.phptests/Unit/Support/GuzzleOptionShapeTest.phptests/Unit/Support/NullSleeperTest.phptests/Unit/Testing/HttpFakeRegistryTest.phptests/Unit/Testing/HttpFakeTest.phptests/Unit/Testing/InteractsWithHttpClientTest.phptests/Unit/Testing/RecordedRequestTest.phptests/Unit/Testing/TestResponseSequenceTest.phptests/Unit/Testing/TestResponseTest.php
💤 Files with no reviewable changes (3)
- docs/05-maintenance/PR1-REVIEW-VERIFICATION.md
- src/Models/Mongo/ClientRequestLog.php
- docs/04-development/confidence-levels.md
| "jooservices/dto": "Optional DTO helpers for ResponseWrapper::toDto()", | ||
| "ext-mongodb": "Required to persist logs with MongoDbLogger (PECL mongodb); mongodb/mongodb hard-requires it at composer install time" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== composer.json ==\n'
cat -n composer.json | sed -n '1,140p'
printf '\n== Search for MongoDbLogger and mongodb usage ==\n'
rg -n "MongoDbLogger|ext-mongodb|mongodb/mongodb|MongoDB" .Repository: jooservices/client
Length of output: 24612
Move ext-mongodb into require.
mongodb/mongodb already makes the native extension mandatory at install time, so listing it under suggest is misleading. Keep the install-time dependency surface explicit by requiring ext-mongodb and removing it from suggest.
🤖 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 `@composer.json` around lines 39 - 40, Move the ext-mongodb entry from the
suggest section into Composer's require section, preserving its existing
constraint and removing the duplicate suggestion entry. Leave the other
dependency declarations unchanged.
| **Name**: jooservices/client | ||
| **Type**: PHP Library (Composer package) | ||
| **Version**: 1.3.0 | ||
| **Release target**: 2.0.0 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the canonical product name for the repository identity.
Line [15] should say JOOservices Client; reserve jooservices/client for the Composer package identifier.
As per coding guidelines, use the canonical product name JOOservices Client; use jooservices/client only as the Composer identifier.
🤖 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 `@docs/00-architecture/01-project-overview.md` around lines 15 - 17, Update the
**Name** field in the project overview to use the canonical product name
“JOOservices Client”, while retaining “jooservices/client” only for the Composer
package identifier.
Source: Coding guidelines
| **Status**: ✅ Confirmed (implementation exists) | ||
|
|
||
| **Unknown**: Production-readiness and intended use case | ||
| **Contract**: One sanitized immutable PSR-3 event document is written for each logger call. Driver timeouts are bounded and write failures are swallowed so logging cannot break transport. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Do not promise a write when writer failures are swallowed.
Because MongoDbLogger::log() catches writer exceptions, “one document is written for each logger call” is not guaranteed. Change this to “one write is attempted” or “one document is written on successful persistence.”
🤖 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 `@docs/00-architecture/04-modules-and-domains.md` around lines 393 - 395,
Update the logging contract near the confirmed MongoDB logger behavior to avoid
guaranteeing persistence when MongoDbLogger::log() swallows writer failures.
Replace the “one document is written for each logger call” wording with an
accurate statement that one write is attempted or that a document is written
only after successful persistence, while retaining the sanitization,
immutability, timeout, and failure-isolation guarantees.
| - MongoDB-Laravel (adds Laravel coupling) ⚠️ | ||
|
|
||
| **Risk (Medium)**: `mongodb/laravel-mongodb` dependency suggests Laravel coupling despite library being framework-agnostic. | ||
| **Compatibility note**: Mongo logging uses native `mongodb/mongodb`. Its `ext-mongodb` requirement applies at Composer install time to all consumers. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the stale Laravel MongoDB claims from this section.
The surrounding text still says MongoDB-Laravel is a dependency and that Laravel integration is implemented, which contradicts the native-driver-only 2.0 contract and this compatibility note. Remove or rewrite those legacy statements.
As per coding guidelines, for 2.0 and later documentation must describe native mongodb/mongodb:^2.0, required ext-mongodb, and the absence of Laravel application integration.
🤖 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 `@docs/01-getting-started/basic-concepts.md` at line 210, Update the
surrounding MongoDB documentation to remove legacy claims that MongoDB-Laravel
is a dependency or that Laravel application integration is implemented. Ensure
the 2.0+ guidance states native mongodb/mongodb:^2.0 usage, the required
ext-mongodb extension, and the absence of Laravel application integration.
Source: Coding guidelines
|
|
||
| | ID | Item | Notes | | ||
| |----|------|-------| | ||
| | M7 | Refresh stale “Features NOT Implemented” | `docs/00-architecture/04-modules-and-domains.md` still lists rate limit, signing, metrics, tracing, etc. as missing — outdated vs codebase/CHANGELOG. | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the stale architecture reference instead of deferring it.
This entry confirms that docs/00-architecture/04-modules-and-domains.md still tells users that implemented middleware is missing. Update that user-facing page now; retain M7 only for any residual cleanup.
As per coding guidelines, stale references should be fixed immediately rather than left as placeholder guidance.
🤖 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 `@docs/05-maintenance/BACKLOG-POST-2.0.md` at line 41, Update the user-facing
architecture page 04-modules-and-domains.md to remove or correct outdated
“Features NOT Implemented” entries for middleware capabilities already present
in the codebase and changelog, including rate limiting, signing, metrics, and
tracing. Keep the M7 backlog entry only for any remaining cleanup after the page
is corrected.
Source: Coding guidelines
| $collection = $mongo->selectCollection('jooservices_client', 'client_request_logs'); | ||
| self::assertNotNull($collection->findOne(['message' => ['$regex' => $marker]])); | ||
| self::assertNotNull($collection->findOne(['response_payload' => 'YYYYYYYYYY'])); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correlate the persisted assertions to one test run.
The marker query and payload query can match separate stale documents. Use a marker-specific collection or a single correlated query/document before asserting the payload.
🤖 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/Feature/Logging/MongoDbLoggingTest.php` around lines 124 - 126, Update
the assertions in the MongoDB logging test to correlate both checks to the same
test run: retrieve one document using the unique $marker and assert its
response_payload equals 'YYYYYYYYYY'. Avoid separate collection-wide queries
that could match unrelated stale documents.
| public function test_withSleeper_overrides_the_default_sleeper_even_when_not_faked(): void | ||
| { | ||
| $sleeper = Mockery::mock(\JOOservices\Client\Contracts\SleeperInterface::class); | ||
| $sleeper->shouldReceive('sleep')->never(); | ||
|
|
||
| ClientBuilder::create() | ||
| ->withSleeper($sleeper) | ||
| ->withRetry(new RetryConfig()) | ||
| ->build(); | ||
|
|
||
| $this->addToAssertionCount(1); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant test file and related symbols
git ls-files 'tests/Unit/Client/ClientBuilderExtendedTest.php' 'src/**/*.php' | sed -n '1,120p'
echo '--- outline: tests/Unit/Client/ClientBuilderExtendedTest.php ---'
ast-grep outline tests/Unit/Client/ClientBuilderExtendedTest.php --view expanded || true
echo '--- search for withSleeper / retry / sleeper usage ---'
rg -n "withSleeper|SleeperInterface|sleep\(|RetryConfig|withRetry|MockHandler|HandlerStack|withOption\('handler'" tests src -g '!vendor' -g '!node_modules'Repository: jooservices/client
Length of output: 37632
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- ClientBuilderExtendedTest excerpt ---'
sed -n '135,160p' tests/Unit/Client/ClientBuilderExtendedTest.php
echo '--- ClientBuilder sleeper/retry wiring ---'
sed -n '280,340p' src/Client/ClientBuilder.php
sed -n '540,610p' src/Client/ClientBuilder.php
sed -n '730,790p' src/Client/ClientBuilder.php
echo '--- RetryMiddleware ---'
sed -n '1,140p' src/Middleware/RetryMiddleware.php
echo '--- existing retry/sleeper tests ---'
sed -n '1,220p' tests/Feature/Resilience/RetryTest.php
sed -n '240,340p' tests/Unit/Middleware/RetryMiddlewareTest.phpRepository: jooservices/client
Length of output: 15833
Exercise the retry path here. build() never calls sleep(), so this still passes if withSleeper() is ignored. Send a 503 then 200 through a mocked handler and assert the injected sleeper is called once.
🤖 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/Unit/Client/ClientBuilderExtendedTest.php` around lines 141 - 151,
Update test_withSleeper_overrides_the_default_sleeper_even_when_not_faked to
exercise the retry path: configure a mocked handler to return 503 followed by
200, execute a request through the built client, and assert the injected
SleeperInterface mock receives exactly one sleep call. Keep the test focused on
verifying withSleeper() is honored during retry handling.
Source: Coding guidelines
| public function test_fromUri_writer_swallows_failures_when_mongo_is_unreachable(): void | ||
| { | ||
| $logger = MongoDbLogger::fromUri('mongodb://127.0.0.1:1', 'logs'); | ||
|
|
||
| $logger->info('unavailable writer is swallowed, not thrown'); | ||
| $this->addToAssertionCount(1); | ||
| } | ||
|
|
||
| public function test_fromConfig_wires_the_configs_database_and_collection(): void | ||
| { | ||
| $config = new MongoDbLogConfig('mongodb://127.0.0.1:1', 'logs', 'custom_collection'); | ||
| $logger = MongoDbLogger::fromConfig($config); | ||
|
|
||
| self::assertSame('logs', $logger->getConnection()); | ||
| self::assertSame('custom_collection', $logger->getCollection()); | ||
|
|
||
| // The writer targets a live Mongo instance and cannot be swapped via fromConfig(); | ||
| // this only proves an unreachable Mongo does not raise out of log(). | ||
| $logger->info('unreachable fromConfig writer is swallowed, not thrown'); | ||
| $this->addToAssertionCount(1); | ||
| } | ||
|
|
||
| public function test_fromCollection_derives_connection_and_collection_from_the_mongo_collection(): void | ||
| { | ||
| $collection = (new MongoClient('mongodb://127.0.0.1:1'))->selectCollection('logs_db', 'events'); | ||
| $logger = MongoDbLogger::fromCollection($collection); | ||
|
|
||
| self::assertSame('logs_db', $logger->getConnection()); | ||
| self::assertSame('events', $logger->getCollection()); | ||
| } | ||
|
|
||
| public function test_fromCollection_default_writer_calls_insertOne_and_swallows_its_failure(): void | ||
| { | ||
| $collection = (new MongoClient('mongodb://127.0.0.1:1'))->selectCollection('logs_db', 'events'); | ||
| $logger = MongoDbLogger::fromCollection($collection); | ||
|
|
||
| $logger->info('unreachable collection insert is swallowed, not thrown'); | ||
| $this->addToAssertionCount(1); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For mongodb/mongodb 2.0, what is MongoDB\Client's default serverSelectionTimeoutMS, and does Collection::insertOne wait for server selection before throwing when no server is reachable?
💡 Result:
For the MongoDB PHP driver, the default value for serverSelectionTimeoutMS is 30,000 milliseconds (30 seconds) [1][2][3]. Regarding the behavior of Collection::insertOne when no server is reachable: yes, it does wait for server selection to complete before throwing an exception [4][3]. When an operation is performed, the driver initiates a server selection process to identify a suitable server (e.g., a primary for write operations) [1][5]. The driver will block and attempt to find a reachable server until the serverSelectionTimeoutMS limit is reached [4][3]. If no suitable server is found within that time, the driver throws a MongoDB\Driver\Exception\ConnectionTimeoutException (or a similar exception indicating that server selection has expired) [4]. Note that by default, the driver may also be configured with serverSelectionTryOnce set to true, which instructs the driver to perform a single scan of the deployment; if that initial scan fails to find a server, the driver may raise an error without exhausting the full timeout period unless explicitly configured otherwise [2][4][3].
Citations:
- 1: https://github.com/mongodb/specifications/blob/44176c1b633819b5a070e05148a989d0c79d406d/source/server-selection/server-selection.md
- 2: https://www.mongodb.com/docs/manual/reference/connection-string-options/
- 3: https://www.php.net/manual/en/mongodb-driver-manager.construct.php
- 4: https://www.mongodb.com/docs/php-library/current/connect/connection-targets/
- 5: https://github.com/mongodb/specifications/blob/master/source/server-selection/server-selection.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'tests/Unit/Logging/MongoDbLoggerFactoriesTest.php' \
'tests/Unit/Client/ClientBuilderMongoLoggingTest.php' \
'src' | sed -n '1,200p'Repository: jooservices/client
Length of output: 4092
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline tests/Unit/Logging/MongoDbLoggerFactoriesTest.php --view expanded
printf '\n---\n'
ast-grep outline tests/Unit/Client/ClientBuilderMongoLoggingTest.php --view expanded
printf '\n---\n'
fd -a 'MongoDbLogger.php|ClientBuilder.php|MongoDbLogConfig.php' src testsRepository: jooservices/client
Length of output: 1362
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- MongoDbLogger.php ---\n'
sed -n '1,240p' src/Logging/MongoDbLogger.php
printf '\n--- ClientBuilder.php ---\n'
sed -n '1,260p' src/Client/ClientBuilder.php
printf '\n--- MongoDbLogConfig.php ---\n'
sed -n '1,200p' src/Logging/MongoDbLogConfig.php
printf '\n--- tests/Unit/Logging/MongoDbLoggerFactoriesTest.php ---\n'
sed -n '1,220p' tests/Unit/Logging/MongoDbLoggerFactoriesTest.php
printf '\n--- tests/Unit/Client/ClientBuilderMongoLoggingTest.php ---\n'
sed -n '1,220p' tests/Unit/Client/ClientBuilderMongoLoggingTest.phpRepository: jooservices/client
Length of output: 247
🏁 Script executed:
#!/bin/bash
set -euo pipefail
php -v >/dev/null 2>&1 && echo "php-present" || echo "php-missing"
composer --version >/dev/null 2>&1 && echo "composer-present" || echo "composer-missing"Repository: jooservices/client
Length of output: 185
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for path in [
Path("src/Logging/MongoDbLogger.php"),
Path("src/Client/ClientBuilder.php"),
Path("src/Logging/MongoDbLogConfig.php"),
Path("tests/Unit/Logging/MongoDbLoggerFactoriesTest.php"),
Path("tests/Unit/Client/ClientBuilderMongoLoggingTest.php"),
]:
print(f"\n### {path} ###")
text = path.read_text()
print(text[:12000])
PYRepository: jooservices/client
Length of output: 31086
Keep MongoDB writes out of the unit suite.
tests/Unit/Logging/MongoDbLoggerFactoriesTest.php#L27-L65 and tests/Unit/Client/ClientBuilderMongoLoggingTest.php#L19-L54 still exercise the real MongoDB insertOne() path against 127.0.0.1:1. Those calls wait on server selection before the logger swallows the failure, so the unit suite can stall for the driver timeout and become flaky. Use a writer double for unit coverage; keep the live MongoDB path in integration tests.
📍 Affects 2 files
tests/Unit/Logging/MongoDbLoggerFactoriesTest.php#L27-L65(this comment)tests/Unit/Client/ClientBuilderMongoLoggingTest.php#L19-L54
🤖 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/Unit/Logging/MongoDbLoggerFactoriesTest.php` around lines 27 - 65, The
unit tests in tests/Unit/Logging/MongoDbLoggerFactoriesTest.php lines 27-65 and
tests/Unit/Client/ClientBuilderMongoLoggingTest.php lines 19-54 must stop
invoking real MongoDB writes against 127.0.0.1:1. Replace the MongoDB writer in
the affected factory and client-builder tests with a writer double while
preserving assertions for configuration wiring and swallowed write failures;
leave live insertOne coverage to integration tests.
| public function test_body_preserves_the_stream_position_for_seekable_bodies(): void | ||
| { | ||
| $wrapper = new ResponseWrapper(new Response(200, [], 'abcdef')); | ||
|
|
||
| $wrapper->body(); | ||
| $wrapper->body(); | ||
|
|
||
| self::assertSame('abcdef', $wrapper->body()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the original stream cursor directly.
Both tests pass without proving cursor restoration: repeated reads still work if each implementation rewinds before reading, and casting a Guzzle stream to string can seek independently.
tests/Unit/Response/ResponseWrapperTest.php#L148-L155: seek the response body to a nonzero offset beforebody(), then asserttell()remains at that offset.tests/Unit/Testing/RecordedRequestTest.php#L34-L40: seek the request body to a nonzero offset beforecapture(), then asserttell()is unchanged.
📍 Affects 2 files
tests/Unit/Response/ResponseWrapperTest.php#L148-L155(this comment)tests/Unit/Testing/RecordedRequestTest.php#L34-L40
🤖 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/Unit/Response/ResponseWrapperTest.php` around lines 148 - 155, Update
tests/Unit/Response/ResponseWrapperTest.php lines 148-155 to seek the response
body to a nonzero offset before calling body(), then assert the underlying
stream tell() remains unchanged. Update
tests/Unit/Testing/RecordedRequestTest.php lines 34-40 similarly: seek the
request body before capture() and assert its tell() position is preserved.
| public function test_sleep_returns_immediately_regardless_of_duration(): void | ||
| { | ||
| $sleeper = new NullSleeper(); | ||
|
|
||
| $start = hrtime(true); | ||
| $sleeper->sleep(500); | ||
| $elapsedMs = (hrtime(true) - $start) / 1_000_000; | ||
|
|
||
| self::assertLessThan(50, $elapsedMs); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Remove the wall-clock assertion.
A paused or overloaded CI worker can exceed 50 ms even when NullSleeper::sleep() is a no-op, making this test flaky. The interface invocation test already covers the observable contract.
🤖 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/Unit/Support/NullSleeperTest.php` around lines 23 - 32, Remove the
hrtime-based elapsedMs measurement and assertLessThan wall-clock check from
test_sleep_returns_immediately_regardless_of_duration in NullSleeperTest. Keep
the NullSleeper::sleep(500) invocation so the interface behavior remains covered
by the existing invocation test.
2 participants
Release merge-back\n\nSynchronizes the published v2.0.0 release from master back into develop.\n\n- Release: https://github.com/jooservices/client/releases/tag/v2.0.0\n- Source release PR: #32\n- Tag: v2.0.0\n\nAll release validation, publication, and Packagist update jobs succeeded.
Summary by CodeRabbit