Bump database lib 7.3.4 - #14
anurag6569201 wants to merge 1 commit into
Conversation
Source PR: appwrite#13439 Source head: 84aec17
⛔ Shipwright · BlockedRecommendation: do not merge PR #14 · Tier
Findings (7)
Fireworks usage: 23,027 input · 1,004 output · 24,031 total tokens · $0.0057 · 17s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
| $this->create($dbForProject, $execution); | ||
| } else { | ||
| $dbForProject->upsertDocument('executions', $execution); | ||
| // upsertDocuments without a callback skips the sequence fetch that |
There was a problem hiding this comment.
Shipwright · CRITICAL
The change from upsertDocument to upsertDocuments silently changes return-value semantics.
Impact: The change from upsertDocument to upsertDocuments silently changes return-value semantics. The old path returned the upserted Document; the new path returns an int count and the code never reads it, so any caller or future code expecting the document will get a type change. More importantly, the comment claims skipping the sequence fetch is safe because 'nothing here reads the result back,' but this is an unverified…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| ->willReturnCallback(function (string $collection, Document $execution) use (&$upserted): Document { | ||
| $upserted = $execution; | ||
| return $execution; | ||
| ->method('upsertDocuments') |
There was a problem hiding this comment.
Shipwright · CRITICAL
The test callback signature for upsertDocuments includes optional parameters ($batchSize, $onNext, $onError) and asserts $onNext is null.
Impact: The test callback signature for upsertDocuments includes optional parameters ($batchSize, $onNext, $onError) and asserts $onNext is null. This test now encodes an implementation detail of the database adapter rather than the worker's behavior. If utopia-php/database 7.3.4 changes the default callback behavior or batch semantics, this test will pass while production silently changes write behavior. The assertion 'No…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| "ext-fileinfo": "*" | ||
| }, | ||
| "plugin-api-version": "2.9.0" | ||
| "plugin-api-version": "2.6.0" |
There was a problem hiding this comment.
Shipwright · CRITICAL
composer.lock downgrades plugin-api-version from 2.9.0 to 2.6.0 while simultaneously upgrading multiple packages.
Impact: composer.lock downgrades plugin-api-version from 2.9.0 to 2.6.0 while simultaneously upgrading multiple packages. This indicates the lock file was generated with an older or incompatible Composer version. A plugin-api downgrade can cause Composer to silently skip security-relevant plugins (e.g., audit, vulnerability scanning) or resolve dependencies differently in CI versus production, potentially shipping with unve…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| $this->create($dbForProject, $execution); | ||
| } else { | ||
| $dbForProject->upsertDocument('executions', $execution); | ||
| // upsertDocuments without a callback skips the sequence fetch that |
There was a problem hiding this comment.
Shipwright · HIGH
The comment 'upsertDocuments without a callback skips the sequence fetch that upsertDocument always pays for' assumes the reader knows what a 'sequence fetch' is and why it matters
Impact: The comment 'upsertDocuments without a callback skips the sequence fetch that upsertDocument always pays for' assumes the reader knows what a 'sequence fetch' is and why it matters. A new maintainer cannot verify this claim without reading the utopia-php/database internals. The optimization is undocumented in the worker's contract and will be cargo-culted or removed by someone who doesn't understand the performance…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| array $executions, | ||
| int $batchSize = 0, | ||
| ?callable $onNext = null, | ||
| ?callable $onError = null |
There was a problem hiding this comment.
Shipwright · HIGH
The test method testUpsertsCompletedExecutionWithLogs now asserts on the internal callback parameter $onNext being null.
Impact: The test method testUpsertsCompletedExecutionWithLogs now asserts on the internal callback parameter $onNext being null. This couples the worker test to the database library's private API shape. If the library adds a default callback or reorders parameters, the test breaks for reasons unrelated to the worker's behavior, creating maintenance noise and obscuring real regressions.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| { | ||
| "name": "appwrite/sdk-generator", | ||
| "version": "4.5.0", | ||
| "version": "4.7.6", |
There was a problem hiding this comment.
Shipwright · HIGH
The dependency bump for appwrite/sdk-generator jumps from 4.5.0 to 4.7.6 (two minor versions) in a single lock update.
Impact: The dependency bump for appwrite/sdk-generator jumps from 4.5.0 to 4.7.6 (two minor versions) in a single lock update. This is a dev dependency, but SDK generator output can be committed to the repository or used in release pipelines. A two-minor-version jump without an accompanying changelog or test evidence means generated SDK code could change behavior in ways not reviewed in this diff.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| { | ||
| "name": "utopia-php/database", | ||
| "version": "7.3.1", | ||
| "version": "7.3.4", |
There was a problem hiding this comment.
Shipwright · HIGH
Multiple utopia-php packages are upgraded in the same lock update (database 7.3.1->7.3.4, queue 1.5.1->1.5.2, usage 0.15.0->0.15.2, vcs 5.2.4->5.2.5).
Impact: Multiple utopia-php packages are upgraded in the same lock update (database 7.3.1->7.3.4, queue 1.5.1->1.5.2, usage 0.15.0->0.15.2, vcs 5.2.4->5.2.5). The database upgrade is directly relevant to the upsertDocuments change in this PR, but the queue and usage upgrades are unrelated and bundled together. Bundling unrelated dependency changes makes it impossible to bisect a production regression to a single…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
What does this PR do?
(Provide a description of what this PR does and why it's needed.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)
Related PRs and Issues
Checklist
Source merge-base:
a7e35dd376c704460e9239cd9dd158fc684e1b4aSource head:
84aec17d403afa54341288b829b5e7074840a7a6