Commit 5b74010bd24 for woocommerce
commit 5b74010bd2413cd1846e0e4382b7da533578789a
Author: Barry Hughes <3594411+barryhughes@users.noreply.github.com>
Date: Fri Nov 13 17:24:47 2026 -0700
Subscriptions engine: scope contract and meta writes by the owning extension (#70967)
* Scope the subscriptions engine contract lock and writes by owner
Contracts::lock(), update() and the contract meta writes now name the
expected owning extension (extension_slug). The lock acquire UPDATE matches
the owner, so a contract of another extension reads as missing: lock() and
an unlocked update() return null, unlocked add_meta() / update_meta() throw
and delete_meta() returns false, also while another operation holds the
foreign contract's lock. The internal lock and its holder read are scoped
the same way, and the due dispatcher locks with the owner it scanned.
A lock carries the owner it was taken for; a write that passes a lock with
another owner throws InvalidArgumentException. The meta key is now checked
at the facade together with the owner, so both errors are reported at once.
WOOSUBS-2080
* Name the expected owner on subscriptions engine cycle writes
Contracts::add_cycle() and transition_cycle() now require extension_slug in
their arguments, like update() and the meta writes, and refuse a lock taken
for another extension with InvalidArgumentException before any cycle is
added or moved. Every facade write names the expected owner.
WOOSUBS-2080
* Scope subscriptions engine plan meta writes by owner
Plans::add_meta(), update_meta() and delete_meta() now take the expected
owning extension as their second parameter and match it inside each write
statement: INSERT ... SELECT from the plan row, and UPDATE / DELETE joined
on the plan. A plan of another extension reads as missing, and add_meta() /
update_meta() on an unknown or foreign plan throw instead of writing an
orphan meta row. The plan and key are read only after a write that changed
nothing. The owner and the key are checked together at the facade.
WOOSUBS-2080
* Document owner-scoped writes in the subscriptions engine README
Describe the expected-owner rule, the lock invariant and the new lock and
meta signatures, and add the changelog entry.
WOOSUBS-2080
* Address review findings on owner-scoped engine writes
- Match the owner exactly (BINARY) in the contract lock acquire and holder
read and in every plan owner predicate (find, exists, update, meta), so a
slug differing only in case reads as another extension, as the PHP
comparisons already do. The plan update becomes a scoped UPDATE.
- Contract meta writes check the lock's contract before its owner.
- Tests: case variants for contracts and plans; argument errors before a
lock for another contract; the label refused after the owner in lock();
the stored owner right after a foreign update; cycle writes with the due
handler's lock; transition_cycle() reporting a missing owner with another
error; message fragments instead of escaped strings.
- Rename the test helper to get_extension_slug() and name the missing key
in the transition_cycle() @throws tag.
WOOSUBS-2080
* Address review findings on owner-scoped plan meta writes
- PlanRepository::exists() throws RuntimeException when its read fails, so
a failed classifying read in a unique add_meta() (or after a 0-row plan
update) is not reported as a missing plan.
- Failure tests for update_meta()'s classifying read and its add-branch
insert, and for the existence read of a unique add.
- The foreign unique-add test also asserts that nothing was written.
WOOSUBS-2080
* Address review findings on owner-scoped writes
- PlanRepository::update_fields() checks each value against its column
with wpdb::strip_invalid_text_for_column() before the hand-written
UPDATE, so an over-long name is refused as $wpdb->update() refused it,
instead of being truncated. A comment says why the query is
hand-written.
- The exact owner match is spelled CAST(%s AS BINARY), which MySQL
8.0.27+ does not flag as deprecated.
- Tests: update refuses an over-long name; a failed plan UPDATE and a
failed existence read after a 0-row update throw; a case-variant
delete_meta() leaves the real owner's value; a meta write with a lock
for another contract of another extension is refused for the contract
first.
WOOSUBS-2080
* Refuse over-long meta keys and correct the delete_meta docblock
The owner-scoped INSERT ... SELECT used for plan and contract meta skips the
length and charset check $wpdb->insert() makes, so an over-long meta key was
silently truncated. Check the key against the meta_key column first and throw
without writing. Reword the delete_meta() docblock: an old-shape call reads as
(owner = $key, key = $value) and is not safe.
* Share one column-fit check across the engine repositories
Move ContractRepository::assert_fits_column() into a final
Integration\Storage\ColumnFit class and rebuild it on
$wpdb->strip_invalid_text_for_column(), so a column that cannot be
checked refuses the write instead of passing it. The contract and plan
meta inserts and PlanRepository::update_fields() now use it in place of
their inline checks, so every hand-built write reports an over-long
value with the same message. Document the over-long key refusal on the
public meta methods, and cover multibyte keys and the fail-closed path.
diff --git a/packages/php/woocommerce-subscriptions-engine/README.md b/packages/php/woocommerce-subscriptions-engine/README.md
index 997ed95c6a4..1c1b4529fda 100644
--- a/packages/php/woocommerce-subscriptions-engine/README.md
+++ b/packages/php/woocommerce-subscriptions-engine/README.md
@@ -5,6 +5,7 @@ This package implements a general-purpose subscriptions engine for WooCommerce.
The engine records subscription facts (plans, contracts, billing cycles) and offers tools to work with them; extensions own every flow and decision. In particular:
- **Due dispatch:** an extension registers a contract due handler with its slug, and the engine calls it when one of its contracts reaches its next due moment, passing the dispatcher's contract lock. What happens then (renew, skip, clear the due moment) is the extension's decision.
-- **Contract lock:** one lock per contract on the facade. `Contracts::lock()` takes it for an operation (null when another operation holds it) and `Contracts::create()` creates every contract locked from birth (null when its optional creation key is taken); every write takes the lock as its last parameter. Every write runs under a lock: the caller's, or, for an update or meta write made without one, an internal lock the engine takes for that call. Cycle writes require the caller's lock. A write is refused with `ContractLockedException` while another operation holds the lock.
+- **Ownership:** every plan and contract records its owning extension (`extension_slug`). Reads work across extensions. Every write names the expected owner, the row's owning extension (also when acting for that extension, for example a listener writing with the owner's lock passes the contract's `extension_slug`), and never touches a row of another extension: such a row reads as missing to the write. This guards against writing another extension's row by mistake; it is not authorization. `Plans::update()`, `Contracts::update()`, `add_cycle()` and `transition_cycle()` take `extension_slug` in their arguments; `Contracts::lock()` and the contract and plan meta writes take it as their second parameter, for example `Contracts::lock( $contract_id, $extension_slug, $operation )` and `Plans::add_meta( $plan_id, $extension_slug, $key, $value )`.
+- **Contract lock:** one lock per contract on the facade. `Contracts::lock( $contract_id, $extension_slug, $operation )` takes it for an operation (null when another operation holds it or no contract of that extension has the id) and `Contracts::create()` creates every contract locked from birth (null when its optional creation key is taken); every write takes the lock as its last parameter. Every write runs under a lock: the caller's, or, for an update or meta write made without one, an internal lock the engine takes for that call. Every contract write is owner-scoped where its lock is taken, and no write path skips the lock. Cycle writes require the caller's lock. A write is refused with `ContractLockedException` while another operation holds the lock, and with `InvalidArgumentException` when it names an owner other than its lock's.
- **Cycle tools:** cycle appends are append-if-absent on the chain's positions, and status transitions are compare-and-set.
- **Payments broker:** `Api\Payments` lets an extension ask the order's gateway to take a scheduled payment off-session (`Payments::process_scheduled_payment( $order )`). Gateways opt in by adding the `subscriptions_engine` feature to their `$supports` and charging the order on `woocommerce_subscriptions_engine_scheduled_payment_{gateway_id}( $order )`, reporting the outcome through the order status. What a gateway can do beyond that (renewal payments, payment method changes, ...) it declares with the existing WooCommerce gateway features; extensions check the ones an operation needs with `Payments::gateway_supports( $gateway_id, $features )`, and the engine does not interpret them.
diff --git a/packages/php/woocommerce-subscriptions-engine/changelog/update-subscriptions-engine-scope-writes-by-owner b/packages/php/woocommerce-subscriptions-engine/changelog/update-subscriptions-engine-scope-writes-by-owner
new file mode 100644
index 00000000000..7426537c0c1
--- /dev/null
+++ b/packages/php/woocommerce-subscriptions-engine/changelog/update-subscriptions-engine-scope-writes-by-owner
@@ -0,0 +1,4 @@
+Significance: patch
+Type: dev
+Comment: Scope the unreleased subscriptions engine contract, cycle and meta writes by the expected owning extension; no changelog entry needed.
+
diff --git a/packages/php/woocommerce-subscriptions-engine/src/Api/ContractLock.php b/packages/php/woocommerce-subscriptions-engine/src/Api/ContractLock.php
index 2bb6b2b10ce..298b2327536 100644
--- a/packages/php/woocommerce-subscriptions-engine/src/Api/ContractLock.php
+++ b/packages/php/woocommerce-subscriptions-engine/src/Api/ContractLock.php
@@ -56,18 +56,19 @@ final class ContractLock implements Serializable {
* by any caller but the engine repository gets `_doing_it_wrong()`: it would bypass the
* lock lifetime.
*
- * @param int $contract_id Contract id.
- * @param string $operation Operation label.
- * @param string $token The stored token.
+ * @param int $contract_id Contract id.
+ * @param string $extension_slug The contract's owning extension, matched when the lock was taken.
+ * @param string $operation Operation label.
+ * @param string $token The stored token.
*/
- public function __construct( int $contract_id, string $operation, string $token ) {
+ public function __construct( int $contract_id, string $extension_slug, string $operation, string $token ) {
if ( ! self::is_called_by_repository() ) {
_doing_it_wrong( __METHOD__, esc_html__( 'Take a contract lock with Contracts::lock() or Contracts::create(); a lock built from a stored token bypasses the lock lifetime.', 'woocommerce' ), '0.0.1' );
}
$this->contract_id = $contract_id;
$this->operation = $operation;
- $this->state = new ContractLockState( $token );
+ $this->state = new ContractLockState( $token, $extension_slug );
}
/**
diff --git a/packages/php/woocommerce-subscriptions-engine/src/Api/Contracts.php b/packages/php/woocommerce-subscriptions-engine/src/Api/Contracts.php
index 894b23c4b83..6b17623e101 100644
--- a/packages/php/woocommerce-subscriptions-engine/src/Api/Contracts.php
+++ b/packages/php/woocommerce-subscriptions-engine/src/Api/Contracts.php
@@ -3,12 +3,16 @@
* Contracts - the engine's public contract facade (reads and writes).
*
* Extensions create and progressively build contracts from explicit argument arrays:
- * the engine records the facts it is given and decides nothing about them. Any caller may
- * read or write any contract (authorization is the caller's concern). Reads return read-only
- * views ({@see ContractView}, {@see CycleView}). No transactions, no cache, no hooks.
+ * the engine records the facts it is given and decides nothing about them. Reads work across
+ * extensions and return read-only views ({@see ContractView}, {@see CycleView}). Every write
+ * names the expected owner (`extension_slug`), the contract's owning extension, also when acting
+ * for that extension; a contract of another extension reads as missing to it. This guards
+ * against writing another extension's contract by mistake; it is not authorization (that is the
+ * caller's concern). No transactions, no cache, no hooks.
*
* One lock per contract ({@see self::lock()}): every write runs under a lock, the caller's or,
* for an update or meta write made without one, an internal one the engine takes for that call.
+ * Every contract write is owner-scoped where its lock is taken, and no write path skips the lock.
* Cycle writes require the caller's lock. A write is refused with {@see ContractLockedException}
* while another operation holds the lock.
*
@@ -43,11 +47,14 @@ defined( 'ABSPATH' ) || exit;
final class Contracts {
/**
- * Keys accepted by {@see self::update()}, as a key map.
+ * Keys accepted by {@see self::update()}, as a key map. The `extension_slug` sets the owning
+ * extension on create and scopes the write on update; it is never written on update. The
+ * other keys are the contract fields.
*
* @var array<string, true>
*/
private const CONTRACT_KEYS = array(
+ 'extension_slug' => true,
'customer_id' => true,
'currency' => true,
'selling_plan_id' => true,
@@ -72,14 +79,12 @@ final class Contracts {
);
/**
- * Keys accepted by {@see self::create()}: the update keys plus the create-only `extension_slug`
- * and `creation_key`.
+ * Keys accepted by {@see self::create()}: the update keys plus the create-only `creation_key`.
*
* @var array<string, true>
*/
private const CREATE_KEYS = array(
- 'extension_slug' => true,
- 'creation_key' => true,
+ 'creation_key' => true,
) + self::CONTRACT_KEYS;
/**
@@ -97,11 +102,13 @@ final class Contracts {
private const CREATE_OPERATION = 'create';
/**
- * Keys accepted by {@see self::add_cycle()}, as a key map.
+ * Keys accepted by {@see self::add_cycle()}, as a key map: the `extension_slug` that scopes the
+ * write (not a cycle field) and the cycle fields.
*
* @var array<string, true>
*/
private const CYCLE_KEYS = array(
+ 'extension_slug' => true,
'status' => true,
'kind' => true,
'sequence_no' => true,
@@ -114,15 +121,17 @@ final class Contracts {
);
/**
- * Keys accepted by {@see self::transition_cycle()}, as a key map.
+ * Keys accepted by {@see self::transition_cycle()}, as a key map: the `extension_slug` that
+ * scopes the write and the transition fields.
*
* @var array<string, true>
*/
private const TRANSITION_KEYS = array(
- 'from_status' => true,
- 'to_status' => true,
- 'order_id' => true,
- 'reason' => true,
+ 'extension_slug' => true,
+ 'from_status' => true,
+ 'to_status' => true,
+ 'order_id' => true,
+ 'reason' => true,
);
/**
@@ -130,14 +139,19 @@ final class Contracts {
* Pass the lock to every write of the operation and release it when done; an unreleased
* lock expires after {@see ContractLock::TTL_SECONDS}. An expired lock is taken over. A lock
* gives no fresh read snapshot: inside the caller's own transaction, read after taking it.
- *
- * @param int $contract_id Contract id.
- * @param string $operation Operation label (1-64 of `[a-z0-9_-]`), shown to callers that conflict.
- * @return ContractLock|null The held lock; null when another operation holds it or the contract does not exist.
- * @throws InvalidArgumentException If the label is invalid.
+ * Only a contract of `$extension_slug` is locked: one of another extension reads as missing.
+ *
+ * @param int $contract_id Contract id.
+ * @param string $extension_slug The expected owner (the contract's owning extension).
+ * @param string $operation Operation label (1-64 of `[a-z0-9_-]`), shown to callers that conflict.
+ * @return ContractLock|null The held lock; null when another operation holds it or no contract of that extension has the id.
+ * @throws ValidationException If `$extension_slug` is empty.
+ * @throws InvalidArgumentException If the label is invalid: refused after the slug, alone.
*/
- public static function lock( int $contract_id, string $operation ): ?ContractLock {
- return ( new ContractRepository() )->acquire_lock( $contract_id, $operation );
+ public static function lock( int $contract_id, string $extension_slug, string $operation ): ?ContractLock {
+ self::assert_scope( $extension_slug );
+
+ return ( new ContractRepository() )->acquire_lock( $contract_id, $extension_slug, $operation );
}
// phpcs:disable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber -- @throws also lists exceptions raised by the methods called.
@@ -195,41 +209,53 @@ final class Contracts {
// phpcs:disable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber -- @throws also lists exceptions raised by the methods called.
/**
- * Write the given fields (the keys of {@see self::create()} except `extension_slug`) to an
- * existing contract. Only the present keys are written; `items` and `addresses` replace the
- * whole set; `null` clears a nullable field (a money field to 0). Unknown keys raise a
- * `_doing_it_wrong()` notice and are ignored. The arguments are checked before the contract is
- * read, so invalid arguments throw even for a missing contract.
+ * Write the given fields to an existing contract of the given extension.
+ *
+ * Takes the keys of {@see self::create()} except `creation_key`. `extension_slug` is required,
+ * as on create, but here it scopes the write to that extension and is never written (a
+ * contract's extension never changes): a contract of another extension reads as missing and
+ * is never written, also while another operation holds its lock. Only the present keys are
+ * written; `items` and `addresses` replace the whole set; `null` clears a nullable field (a
+ * money field to 0). Unknown keys raise a `_doing_it_wrong()` notice and are ignored. The
+ * arguments are checked before the lock and the contract, so invalid arguments throw even
+ * for a missing contract.
*
* @param int $contract_id Contract id.
- * @param array<string, mixed> $args Fields to write.
+ * @param array<string, mixed> $args `extension_slug` (required, the expected owner) and the fields to write.
* @param ContractLock|null $lock The caller's lock on this contract, or null for an internal one.
* @return ContractView|null The row as read before the write plus the written fields; null
- * when the contract does not exist (also when it is deleted before the write).
- * @throws ValidationException If a value is invalid, with every invalid value reported.
- * @throws InvalidArgumentException If the lock is for another contract.
+ * when no contract of that extension has the id (also when it is
+ * deleted before the write).
+ * @throws ValidationException If `extension_slug` is missing or a value is invalid, with every invalid value reported.
+ * @throws InvalidArgumentException If the lock is for another contract or another extension.
* @throws ContractLockedException If another operation holds the lock, or `$lock` was lost.
*/
public static function update( int $contract_id, array $args, ?ContractLock $lock = null ): ?ContractView {
- if ( null !== $lock ) {
- ContractRepository::assert_lock_for( $lock, $contract_id );
- }
-
$filtered_args = ArgumentFilter::filter_known_keys( __METHOD__, $args, self::CONTRACT_KEYS );
- $validator = new ArgumentValidator();
- $changes = self::get_contract_fields( $validator, $filtered_args );
+ $validator = new ArgumentValidator();
+ $extension_slug = (string) $validator->get_non_empty_string( 'extension_slug', $filtered_args['extension_slug'] ?? null );
+ $changes = self::get_contract_fields( $validator, $filtered_args );
try {
$validator->throw_errors();
} catch ( InvalidInputException $e ) {
throw ValidationException::from_core( $e ); // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped -- from_core() escapes the message.
}
+ if ( null !== $lock ) {
+ ContractRepository::assert_lock_for( $lock, $contract_id );
+ ContractRepository::assert_lock_owner( $lock, $extension_slug );
+ }
+
$repository = new ContractRepository();
if ( array() === $changes ) {
$contract = $repository->find( $contract_id );
- return null === $contract ? null : ContractView::from_contract( $contract, true );
+ if ( null === $contract || $extension_slug !== $contract->get_extension_slug() ) {
+ return null;
+ }
+
+ return ContractView::from_contract( $contract, true );
}
if ( null !== $lock ) {
@@ -238,7 +264,7 @@ final class Contracts {
}
// The internal lock covers the read the write is validated against, not only the write.
- $internal_lock = $repository->acquire_internal_lock( $contract_id );
+ $internal_lock = $repository->acquire_internal_lock( $contract_id, $extension_slug );
if ( null === $internal_lock ) {
return null;
}
@@ -297,10 +323,12 @@ final class Contracts {
* unique positions: a taken position or count is a lost race and returns null. A storage
* failure throws a `RuntimeException` and writes nothing. The lock is the contract's
* existence check: the engine deletes a contract only to discard a failed create, before its
- * lock is handed out. Unknown keys raise a `_doing_it_wrong()` notice.
+ * lock is handed out. `extension_slug` names the expected owner, which must be the lock's;
+ * it is not a cycle field. Unknown keys raise a `_doing_it_wrong()` notice.
*
* @param int $contract_id Contract id.
- * @param array<string, mixed> $args Cycle fields: `starts_at_gmt`, `ends_at_gmt` and
+ * @param array<string, mixed> $args `extension_slug` (required, the expected owner) and the
+ * cycle fields: `starts_at_gmt`, `ends_at_gmt` and
* `currency` (required); `status` (registered, default
* `pending`); `kind` (default `billing`); `sequence_no`
* (null: the head's plus one); `count` (null: not
@@ -311,14 +339,15 @@ final class Contracts {
* @return CycleView|null The appended cycle, or null when the position or count is already taken.
* @throws ValidationException If a required key is missing or a value is invalid, with every
* invalid value reported (ill-typed arguments before the cycle is built).
- * @throws InvalidArgumentException If the lock is for another contract.
+ * @throws InvalidArgumentException If the lock is for another contract or another extension.
* @throws ContractLockedException If `$lock` was lost.
*/
public static function add_cycle( int $contract_id, array $args, ContractLock $lock ): ?CycleView {
$filtered_args = ArgumentFilter::filter_known_keys( __METHOD__, $args, self::CYCLE_KEYS );
- $validator = new ArgumentValidator();
- $cycle_args = array(
+ $validator = new ArgumentValidator();
+ $extension_slug = (string) $validator->get_non_empty_string( 'extension_slug', $filtered_args['extension_slug'] ?? null );
+ $cycle_args = array(
'contract_id' => $contract_id,
'kind' => $filtered_args['kind'] ?? null,
'sequence_no' => $validator->get_nullable_id( 'sequence_no', $filtered_args['sequence_no'] ?? null ),
@@ -336,6 +365,8 @@ final class Contracts {
throw ValidationException::from_core( $e ); // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped -- from_core() escapes the message.
}
+ ContractRepository::assert_lock_owner( $lock, $extension_slug );
+
try {
$cycle = Cycle::create( $cycle_args );
} catch ( InvalidInputException $e ) {
@@ -356,10 +387,13 @@ final class Contracts {
/**
* Move a cycle between statuses as a compare-and-set: of concurrent callers that read
* `from_status`, exactly one gets true. A same-status call returns true once the cycle
- * carries the status and the values. Unknown keys raise a `_doing_it_wrong()` notice.
+ * carries the status and the values. `extension_slug` names the expected owner, which must be
+ * the lock's; the cycle's contract is matched to the lock in the write itself, so no cycle is
+ * read first. Unknown keys raise a `_doing_it_wrong()` notice.
*
* @param int $cycle_id Cycle id.
- * @param array<string, mixed> $args `from_status` (required, the status the caller read),
+ * @param array<string, mixed> $args `extension_slug` (required, the expected owner),
+ * `from_status` (required, the status the caller read),
* `to_status` (required, registered), and `order_id`
* (?int) and `reason` (?string), written when present
* (null clears).
@@ -367,18 +401,20 @@ final class Contracts {
* {@see self::create()} or the due handler's context.
* @return bool True when this call made the transition, or for a same-status call when the
* cycle carries the status and the values.
- * @throws ValidationException If a status is missing or unregistered (field `to_status`), or a value is
+ * @throws ValidationException If a required key is missing, `to_status` is unregistered, or a value is
* invalid, with every invalid value reported.
- * @throws InvalidArgumentException If the cycle does not exist, or it belongs to another contract than the lock.
+ * @throws InvalidArgumentException If the cycle does not exist, it belongs to another contract than the lock, or the
+ * lock is for another extension.
* @throws ContractLockedException If `$lock` was lost.
*/
public static function transition_cycle( int $cycle_id, array $args, ContractLock $lock ): bool {
$filtered_args = ArgumentFilter::filter_known_keys( __METHOD__, $args, self::TRANSITION_KEYS );
- $validator = new ArgumentValidator();
- $from_status = $validator->get_non_empty_string( 'from_status', $filtered_args['from_status'] ?? null );
- $to_status = $validator->get_non_empty_string( 'to_status', $filtered_args['to_status'] ?? null );
- $changes = array();
+ $validator = new ArgumentValidator();
+ $extension_slug = (string) $validator->get_non_empty_string( 'extension_slug', $filtered_args['extension_slug'] ?? null );
+ $from_status = $validator->get_non_empty_string( 'from_status', $filtered_args['from_status'] ?? null );
+ $to_status = $validator->get_non_empty_string( 'to_status', $filtered_args['to_status'] ?? null );
+ $changes = array();
if ( array_key_exists( 'order_id', $filtered_args ) ) {
$changes['order_id'] = $validator->get_nullable_id( 'order_id', $filtered_args['order_id'] );
}
@@ -388,6 +424,7 @@ final class Contracts {
try {
$validator->throw_errors();
+ ContractRepository::assert_lock_owner( $lock, $extension_slug );
return ( new ContractRepository() )->transition_cycle( $cycle_id, (string) $from_status, (string) $to_status, $changes, $lock );
} catch ( InvalidInputException $e ) {
@@ -408,53 +445,71 @@ final class Contracts {
}
/**
- * Add a meta value to a contract, like `add_post_meta()`. A key may hold several values.
- *
- * @param int $contract_id Contract id.
- * @param string $key Meta key.
- * @param mixed $value Meta value; serialized when not scalar.
- * @param bool $unique When true, add nothing if the key already exists.
- * @param ContractLock|null $lock The caller's lock on this contract, or null for an internal one.
+ * Add a meta value to a contract of the given extension, like `add_post_meta()`. A key may
+ * hold several values. A contract of another extension reads as missing.
+ *
+ * @param int $contract_id Contract id.
+ * @param string $extension_slug The expected owner (the contract's owning extension).
+ * @param string $key Meta key.
+ * @param mixed $value Meta value; serialized when not scalar.
+ * @param bool $unique When true, add nothing if the key already exists.
+ * @param ContractLock|null $lock The caller's lock on this contract, or null for an internal one.
* @return int|null The meta row id; null when `$unique` and the key exists.
- * @throws InvalidArgumentException If `$key` is empty, the contract does not exist, or the lock is for another contract.
+ * @throws ValidationException If `$extension_slug` or `$key` is empty, with both reported.
+ * @throws InvalidArgumentException If no contract of that extension has the id, or the lock is for another contract or extension.
* @throws ContractLockedException If another operation holds the lock, or `$lock` was lost.
+ * @throws \RuntimeException If the key is longer than the meta_key column (255 characters).
*/
- public static function add_meta( int $contract_id, string $key, $value, bool $unique = false, ?ContractLock $lock = null ): ?int {
- return ( new ContractRepository() )->add_meta( $contract_id, $key, $value, $unique, $lock );
+ public static function add_meta( int $contract_id, string $extension_slug, string $key, $value, bool $unique = false, ?ContractLock $lock = null ): ?int {
+ self::assert_scope( $extension_slug, $key );
+
+ return ( new ContractRepository() )->add_meta( $contract_id, $extension_slug, $key, $value, $unique, $lock );
}
/**
- * Update a contract's meta values for `$key`, like `update_post_meta()`: adds the key
- * when absent, else rewrites every value, or only the values equal to `$prev_value`.
- *
- * @param int $contract_id Contract id.
- * @param string $key Meta key.
- * @param mixed $value New value; serialized when not scalar.
- * @param mixed $prev_value Only update values equal to this; null updates all. Any other
- * value ('' and false included) matches literally.
- * @param ContractLock|null $lock The caller's lock on this contract, or null for an internal one.
+ * Update the meta values for `$key` of a contract of the given extension, like
+ * `update_post_meta()`: adds the key when absent, else rewrites every value, or only the
+ * values equal to `$prev_value`. A contract of another extension reads as missing.
+ *
+ * @param int $contract_id Contract id.
+ * @param string $extension_slug The expected owner (the contract's owning extension).
+ * @param string $key Meta key.
+ * @param mixed $value New value; serialized when not scalar.
+ * @param mixed $prev_value Only update values equal to this; null updates all. Any other
+ * value ('' and false included) matches literally.
+ * @param ContractLock|null $lock The caller's lock on this contract, or null for an internal one.
* @return bool True when a value was added or changed; false when nothing changed.
- * @throws InvalidArgumentException If `$key` is empty, the contract does not exist, or the lock is for another contract.
+ * @throws ValidationException If `$extension_slug` or `$key` is empty, with both reported.
+ * @throws InvalidArgumentException If no contract of that extension has the id, or the lock is for another contract or extension.
* @throws ContractLockedException If another operation holds the lock, or `$lock` was lost.
+ * @throws \RuntimeException If the key is longer than the meta_key column (255 characters).
*/
- public static function update_meta( int $contract_id, string $key, $value, $prev_value = null, ?ContractLock $lock = null ): bool {
- return ( new ContractRepository() )->update_meta( $contract_id, $key, $value, $prev_value, $lock );
+ public static function update_meta( int $contract_id, string $extension_slug, string $key, $value, $prev_value = null, ?ContractLock $lock = null ): bool {
+ self::assert_scope( $extension_slug, $key );
+
+ return ( new ContractRepository() )->update_meta( $contract_id, $extension_slug, $key, $value, $prev_value, $lock );
}
/**
- * Delete a contract's meta values for `$key`, like `delete_post_meta()`.
- *
- * @param int $contract_id Contract id.
- * @param string $key Meta key.
- * @param mixed $value Only delete values equal to this; null deletes every value for the key.
- * Any other value ('' and false included) matches literally.
- * @param ContractLock|null $lock The caller's lock on this contract, or null for an internal one.
- * @return bool True when at least one value was deleted; false also for a missing contract.
- * @throws InvalidArgumentException If `$key` is empty, or the lock is for another contract.
+ * Delete the meta values for `$key` of a contract of the given extension, like
+ * `delete_post_meta()`. Unlike `delete_post_meta()`, the owner comes second: an old-shape
+ * call `delete_meta( $id, $key, $value )` reads as owner `$key`, key `$value`, and is not safe.
+ *
+ * @param int $contract_id Contract id.
+ * @param string $extension_slug The expected owner (the contract's owning extension).
+ * @param string $key Meta key.
+ * @param mixed $value Only delete values equal to this; null deletes every value for the key.
+ * Any other value ('' and false included) matches literally.
+ * @param ContractLock|null $lock The caller's lock on this contract, or null for an internal one.
+ * @return bool True when at least one value was deleted; false also when no contract of that extension has the id.
+ * @throws ValidationException If `$extension_slug` or `$key` is empty, with both reported.
+ * @throws InvalidArgumentException If the lock is for another contract or extension.
* @throws ContractLockedException If another operation holds the lock, or `$lock` was lost.
*/
- public static function delete_meta( int $contract_id, string $key, $value = null, ?ContractLock $lock = null ): bool {
- return ( new ContractRepository() )->delete_meta( $contract_id, $key, $value, $lock );
+ public static function delete_meta( int $contract_id, string $extension_slug, string $key, $value = null, ?ContractLock $lock = null ): bool {
+ self::assert_scope( $extension_slug, $key );
+
+ return ( new ContractRepository() )->delete_meta( $contract_id, $extension_slug, $key, $value, $lock );
}
/**
@@ -626,6 +681,27 @@ final class Contracts {
return self::cycle_views( ( new ContractRepository() )->find_cycle_history( $contract_id, Cycle::KIND_BILLING, $limit, $offset ) );
}
+ /**
+ * Check the positional scope arguments of a write: the expected owner and, for a meta write,
+ * the key; every error is reported at once.
+ *
+ * @param string $extension_slug The expected owner.
+ * @param string|null $key Meta key, or null for a write that takes none.
+ * @throws ValidationException If the owner or the key is empty.
+ */
+ private static function assert_scope( string $extension_slug, ?string $key = null ): void {
+ $validator = new ArgumentValidator();
+ $validator->get_non_empty_string( 'extension_slug', $extension_slug );
+ if ( null !== $key ) {
+ $validator->get_non_empty_string( 'key', $key );
+ }
+ try {
+ $validator->throw_errors();
+ } catch ( InvalidInputException $e ) {
+ throw ValidationException::from_core( $e ); // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped -- from_core() escapes the message.
+ }
+ }
+
/**
* Check the shape of the present contract fields with the collector and return them
* normalized, `items` and `addresses` without unknown keys (a refused field is left
diff --git a/packages/php/woocommerce-subscriptions-engine/src/Api/Plans.php b/packages/php/woocommerce-subscriptions-engine/src/Api/Plans.php
index 3c3f1999f0d..101348dace1 100644
--- a/packages/php/woocommerce-subscriptions-engine/src/Api/Plans.php
+++ b/packages/php/woocommerce-subscriptions-engine/src/Api/Plans.php
@@ -6,8 +6,10 @@
* the payloads it is given and interprets none of them. It checks integrity only (a
* non-empty name, a registered status, object-shaped policies), then lets the plan's
* owning extension validate the write through `woocommerce_subscriptions_engine_validate_plan`.
- * Any caller may read any plan; an update names the plan's owning extension and never
- * reaches a plan of another one (authorization is the caller's concern). Reads return
+ * Any caller may read any plan. Every write names the expected owner (`extension_slug`), the
+ * plan's owning extension, also when acting for that extension, and never reaches a plan of
+ * another one: such a plan reads as missing. This guards against mistakes; it is not
+ * authorization (that is the caller's concern). Reads return
* read-only {@see PlanView}s. The engine opens no transaction and keeps no cache.
*
* Plan policies are stored as given; the engine does not read them.
@@ -202,51 +204,65 @@ final class Plans {
// phpcs:enable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber
/**
- * Add a meta value to a plan, like `add_post_meta()`. A key may hold several values.
- * The plan is not looked up: meta for an unknown plan id is a caller error.
+ * Add a meta value to a plan of the given extension, like `add_post_meta()`. A key may hold
+ * several values. A plan of another extension reads as missing.
*
- * @param int $plan_id Plan id.
- * @param string $key Meta key.
- * @param mixed $value Meta value; serialized when not scalar.
- * @param bool $unique When true, add nothing if the key already exists. Advisory: checked
- * before the insert with no unique index, so concurrent adds can both write.
+ * @param int $plan_id Plan id.
+ * @param string $extension_slug The expected owner (the plan's owning extension).
+ * @param string $key Meta key.
+ * @param mixed $value Meta value; serialized when not scalar.
+ * @param bool $unique When true, add nothing if the key already exists. Advisory: no
+ * unique index, so concurrent adds can both write.
* @return int|null The meta row id; null when `$unique` and the key exists.
- * @throws InvalidArgumentException If `$key` is empty.
+ * @throws ValidationException If `$extension_slug` or `$key` is empty, with both reported.
+ * @throws InvalidArgumentException If no plan of that extension has the id.
+ * @throws RuntimeException If the key is longer than the meta_key column (255 characters).
*/
- public static function add_meta( int $plan_id, string $key, $value, bool $unique = false ): ?int {
- return ( new PlanRepository() )->add_meta( $plan_id, $key, $value, $unique );
+ public static function add_meta( int $plan_id, string $extension_slug, string $key, $value, bool $unique = false ): ?int {
+ self::assert_scope( $extension_slug, $key );
+
+ return ( new PlanRepository() )->add_meta( $plan_id, $extension_slug, $key, $value, $unique );
}
/**
- * Update a plan's meta values for `$key`, like `update_post_meta()`: adds the key
- * when absent, else rewrites every value, or only the values equal to `$prev_value`.
- * The absent-key check runs before the write with no unique index, so it is not a lock.
- * The plan is not looked up: meta for an unknown plan id is a caller error.
+ * Update the meta values for `$key` of a plan of the given extension, like
+ * `update_post_meta()`: adds the key when absent, else rewrites every value, or only the
+ * values equal to `$prev_value`. A plan of another extension reads as missing.
*
- * @param int $plan_id Plan id.
- * @param string $key Meta key.
- * @param mixed $value New value; serialized when not scalar.
- * @param mixed $prev_value Only update values equal to this; null updates all. Any other
- * value ('' and false included) matches literally.
+ * @param int $plan_id Plan id.
+ * @param string $extension_slug The expected owner (the plan's owning extension).
+ * @param string $key Meta key.
+ * @param mixed $value New value; serialized when not scalar.
+ * @param mixed $prev_value Only update values equal to this; null updates all. Any other
+ * value ('' and false included) matches literally.
* @return bool True when a value was added or changed; false when nothing changed.
- * @throws InvalidArgumentException If `$key` is empty.
+ * @throws ValidationException If `$extension_slug` or `$key` is empty, with both reported.
+ * @throws InvalidArgumentException If no plan of that extension has the id.
+ * @throws RuntimeException If the key is longer than the meta_key column (255 characters).
*/
- public static function update_meta( int $plan_id, string $key, $value, $prev_value = null ): bool {
- return ( new PlanRepository() )->update_meta( $plan_id, $key, $value, $prev_value );
+ public static function update_meta( int $plan_id, string $extension_slug, string $key, $value, $prev_value = null ): bool {
+ self::assert_scope( $extension_slug, $key );
+
+ return ( new PlanRepository() )->update_meta( $plan_id, $extension_slug, $key, $value, $prev_value );
}
/**
- * Delete a plan's meta values for `$key`, like `delete_post_meta()`.
+ * Delete the meta values for `$key` of a plan of the given extension, like
+ * `delete_post_meta()`. Unlike `delete_post_meta()`, the owner comes second: an old-shape
+ * call `delete_meta( $id, $key, $value )` reads as owner `$key`, key `$value`, and is not safe.
*
- * @param int $plan_id Plan id.
- * @param string $key Meta key.
- * @param mixed $value Only delete values equal to this; null deletes every value for the key.
- * Any other value ('' and false included) matches literally.
- * @return bool True when at least one value was deleted.
- * @throws InvalidArgumentException If `$key` is empty.
+ * @param int $plan_id Plan id.
+ * @param string $extension_slug The expected owner (the plan's owning extension).
+ * @param string $key Meta key.
+ * @param mixed $value Only delete values equal to this; null deletes every value for the key.
+ * Any other value ('' and false included) matches literally.
+ * @return bool True when at least one value was deleted; false also when no plan of that extension has the id.
+ * @throws ValidationException If `$extension_slug` or `$key` is empty, with both reported.
*/
- public static function delete_meta( int $plan_id, string $key, $value = null ): bool {
- return ( new PlanRepository() )->delete_meta( $plan_id, $key, $value );
+ public static function delete_meta( int $plan_id, string $extension_slug, string $key, $value = null ): bool {
+ self::assert_scope( $extension_slug, $key );
+
+ return ( new PlanRepository() )->delete_meta( $plan_id, $extension_slug, $key, $value );
}
/**
@@ -340,6 +356,25 @@ final class Plans {
);
}
+ /**
+ * Check the positional scope arguments of a meta write, the expected owner and the key,
+ * reporting every error at once.
+ *
+ * @param string $extension_slug The expected owner.
+ * @param string $key Meta key.
+ * @throws ValidationException If the owner or the key is empty.
+ */
+ private static function assert_scope( string $extension_slug, string $key ): void {
+ $validator = new ArgumentValidator();
+ $validator->get_non_empty_string( 'extension_slug', $extension_slug );
+ $validator->get_non_empty_string( 'key', $key );
+ try {
+ $validator->throw_errors();
+ } catch ( InvalidInputException $e ) {
+ throw ValidationException::from_core( $e ); // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped -- from_core() escapes the message.
+ }
+ }
+
/**
* Check the shape of the present plan fields with the collector and return them
* normalized (a refused field is left out, or null for a policy; the collector holds
diff --git a/packages/php/woocommerce-subscriptions-engine/src/Integration/Dispatch/DueDispatcher.php b/packages/php/woocommerce-subscriptions-engine/src/Integration/Dispatch/DueDispatcher.php
index 484b0340897..0306ed9a410 100644
--- a/packages/php/woocommerce-subscriptions-engine/src/Integration/Dispatch/DueDispatcher.php
+++ b/packages/php/woocommerce-subscriptions-engine/src/Integration/Dispatch/DueDispatcher.php
@@ -195,7 +195,7 @@ final class DueDispatcher {
$contract_id = $row['id'];
try {
- $lock = $this->contracts->acquire_lock( $contract_id, self::LOCK_OPERATION, array( 'next_payment_gmt' => $row['next_payment_gmt'] ) );
+ $lock = $this->contracts->acquire_lock( $contract_id, $row['extension_slug'], self::LOCK_OPERATION, array( 'next_payment_gmt' => $row['next_payment_gmt'] ) );
if ( null === $lock ) {
++$skipped;
continue;
diff --git a/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/ColumnFit.php b/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/ColumnFit.php
new file mode 100644
index 00000000000..03165142d22
--- /dev/null
+++ b/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/ColumnFit.php
@@ -0,0 +1,61 @@
+<?php
+/**
+ * ColumnFit - checks a string fits its column before a write, as `$wpdb->insert()` does.
+ *
+ * @package Automattic\WooCommerce\SubscriptionsEngine\Integration\Storage
+ */
+
+declare( strict_types=1 );
+
+namespace Automattic\WooCommerce\SubscriptionsEngine\Integration\Storage;
+
+use Automattic\WooCommerce\SubscriptionsEngine\Core\Support\Coercion;
+
+defined( 'ABSPATH' ) || exit;
+
+/**
+ * Column-fit check shared by the repositories.
+ */
+final class ColumnFit {
+
+ /**
+ * Refuse a string `$wpdb` would truncate or strip for its column: longer than the column
+ * (characters for CHAR/VARCHAR, bytes for TEXT types) or invalid for its charset. A column
+ * `$wpdb` cannot check is refused; a non-string column or a non-MySQL database is not checked.
+ *
+ * @param string $table Prefixed table name.
+ * @param string $column Column name.
+ * @param string $value Value about to be written.
+ * @throws \RuntimeException If the value does not fit the column, or the column cannot be checked.
+ */
+ public static function assert_fits_column( string $table, string $column, string $value ): void {
+ global $wpdb;
+
+ // `wpdb::query()` refuses invalid charset but not over-long values, so hand-built writes check here.
+ $checked = $wpdb->strip_invalid_text_for_column( $table, $column, $value );
+ if ( $checked === $value ) {
+ return;
+ }
+
+ if ( is_wp_error( $checked ) ) {
+ throw new \RuntimeException(
+ esc_html( sprintf( 'Failed to write %s: the value for "%s" could not be checked against the column (%s).', $table, $column, $checked->get_error_message() ) )
+ );
+ }
+
+ $length = $wpdb->get_col_length( $table, $column );
+ if ( is_array( $length ) ) {
+ $max = Coercion::coerce_int( $length['length'] ?? 0 );
+ $size = 'byte' === ( $length['type'] ?? '' ) ? strlen( $value ) : mb_strlen( $value, 'UTF-8' );
+ if ( $max > 0 && $size > $max ) {
+ throw new \RuntimeException(
+ esc_html( sprintf( 'Failed to write %s: the value for "%s" is longer than the column allows (%d).', $table, $column, $max ) )
+ );
+ }
+ }
+
+ throw new \RuntimeException(
+ esc_html( sprintf( 'Failed to write %s: the value for "%s" has characters the column cannot store.', $table, $column ) )
+ );
+ }
+}
diff --git a/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/ContractLockState.php b/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/ContractLockState.php
index 3d799b90220..8d500919535 100644
--- a/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/ContractLockState.php
+++ b/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/ContractLockState.php
@@ -14,7 +14,7 @@ use Automattic\WooCommerce\SubscriptionsEngine\Api\ContractLock;
defined( 'ABSPATH' ) || exit;
/**
- * Token, trust window and extend / release flags of one lock. Held by the lock and read by
+ * Token, owner, trust window and extend / release flags of one lock. Held by the lock and read by
* {@see ContractRepository} through {@see ContractLock::get_state()}; any other caller gets
* `_doing_it_wrong()`.
*
@@ -42,6 +42,13 @@ final class ContractLockState {
*/
private $token;
+ /**
+ * Owning extension of the locked contract, as matched when the lock was taken.
+ *
+ * @var string
+ */
+ private $extension_slug;
+
/**
* Monotonic moment until which child writes trust the lock.
*
@@ -73,10 +80,12 @@ final class ContractLockState {
/**
* State of a lock just stamped with a full TTL.
*
- * @param string $token The stored token.
+ * @param string $token The stored token.
+ * @param string $extension_slug The locked contract's owning extension.
*/
- public function __construct( string $token ) {
- $this->token = $token;
+ public function __construct( string $token, string $extension_slug ) {
+ $this->token = $token;
+ $this->extension_slug = $extension_slug;
$this->stamp( ContractLock::TTL_SECONDS );
}
@@ -87,6 +96,14 @@ final class ContractLockState {
return $this->token;
}
+ /**
+ * The locked contract's owning extension. It cannot change while the lock is held: no
+ * contract write changes `extension_slug`.
+ */
+ public function get_extension_slug(): string {
+ return $this->extension_slug;
+ }
+
/**
* Record a database stamp that left `$remaining_seconds` until expiry (acquire, extend or
* verify): trust for the window, or less when the lock ends sooner, and not at all with a
diff --git a/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/ContractRepository.php b/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/ContractRepository.php
index 04e2f66a0de..9b74f059889 100644
--- a/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/ContractRepository.php
+++ b/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/ContractRepository.php
@@ -75,6 +75,12 @@ final class ContractRepository {
*/
private const HOLDS_LOCK = 'lock_token = %s AND locked_until_gmt > UTC_TIMESTAMP()';
+ /**
+ * SQL condition: the row's owner is exactly the bound slug. The binary cast makes the match
+ * case-sensitive, as the engine's PHP comparisons of the owner are.
+ */
+ private const OWNER_MATCH = 'extension_slug = CAST(%s AS BINARY)';
+
/**
* Contract columns a lock may be conditioned on ({@see self::acquire_lock()}), mapped to the
* column name the SQL uses.
@@ -110,7 +116,7 @@ final class ContractRepository {
return null;
}
- $lock = new ContractLock( (int) $contract->get_id(), $operation, $token );
+ $lock = new ContractLock( (int) $contract->get_id(), (string) $contract->get_extension_slug(), $operation, $token );
$this->insert_children_or_discard( $contract, $lock );
@@ -185,7 +191,7 @@ final class ContractRepository {
continue;
}
if ( is_string( $value ) ) {
- self::assert_fits_column( $table, $column, $value );
+ ColumnFit::assert_fits_column( $table, $column, $value );
}
$placeholders[] = '%s';
$values[] = (string) $value;
@@ -787,20 +793,22 @@ final class ContractRepository {
// phpcs:disable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber -- @throws also lists exceptions raised by the methods called.
/**
- * Take the contract's lock for `$operation`: one conditional UPDATE that lands only while
- * no live lock is held (an expired one is taken over) and every condition holds, stamped on
- * the database clock.
- *
- * @param int $contract_id Contract id.
- * @param string $operation Operation label, 1-64 of `[a-z0-9_-]`.
- * @param array<string, mixed> $conditions Contract column => the value the caller read: a
- * scalar must equal it, null requires NULL. Columns:
- * {@see self::LOCK_CONDITION_COLUMNS}.
- * @return ContractLock|null The lock, or null when the contract is locked, missing or a condition fails.
+ * Take the contract's lock for `$operation`: one conditional UPDATE that lands only on a contract
+ * of `$extension_slug`, while no live lock is held (an expired one is taken over) and every
+ * condition holds, stamped on the database clock. Every contract write is owner-scoped here.
+ *
+ * @param int $contract_id Contract id.
+ * @param string $extension_slug The expected owning extension.
+ * @param string $operation Operation label, 1-64 of `[a-z0-9_-]`.
+ * @param array<string, mixed> $conditions Contract column => the value the caller read: a
+ * scalar must equal it, null requires NULL. Columns:
+ * {@see self::LOCK_CONDITION_COLUMNS}.
+ * @return ContractLock|null The lock, or null when the contract is locked, missing, of another
+ * extension, or a condition fails.
* @throws \InvalidArgumentException If `$operation` is not a valid label, or a condition column or value is not accepted.
* @throws \RuntimeException If the write fails.
*/
- public function acquire_lock( int $contract_id, string $operation, array $conditions = array() ): ?ContractLock {
+ public function acquire_lock( int $contract_id, string $extension_slug, string $operation, array $conditions = array() ): ?ContractLock {
global $wpdb;
self::assert_operation( $operation );
@@ -808,11 +816,12 @@ final class ContractRepository {
$table = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_CONTRACTS );
$no_live_lock = self::NO_LIVE_LOCK;
+ $owner_match = self::OWNER_MATCH;
$token = self::new_token();
- $values = array_merge( array( $token, ContractLock::TTL_SECONDS, $operation, $contract_id ), $condition_values );
+ $values = array_merge( array( $token, ContractLock::TTL_SECONDS, $operation, $contract_id, $extension_slug ), $condition_values );
$sql = "UPDATE {$table} SET lock_token = %s, locked_until_gmt = DATE_ADD( UTC_TIMESTAMP(), INTERVAL %d SECOND ), lock_operation = %s
- WHERE id = %d{$condition_sql} AND {$no_live_lock}";
+ WHERE id = %d AND {$owner_match}{$condition_sql} AND {$no_live_lock}";
// Fixed SQL fragments only; every value is a placeholder.
// phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.NotPrepared
@@ -822,29 +831,36 @@ final class ContractRepository {
throw new \RuntimeException( esc_html( sprintf( 'Failed to lock contract %d: %s', $contract_id, $wpdb->last_error ) ) );
}
- return 1 === $updated ? new ContractLock( $contract_id, $operation, $token ) : null;
+ return 1 === $updated ? new ContractLock( $contract_id, $extension_slug, $operation, $token ) : null;
}
// phpcs:enable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber
/**
* Whether the contract exists and who holds its lock, read on the database clock. The
* operation is set only while the lock is live. Writes use it to shape errors only (it may
- * lag on a replica), never to decide that a write landed.
+ * lag on a replica), never to decide that a write landed. With `$extension_slug`, a contract
+ * of another extension reads as missing.
*
- * @param int $contract_id Contract id.
+ * @param int $contract_id Contract id.
+ * @param string|null $extension_slug The expected owning extension; null reads any contract.
* @return array{exists: bool, operation: ?string}
* @throws \RuntimeException If the read fails.
*/
- private function get_lock_holder( int $contract_id ): array {
+ private function get_lock_holder( int $contract_id, ?string $extension_slug = null ): array {
global $wpdb;
$table = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_CONTRACTS );
$lock_live = self::LOCK_LIVE;
$sql = "SELECT {$lock_live} AS live, lock_operation FROM {$table} WHERE id = %d";
+ $values = array( $contract_id );
+ if ( null !== $extension_slug ) {
+ $sql .= ' AND ' . self::OWNER_MATCH;
+ $values[] = $extension_slug;
+ }
// phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.NotPrepared
- $row = $wpdb->get_row( $wpdb->prepare( $sql, $contract_id ), ARRAY_A );
+ $row = $wpdb->get_row( $wpdb->prepare( $sql, $values ), ARRAY_A );
if ( '' !== $wpdb->last_error ) {
throw new \RuntimeException( esc_html( sprintf( 'Failed to read the lock on contract %d: %s', $contract_id, $wpdb->last_error ) ) );
}
@@ -994,21 +1010,24 @@ final class ContractRepository {
// phpcs:disable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber -- @throws also lists exceptions raised by the methods called.
/**
- * Take the internal lock for a write made without one: null when the contract does not exist.
+ * Take the internal lock for a write made without one: null when no contract of
+ * `$extension_slug` has the id (a contract of another extension reads as missing, also while
+ * another operation holds its lock).
*
* @internal Public for {@see \Automattic\WooCommerce\SubscriptionsEngine\Api\Contracts::update()}, which holds it across its read and write.
*
- * @param int $contract_id Contract id.
+ * @param int $contract_id Contract id.
+ * @param string $extension_slug The expected owning extension.
* @throws ContractLockedException If another operation holds the lock.
* @throws \RuntimeException If the lock write or the holder read fails.
*/
- public function acquire_internal_lock( int $contract_id ): ?ContractLock {
- $lock = $this->acquire_lock( $contract_id, self::INTERNAL_LOCK_OPERATION );
+ public function acquire_internal_lock( int $contract_id, string $extension_slug ): ?ContractLock {
+ $lock = $this->acquire_lock( $contract_id, $extension_slug, self::INTERNAL_LOCK_OPERATION );
if ( null !== $lock ) {
return $lock;
}
- $holder = $this->get_lock_holder( $contract_id );
+ $holder = $this->get_lock_holder( $contract_id, $extension_slug );
if ( ! $holder['exists'] ) {
return null;
}
@@ -1067,6 +1086,34 @@ final class ContractRepository {
}
}
+ /**
+ * Refuse a lock whose contract is not of the expected extension. In memory: the owner was
+ * matched when the lock was taken and no contract write changes it.
+ *
+ * @internal Public for {@see \Automattic\WooCommerce\SubscriptionsEngine\Api\Contracts}, which passes the caller's lock and expected owner.
+ *
+ * @param ContractLock $lock The lock.
+ * @param string $extension_slug The expected owning extension.
+ * @throws \InvalidArgumentException If the lock is for a contract of another extension.
+ */
+ public static function assert_lock_owner( ContractLock $lock, string $extension_slug ): void {
+ if ( $lock->get_state()->get_extension_slug() !== $extension_slug ) {
+ throw new \InvalidArgumentException( 'The lock is for a contract of another extension.' );
+ }
+ }
+
+ /**
+ * Refuse a meta write for a contract id no contract of `$extension_slug` has.
+ *
+ * @param int $contract_id Contract id.
+ * @param string $extension_slug The expected owning extension.
+ * @return never
+ * @throws \InvalidArgumentException Always.
+ */
+ private static function throw_not_found( int $contract_id, string $extension_slug ): void {
+ throw new \InvalidArgumentException( esc_html( sprintf( 'No contract of extension "%s" has id %d.', $extension_slug, $contract_id ) ) );
+ }
+
/**
* Throw a conflict carrying the contract's current lock holder, if any.
*
@@ -1291,31 +1338,31 @@ final class ContractRepository {
// phpcs:disable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber -- @throws also lists exceptions raised by the methods called.
/**
- * Add a meta row for a contract, like `add_post_meta()`, under `$lock` or, without one, under
- * an internal lock taken for the call. With `$unique` the insert checks the key is absent.
+ * Add a meta row for a contract of `$extension_slug`, like `add_post_meta()`, under `$lock` or,
+ * without one, under an internal lock taken for the call. With `$unique` the insert checks the
+ * key is absent.
*
- * @param int $contract_id Contract id.
- * @param string $key Meta key.
- * @param mixed $value Meta value; serialized when not scalar.
- * @param bool $unique When true, add nothing if the key already exists.
- * @param ContractLock|null $lock The caller's lock on this contract, or null.
+ * @param int $contract_id Contract id.
+ * @param string $extension_slug The expected owning extension.
+ * @param string $key Meta key.
+ * @param mixed $value Meta value; serialized when not scalar.
+ * @param bool $unique When true, add nothing if the key already exists.
+ * @param ContractLock|null $lock The caller's lock on this contract, or null.
* @return int|null The new meta row id, or null when `$unique` and the key exists.
- * @throws \InvalidArgumentException If `$key` is empty, the contract does not exist, or the lock is for another contract.
+ * @throws \InvalidArgumentException If no contract of that extension has the id, or the lock is for another contract or extension.
* @throws ContractLockedException If another operation holds the lock, or `$lock` was lost.
- * @throws \RuntimeException If the insert fails.
+ * @throws \RuntimeException If the key is longer than the meta_key column (255 characters), or the insert fails.
*/
- public function add_meta( int $contract_id, string $key, $value, bool $unique = false, ?ContractLock $lock = null ): ?int {
- if ( '' === $key ) {
- throw new \InvalidArgumentException( 'Contract meta key must not be empty.' );
- }
-
+ public function add_meta( int $contract_id, string $extension_slug, string $key, $value, bool $unique = false, ?ContractLock $lock = null ): ?int {
if ( null !== $lock ) {
+ self::assert_lock_for( $lock, $contract_id );
+ self::assert_lock_owner( $lock, $extension_slug );
return $this->add_meta_row( $contract_id, $key, $value, $unique, $lock );
}
- $internal_lock = $this->acquire_internal_lock( $contract_id );
+ $internal_lock = $this->acquire_internal_lock( $contract_id, $extension_slug );
if ( null === $internal_lock ) {
- throw new \InvalidArgumentException( esc_html( sprintf( 'Contract %d does not exist.', $contract_id ) ) );
+ self::throw_not_found( $contract_id, $extension_slug );
}
try {
return $this->add_meta_row( $contract_id, $key, $value, $unique, $internal_lock );
@@ -1361,14 +1408,16 @@ final class ContractRepository {
* @return int|null The new meta row id, or null when nothing was inserted.
* @throws \InvalidArgumentException If the lock is for another contract.
* @throws ContractLockedException If the lock was lost.
- * @throws \RuntimeException If the insert fails.
+ * @throws \RuntimeException If the key does not fit the meta_key column, or the insert fails.
*/
private function insert_meta_row( int $contract_id, string $key, $value, ContractLock $lock ): ?int {
global $wpdb;
+ $meta = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_CONTRACT_META );
+ ColumnFit::assert_fits_column( $meta, 'meta_key', $key );
+
$this->fence_child_write( $lock, $contract_id );
- $meta = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_CONTRACT_META );
$contracts = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_CONTRACTS );
// phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.InterpolatedNotPrepared
@@ -1410,33 +1459,32 @@ final class ContractRepository {
// phpcs:disable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber -- @throws also lists exceptions raised by the methods called.
/**
- * Update a contract's meta rows for `$key`, like `update_post_meta()`, under `$lock` or, without
- * one, under an internal lock taken for the call: rewrites every row for the key, or only the
- * rows holding `$prev_value`, and adds the key when it has no row.
- *
- * @param int $contract_id Contract id.
- * @param string $key Meta key.
- * @param mixed $value New value; serialized when not scalar.
- * @param mixed $prev_value Only update rows holding this value; null updates all rows for the key.
- * Any other value ('' and false included) matches literally.
- * @param ContractLock|null $lock The caller's lock on this contract, or null.
+ * Update the meta rows for `$key` of a contract of `$extension_slug`, like `update_post_meta()`,
+ * under `$lock` or, without one, under an internal lock taken for the call: rewrites every row
+ * for the key, or only the rows holding `$prev_value`, and adds the key when it has no row.
+ *
+ * @param int $contract_id Contract id.
+ * @param string $extension_slug The expected owning extension.
+ * @param string $key Meta key.
+ * @param mixed $value New value; serialized when not scalar.
+ * @param mixed $prev_value Only update rows holding this value; null updates all rows for the key.
+ * Any other value ('' and false included) matches literally.
+ * @param ContractLock|null $lock The caller's lock on this contract, or null.
* @return bool True when a row was added or at least one row changed.
- * @throws \InvalidArgumentException If `$key` is empty, the contract does not exist, or the lock is for another contract.
+ * @throws \InvalidArgumentException If no contract of that extension has the id, or the lock is for another contract or extension.
* @throws ContractLockedException If another operation holds the lock, or `$lock` was lost.
- * @throws \RuntimeException If a write fails.
+ * @throws \RuntimeException If the key is longer than the meta_key column (255 characters), or a write fails.
*/
- public function update_meta( int $contract_id, string $key, $value, $prev_value = null, ?ContractLock $lock = null ): bool {
- if ( '' === $key ) {
- throw new \InvalidArgumentException( 'Contract meta key must not be empty.' );
- }
-
+ public function update_meta( int $contract_id, string $extension_slug, string $key, $value, $prev_value = null, ?ContractLock $lock = null ): bool {
if ( null !== $lock ) {
+ self::assert_lock_for( $lock, $contract_id );
+ self::assert_lock_owner( $lock, $extension_slug );
return $this->update_meta_value( $contract_id, $key, $value, $prev_value, $lock );
}
- $internal_lock = $this->acquire_internal_lock( $contract_id );
+ $internal_lock = $this->acquire_internal_lock( $contract_id, $extension_slug );
if ( null === $internal_lock ) {
- throw new \InvalidArgumentException( esc_html( sprintf( 'Contract %d does not exist.', $contract_id ) ) );
+ self::throw_not_found( $contract_id, $extension_slug );
}
try {
return $this->update_meta_value( $contract_id, $key, $value, $prev_value, $internal_lock );
@@ -1515,30 +1563,29 @@ final class ContractRepository {
// phpcs:disable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber -- @throws also lists exceptions raised by the methods called.
/**
- * Delete a contract's meta rows for `$key`, like `delete_post_meta()`, under `$lock` or, without
- * one, under an internal lock taken for the call.
- *
- * @param int $contract_id Contract id.
- * @param string $key Meta key.
- * @param mixed $value Only delete rows holding this value; null deletes every row for the key.
- * Any other value ('' and false included) matches literally.
- * @param ContractLock|null $lock The caller's lock on this contract, or null.
- * @return bool True when at least one row was deleted (false for a missing contract).
- * @throws \InvalidArgumentException If `$key` is empty, or the lock is for another contract.
+ * Delete the meta rows for `$key` of a contract of `$extension_slug`, like `delete_post_meta()`,
+ * under `$lock` or, without one, under an internal lock taken for the call.
+ *
+ * @param int $contract_id Contract id.
+ * @param string $extension_slug The expected owning extension.
+ * @param string $key Meta key.
+ * @param mixed $value Only delete rows holding this value; null deletes every row for the key.
+ * Any other value ('' and false included) matches literally.
+ * @param ContractLock|null $lock The caller's lock on this contract, or null.
+ * @return bool True when at least one row was deleted (false when no contract of that extension has the id).
+ * @throws \InvalidArgumentException If the lock is for another contract or extension.
* @throws ContractLockedException If another operation holds the lock, or `$lock` was lost.
* @throws \RuntimeException If the delete fails.
*/
- public function delete_meta( int $contract_id, string $key, $value = null, ?ContractLock $lock = null ): bool {
- if ( '' === $key ) {
- throw new \InvalidArgumentException( 'Contract meta key must not be empty.' );
- }
-
+ public function delete_meta( int $contract_id, string $extension_slug, string $key, $value = null, ?ContractLock $lock = null ): bool {
if ( null !== $lock ) {
+ self::assert_lock_for( $lock, $contract_id );
+ self::assert_lock_owner( $lock, $extension_slug );
$deleted = $this->delete_meta_rows( $contract_id, $key, $value, $lock );
return $deleted > 0;
}
- $internal_lock = $this->acquire_internal_lock( $contract_id );
+ $internal_lock = $this->acquire_internal_lock( $contract_id, $extension_slug );
if ( null === $internal_lock ) {
return false;
}
@@ -1651,7 +1698,7 @@ final class ContractRepository {
// Checked first so an over-long value is refused with its column named.
foreach ( $columns as $column => $value ) {
if ( is_string( $value ) ) {
- self::assert_fits_column( $table, $column, $value );
+ ColumnFit::assert_fits_column( $table, $column, $value );
}
}
@@ -2154,7 +2201,7 @@ final class ContractRepository {
foreach ( $columns as $column => $value ) {
if ( is_string( $value ) ) {
- self::assert_fits_column( $table, $column, $value );
+ ColumnFit::assert_fits_column( $table, $column, $value );
}
}
@@ -2187,33 +2234,6 @@ final class ContractRepository {
return (int) $updated;
}
- /**
- * Refuse a string longer than its column, measured the way `$wpdb` measures it
- * (characters for CHAR/VARCHAR, bytes for TEXT/BLOB types). A column whose length
- * `$wpdb` cannot read (non-MySQL, or a non-string type such as DATETIME) is not checked.
- *
- * @param string $table Prefixed table name.
- * @param string $column Column name.
- * @param string $value Value about to be written.
- * @throws \RuntimeException If the value is longer than the column.
- */
- private static function assert_fits_column( string $table, string $column, string $value ): void {
- global $wpdb;
-
- $length = $wpdb->get_col_length( $table, $column );
- if ( ! is_array( $length ) ) {
- return;
- }
-
- $max = Coercion::coerce_int( $length['length'] ?? 0 );
- $size = 'byte' === ( $length['type'] ?? '' ) ? strlen( $value ) : mb_strlen( $value, 'UTF-8' );
- if ( $max > 0 && $size > $max ) {
- throw new \RuntimeException(
- esc_html( sprintf( 'Failed to write %s: the value for "%s" is longer than the column allows (%d).', $table, $column, $max ) )
- );
- }
- }
-
// phpcs:disable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber -- @throws also lists exceptions raised by the methods called.
/**
* Replace a contract's item rows: insert the new rows, then delete every other row of the
@@ -2288,7 +2308,7 @@ final class ContractRepository {
continue;
}
if ( is_string( $value ) ) {
- self::assert_fits_column( $table, $column, $value );
+ ColumnFit::assert_fits_column( $table, $column, $value );
}
$placeholders[] = '%s';
$values[] = (string) $value;
diff --git a/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/PlanRepository.php b/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/PlanRepository.php
index 196f44d6367..4532b50d478 100644
--- a/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/PlanRepository.php
+++ b/packages/php/woocommerce-subscriptions-engine/src/Integration/Storage/PlanRepository.php
@@ -20,6 +20,12 @@ defined( 'ABSPATH' ) || exit;
*/
final class PlanRepository {
+ /**
+ * SQL condition on the `p` alias: the plan's owner is exactly the bound slug. The binary cast
+ * makes the match case-sensitive, as the engine's PHP comparisons of the owner are.
+ */
+ private const OWNER_MATCH = 'p.extension_slug = CAST(%s AS BINARY)';
+
/**
* Policy columns stored as JSON.
*
@@ -96,8 +102,9 @@ final class PlanRepository {
// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared
$sql = $wpdb->prepare( "SELECT * FROM {$table} WHERE id = %d", $id );
} else {
- // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared
- $sql = $wpdb->prepare( "SELECT * FROM {$table} WHERE id = %d AND extension_slug = %s", $id, $extension_slug );
+ $owner_match = self::OWNER_MATCH;
+ // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared,WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumber -- the owner condition carries one placeholder.
+ $sql = $wpdb->prepare( "SELECT p.* FROM {$table} p WHERE p.id = %d AND {$owner_match}", $id, $extension_slug );
}
// phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.NotPrepared
@@ -201,7 +208,8 @@ final class PlanRepository {
* `pricing_policy`, `delivery_policy`.
* @return bool False when no row of the plan's extension has its id (nothing is written).
* @throws \InvalidArgumentException If a field is not a writable column.
- * @throws \RuntimeException If the plan has no id or no extension slug, or the update fails.
+ * @throws \RuntimeException If the plan has no id or no extension slug, a value does not fit its
+ * column, the update fails, or the existence read after a 0-row update fails.
*/
public function update_fields( Plan $plan, array $fields ): bool {
global $wpdb;
@@ -216,26 +224,40 @@ final class PlanRepository {
throw new \RuntimeException( 'Cannot update a plan that has no extension slug.' );
}
+ $table = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLANS );
$row = $this->get_row_data( $plan );
$columns = array();
foreach ( $fields as $field ) {
if ( 'extension_slug' === $field || ! array_key_exists( $field, $row ) ) {
throw new \InvalidArgumentException( esc_html( sprintf( 'Cannot update plan field "%s".', $field ) ) );
}
+ if ( is_string( $row[ $field ] ) ) {
+ ColumnFit::assert_fits_column( $table, $field, $row[ $field ] );
+ }
$columns[ $field ] = $row[ $field ];
}
$now = gmdate( 'Y-m-d H:i:s' );
- // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching
- $updated = $wpdb->update(
- SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLANS ),
- array_merge( $columns, array( 'date_updated_gmt' => $now ) ),
- array(
- 'id' => $id,
- 'extension_slug' => $extension_slug,
- )
- );
+ $set = array();
+ $values = array();
+ foreach ( array_merge( $columns, array( 'date_updated_gmt' => $now ) ) as $column => $value ) {
+ if ( null === $value ) {
+ $set[] = "`{$column}` = NULL";
+ } else {
+ $set[] = "`{$column}` = %s";
+ $values[] = $value;
+ }
+ }
+ $values[] = $id;
+ $values[] = $extension_slug;
+
+ $sql = "UPDATE {$table} p SET " . implode( ', ', $set ) . ' WHERE p.id = %d AND ' . self::OWNER_MATCH;
+
+ // Not `$wpdb->update()`: its WHERE cannot express the binary owner match. Column names come
+ // from the plan's own row data; every value is a placeholder.
+ // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.NotPrepared
+ $updated = $wpdb->query( $wpdb->prepare( $sql, $values ) );
if ( false === $updated ) {
throw new \RuntimeException( sprintf( 'Failed to update plan %d: %s', (int) $id, esc_html( $wpdb->last_error ) ) );
@@ -256,6 +278,7 @@ final class PlanRepository {
*
* @param int $id Plan id.
* @param string|null $extension_slug Owning extension slug to scope the check to; null checks any extension.
+ * @throws \RuntimeException If the read fails: a failed read must not look like a missing plan.
*/
public function exists( int $id, ?string $extension_slug = null ): bool {
global $wpdb;
@@ -266,12 +289,16 @@ final class PlanRepository {
// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared
$sql = $wpdb->prepare( "SELECT id FROM {$table} WHERE id = %d", $id );
} else {
- // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared
- $sql = $wpdb->prepare( "SELECT id FROM {$table} WHERE id = %d AND extension_slug = %s", $id, $extension_slug );
+ $owner_match = self::OWNER_MATCH;
+ // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared,WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumber -- the owner condition carries one placeholder.
+ $sql = $wpdb->prepare( "SELECT p.id FROM {$table} p WHERE p.id = %d AND {$owner_match}", $id, $extension_slug );
}
// phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.NotPrepared
$found = $wpdb->get_var( $sql );
+ if ( '' !== $wpdb->last_error ) {
+ throw new \RuntimeException( sprintf( 'Failed to read plan %d: %s', (int) $id, esc_html( $wpdb->last_error ) ) );
+ }
return null !== $found;
}
@@ -319,121 +346,184 @@ final class PlanRepository {
}
/**
- * Add a meta row for a plan, like `add_post_meta()`.
+ * Add a meta row for a plan of `$extension_slug`, like `add_post_meta()`, as one
+ * `INSERT ... SELECT` from the plan row, so no row is written for a missing plan or a plan of
+ * another extension. With `$unique` the same statement checks that the key is absent.
*
- * @param int $plan_id Plan id.
- * @param string $key Meta key.
- * @param mixed $value Meta value; serialized when not scalar.
- * @param bool $unique When true, add nothing if the key already exists. Advisory:
- * checked before the insert with no unique index.
+ * @param int $plan_id Plan id.
+ * @param string $extension_slug The expected owning extension.
+ * @param string $key Meta key.
+ * @param mixed $value Meta value; serialized when not scalar.
+ * @param bool $unique When true, add nothing if the key already exists. Advisory:
+ * no unique index, so concurrent adds can both write.
* @return int|null The new meta row id, or null when `$unique` and the key exists.
- * @throws \InvalidArgumentException If `$key` is empty.
- * @throws \RuntimeException If the insert fails.
+ * @throws \InvalidArgumentException If no plan of that extension has the id.
+ * @throws \RuntimeException If the key is longer than the meta_key column (255 characters), or the insert or the existence read fails.
*/
- public function add_meta( int $plan_id, string $key, $value, bool $unique = false ): ?int {
- global $wpdb;
+ public function add_meta( int $plan_id, string $extension_slug, string $key, $value, bool $unique = false ): ?int {
+ $meta_id = $this->insert_meta_row( $plan_id, $extension_slug, $key, $value, $unique );
+ if ( null !== $meta_id ) {
+ return $meta_id;
+ }
- if ( '' === $key ) {
- throw new \InvalidArgumentException( 'Plan meta key must not be empty.' );
+ // Zero rows: the key is taken (unique only) or no plan of that extension has the id.
+ $owned = $unique && $this->exists( $plan_id, $extension_slug );
+ if ( ! $owned ) {
+ self::throw_not_found( $plan_id, $extension_slug );
}
- if ( $unique && array() !== $this->find_meta_values( $plan_id, $key ) ) {
- return null;
+ return null;
+ }
+
+ /**
+ * Insert one meta row selected from the plan row of `$extension_slug`; with `$unique`, only
+ * while the plan has no row for `$key`.
+ *
+ * @param int $plan_id Plan id.
+ * @param string $extension_slug The expected owning extension.
+ * @param string $key Meta key.
+ * @param mixed $value Meta value; serialized when not scalar.
+ * @param bool $unique Whether the key must be absent.
+ * @return int|null The new meta row id, or null when nothing was inserted.
+ * @throws \RuntimeException If the key does not fit the meta_key column, or the insert fails.
+ */
+ private function insert_meta_row( int $plan_id, string $extension_slug, string $key, $value, bool $unique ): ?int {
+ global $wpdb;
+
+ $meta = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLAN_META );
+ ColumnFit::assert_fits_column( $meta, 'meta_key', $key );
+
+ $plans = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLANS );
+ $sql = "INSERT INTO {$meta} ( plan_id, meta_key, meta_value ) SELECT p.id, %s, %s FROM {$plans} p WHERE p.id = %d AND " . self::OWNER_MATCH;
+ $values = array( $key, maybe_serialize( $value ), $plan_id, $extension_slug );
+ if ( $unique ) {
+ $sql .= " AND NOT EXISTS ( SELECT 1 FROM {$meta} m WHERE m.plan_id = p.id AND m.meta_key = %s )";
+ $values[] = $key;
}
- // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.SlowDBQuery.slow_db_query_meta_key,WordPress.DB.SlowDBQuery.slow_db_query_meta_value
- $inserted = $wpdb->insert(
- SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLAN_META ),
- array(
- 'plan_id' => $plan_id,
- 'meta_key' => $key,
- 'meta_value' => maybe_serialize( $value ),
- )
- );
+ // Fixed SQL fragments only; every value is a placeholder.
+ // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.NotPrepared
+ $inserted = $wpdb->query( $wpdb->prepare( $sql, $values ) );
if ( false === $inserted ) {
throw new \RuntimeException( sprintf( 'Failed to add plan meta "%s" for plan %d: %s', esc_html( $key ), (int) $plan_id, esc_html( $wpdb->last_error ) ) );
}
+ if ( 1 !== (int) $inserted ) {
+ return null;
+ }
return (int) $wpdb->insert_id;
}
+ // phpcs:disable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber -- @throws also lists exceptions raised by the methods called.
/**
- * Update a plan's meta rows for `$key`, like `update_post_meta()`: adds a row when
- * the key is absent, else rewrites every row for the key, or only the rows holding
- * `$prev_value`. The absent-key check runs before the write with no unique index.
+ * Update the meta rows for `$key` of a plan of `$extension_slug`, like `update_post_meta()`:
+ * rewrites every row for the key, or only the rows holding `$prev_value`, and adds the key when
+ * the plan has no row for it. The rows are written first; the plan and key are read only after
+ * a write that changed nothing.
*
- * @param int $plan_id Plan id.
- * @param string $key Meta key.
- * @param mixed $value New value; serialized when not scalar.
- * @param mixed $prev_value Only update rows holding this value; null updates all rows for the key.
- * Any other value ('' and false included) matches literally.
+ * @param int $plan_id Plan id.
+ * @param string $extension_slug The expected owning extension.
+ * @param string $key Meta key.
+ * @param mixed $value New value; serialized when not scalar.
+ * @param mixed $prev_value Only update rows holding this value; null updates all rows for the key.
+ * Any other value ('' and false included) matches literally.
* @return bool True when a row was added or at least one row changed.
- * @throws \InvalidArgumentException If `$key` is empty.
- * @throws \RuntimeException If a write fails.
+ * @throws \InvalidArgumentException If no plan of that extension has the id.
+ * @throws \RuntimeException If the key is longer than the meta_key column (255 characters), or a write or the classifying read fails.
*/
- public function update_meta( int $plan_id, string $key, $value, $prev_value = null ): bool {
+ public function update_meta( int $plan_id, string $extension_slug, string $key, $value, $prev_value = null ): bool {
global $wpdb;
- if ( '' === $key ) {
- throw new \InvalidArgumentException( 'Plan meta key must not be empty.' );
+ $meta = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLAN_META );
+ $plans = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLANS );
+ $values = array( maybe_serialize( $value ), $plan_id, $extension_slug, $key );
+ $where = 'm.plan_id = %d AND ' . self::OWNER_MATCH . ' AND m.meta_key = %s';
+ if ( null !== $prev_value ) {
+ $where .= ' AND m.meta_value = %s';
+ $values[] = maybe_serialize( $prev_value );
}
- if ( array() === $this->find_meta_values( $plan_id, $key ) ) {
- $this->add_meta( $plan_id, $key, $value );
+ // The WHERE clause is built from fixed fragments above; every value is a placeholder.
+ // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.InterpolatedNotPrepared,WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumber,WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare
+ $updated = $wpdb->query( $wpdb->prepare( "UPDATE {$meta} m JOIN {$plans} p ON p.id = m.plan_id SET m.meta_value = %s WHERE {$where}", $values ) );
+
+ if ( false === $updated ) {
+ throw new \RuntimeException( sprintf( 'Failed to update plan meta "%s" for plan %d: %s', esc_html( $key ), (int) $plan_id, esc_html( $wpdb->last_error ) ) );
+ }
+ if ( $updated > 0 ) {
return true;
}
- $where = array(
- 'plan_id' => $plan_id,
- 'meta_key' => $key,
- );
- if ( null !== $prev_value ) {
- $where['meta_value'] = maybe_serialize( $prev_value );
+ $has_key = $this->get_meta_key_presence( $plan_id, $extension_slug, $key );
+ if ( null === $has_key ) {
+ self::throw_not_found( $plan_id, $extension_slug );
+ }
+ if ( $has_key ) {
+ return false;
}
- // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.SlowDBQuery.slow_db_query_meta_key,WordPress.DB.SlowDBQuery.slow_db_query_meta_value
- $updated = $wpdb->update(
- SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLAN_META ),
- array( 'meta_value' => maybe_serialize( $value ) ), // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_value
- $where
- );
-
- if ( false === $updated ) {
- throw new \RuntimeException( sprintf( 'Failed to update plan meta "%s" for plan %d: %s', esc_html( $key ), (int) $plan_id, esc_html( $wpdb->last_error ) ) );
+ $meta_id = $this->insert_meta_row( $plan_id, $extension_slug, $key, $value, false );
+ if ( null === $meta_id ) {
+ self::throw_not_found( $plan_id, $extension_slug );
}
- return $updated > 0;
+ return true;
}
+ // phpcs:enable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber
/**
- * Delete a plan's meta rows for `$key`, like `delete_post_meta()`.
+ * Whether a plan of `$extension_slug` has a meta row for `$key`, read in one statement.
*
- * @param int $plan_id Plan id.
- * @param string $key Meta key.
- * @param mixed $value Only delete rows holding this value; null deletes every row for the key.
- * Any other value ('' and false included) matches literally.
- * @return bool True when at least one row was deleted.
- * @throws \InvalidArgumentException If `$key` is empty.
- * @throws \RuntimeException If the delete fails.
+ * @param int $plan_id Plan id.
+ * @param string $extension_slug The expected owning extension.
+ * @param string $key Meta key.
+ * @return bool|null Whether the key has a row; null when no plan of that extension has the id.
+ * @throws \RuntimeException If the read fails.
*/
- public function delete_meta( int $plan_id, string $key, $value = null ): bool {
+ private function get_meta_key_presence( int $plan_id, string $extension_slug, string $key ): ?bool {
global $wpdb;
- if ( '' === $key ) {
- throw new \InvalidArgumentException( 'Plan meta key must not be empty.' );
+ $meta = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLAN_META );
+ $plans = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLANS );
+
+ $owner_match = self::OWNER_MATCH;
+
+ // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.InterpolatedNotPrepared,WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumber -- the owner condition carries one placeholder.
+ $has_key = $wpdb->get_var( $wpdb->prepare( "SELECT EXISTS ( SELECT 1 FROM {$meta} m WHERE m.plan_id = p.id AND m.meta_key = %s ) FROM {$plans} p WHERE p.id = %d AND {$owner_match}", $key, $plan_id, $extension_slug ) );
+ if ( '' !== $wpdb->last_error ) {
+ throw new \RuntimeException( sprintf( 'Failed to read plan meta "%s" for plan %d: %s', esc_html( $key ), (int) $plan_id, esc_html( $wpdb->last_error ) ) );
}
- $where = array(
- 'plan_id' => $plan_id,
- 'meta_key' => $key,
- );
+ return null === $has_key ? null : '1' === (string) $has_key;
+ }
+
+ /**
+ * Delete the meta rows for `$key` of a plan of `$extension_slug`, like `delete_post_meta()`.
+ *
+ * @param int $plan_id Plan id.
+ * @param string $extension_slug The expected owning extension.
+ * @param string $key Meta key.
+ * @param mixed $value Only delete rows holding this value; null deletes every row for the key.
+ * Any other value ('' and false included) matches literally.
+ * @return bool True when at least one row was deleted (false when no plan of that extension has the id).
+ * @throws \RuntimeException If the delete fails.
+ */
+ public function delete_meta( int $plan_id, string $extension_slug, string $key, $value = null ): bool {
+ global $wpdb;
+
+ $meta = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLAN_META );
+ $plans = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLANS );
+ $values = array( $plan_id, $extension_slug, $key );
+ $where = 'm.plan_id = %d AND ' . self::OWNER_MATCH . ' AND m.meta_key = %s';
if ( null !== $value ) {
- $where['meta_value'] = maybe_serialize( $value );
+ $where .= ' AND m.meta_value = %s';
+ $values[] = maybe_serialize( $value );
}
- // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching
- $deleted = $wpdb->delete( SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLAN_META ), $where );
+ // The WHERE clause is built from fixed fragments above; every value is a placeholder.
+ // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.InterpolatedNotPrepared,WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumber,WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare
+ $deleted = $wpdb->query( $wpdb->prepare( "DELETE m FROM {$meta} m JOIN {$plans} p ON p.id = m.plan_id WHERE {$where}", $values ) );
if ( false === $deleted ) {
throw new \RuntimeException( sprintf( 'Failed to delete plan meta "%s" for plan %d: %s', esc_html( $key ), (int) $plan_id, esc_html( $wpdb->last_error ) ) );
@@ -442,6 +532,18 @@ final class PlanRepository {
return $deleted > 0;
}
+ /**
+ * Refuse a meta write for a plan id no plan of `$extension_slug` has.
+ *
+ * @param int $plan_id Plan id.
+ * @param string $extension_slug The expected owning extension.
+ * @return never
+ * @throws \InvalidArgumentException Always.
+ */
+ private static function throw_not_found( int $plan_id, string $extension_slug ): void {
+ throw new \InvalidArgumentException( esc_html( sprintf( 'No plan of extension "%s" has id %d.', $extension_slug, $plan_id ) ) );
+ }
+
/**
* Read plan meta (WordPress `get_post_meta()` semantics), values unserialized,
* oldest row first.
diff --git a/packages/php/woocommerce-subscriptions-engine/tests/integration/Api/ContractsTest.php b/packages/php/woocommerce-subscriptions-engine/tests/integration/Api/ContractsTest.php
index e37d94c67b0..72abf6b18fa 100644
--- a/packages/php/woocommerce-subscriptions-engine/tests/integration/Api/ContractsTest.php
+++ b/packages/php/woocommerce-subscriptions-engine/tests/integration/Api/ContractsTest.php
@@ -373,7 +373,13 @@ class ContractsTest extends EngineIntegrationTestCase {
$id = $this->contract_with_currency();
$this->assert_validation_errors(
static function () use ( $id ): void {
- Contracts::update( $id, array( 'addresses' => array( 'home' => array( 'first_name' => 'Ada' ) ) ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'addresses' => array( 'home' => array( 'first_name' => 'Ada' ) ),
+ )
+ );
},
array( array( ValidationErrorCode::NOT_IN_ENUM, 'addresses.home' ) )
);
@@ -479,6 +485,7 @@ class ContractsTest extends EngineIntegrationTestCase {
Contracts::update(
$id,
array(
+ 'extension_slug' => self::EXTENSION_SLUG,
'status' => 'never-registered',
'schedule_source' => 'Not a slug',
)
@@ -498,11 +505,25 @@ class ContractsTest extends EngineIntegrationTestCase {
public function test_update_checks_its_arguments_before_reading(): void {
$this->assert_validation_errors(
static function (): void {
- Contracts::update( 999999, array( 'customer_id' => 0 ) );
+ Contracts::update(
+ 999999,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'customer_id' => 0,
+ )
+ );
},
array( array( ValidationErrorCode::OUT_OF_BOUNDS, 'customer_id' ) )
);
- $this->assertNull( Contracts::update( 999999, array( 'customer_id' => 5 ) ) );
+ $this->assertNull(
+ Contracts::update(
+ 999999,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'customer_id' => 5,
+ )
+ )
+ );
}
/**
@@ -643,6 +664,7 @@ class ContractsTest extends EngineIntegrationTestCase {
$id,
array_merge(
array(
+ 'extension_slug' => self::EXTENSION_SLUG,
'payment_method' => 'other',
'billing_total' => '25',
),
@@ -672,13 +694,28 @@ class ContractsTest extends EngineIntegrationTestCase {
$this->expectException( InvalidArgumentException::class );
- Contracts::update( $id, array( 'billing_total' => '10' ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'billing_total' => '10',
+ )
+ );
}
public function test_a_null_money_value_resets_to_zero_without_a_currency(): void {
$id = $this->create_released( array( 'extension_slug' => self::EXTENSION_SLUG ) )->get_id();
- $this->assertInstanceOf( ContractView::class, Contracts::update( $id, array( 'billing_total' => null ) ) );
+ $this->assertInstanceOf(
+ ContractView::class,
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'billing_total' => null,
+ )
+ )
+ );
$this->assertSame( '0.00000000', $this->view( $id )->get_billing_total() );
}
@@ -691,9 +728,21 @@ class ContractsTest extends EngineIntegrationTestCase {
)
)->get_id();
- Contracts::update( $id, array( 'billing_total' => '10' ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'billing_total' => '10',
+ )
+ );
try {
- Contracts::update( $id, array( 'currency' => null ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'currency' => null,
+ )
+ );
$this->fail( 'Expected an InvalidArgumentException.' );
} catch ( InvalidArgumentException $e ) {
$this->assertSame( 'USD', $this->view( $id )->get_currency() );
@@ -705,8 +754,9 @@ class ContractsTest extends EngineIntegrationTestCase {
Contracts::update(
$id,
array(
- 'currency' => null,
- 'billing_total' => null,
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'currency' => null,
+ 'billing_total' => null,
)
)
);
@@ -719,18 +769,34 @@ class ContractsTest extends EngineIntegrationTestCase {
$id = $this->create_released( array( 'extension_slug' => self::EXTENSION_SLUG ) )->get_id();
- $this->assertInstanceOf( ContractView::class, Contracts::update( $id, array( 'customer_id' => $customer ) ) );
$this->assertInstanceOf(
ContractView::class,
Contracts::update(
$id,
array(
- 'currency' => 'USD',
- 'billing_total' => '19.99',
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'customer_id' => $customer,
+ )
+ )
+ );
+ $this->assertInstanceOf(
+ ContractView::class,
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'currency' => 'USD',
+ 'billing_total' => '19.99',
)
)
);
- $updated = Contracts::update( $id, array( 'status' => ContractStatus::ACTIVE ) );
+ $updated = Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'status' => ContractStatus::ACTIVE,
+ )
+ );
$this->assertInstanceOf( ContractView::class, $updated );
$this->assertEquals( $this->view( $id ), $updated, 'The returned view matches a fresh read.' );
@@ -752,7 +818,13 @@ class ContractsTest extends EngineIntegrationTestCase {
)
)->get_id();
- Contracts::update( $id, array( 'payment_token_id' => 6 ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'payment_token_id' => 6,
+ )
+ );
$view = $this->view( $id );
$this->assertSame( 'dummy', $view->get_payment_method() );
@@ -763,7 +835,7 @@ class ContractsTest extends EngineIntegrationTestCase {
public function test_update_with_no_fields_returns_the_view(): void {
$id = $this->create_released( array( 'extension_slug' => self::EXTENSION_SLUG ) )->get_id();
- $updated = Contracts::update( $id, array() );
+ $updated = Contracts::update( $id, array( 'extension_slug' => self::EXTENSION_SLUG ) );
$this->assertInstanceOf( ContractView::class, $updated );
$this->assertSame( $id, $updated->get_id() );
@@ -789,7 +861,13 @@ class ContractsTest extends EngineIntegrationTestCase {
add_filter( 'query', $race );
try {
- $updated = Contracts::update( $id, array( 'payment_method_title' => 'X' ) );
+ $updated = Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'payment_method_title' => 'X',
+ )
+ );
} finally {
remove_filter( 'query', $race );
}
@@ -832,7 +910,13 @@ class ContractsTest extends EngineIntegrationTestCase {
add_filter( 'query', $race );
try {
- $updated = Contracts::update( $id, array( 'payment_method_title' => 'X' ) );
+ $updated = Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'payment_method_title' => 'X',
+ )
+ );
} finally {
remove_filter( 'query', $race );
}
@@ -862,8 +946,9 @@ class ContractsTest extends EngineIntegrationTestCase {
$updated = Contracts::update(
$id,
array(
- 'items' => array( array( 'item_name' => 'Cocoa' ) ),
- 'addresses' => array( 'billing' => array( 'city' => 'Faro' ) ),
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'items' => array( array( 'item_name' => 'Cocoa' ) ),
+ 'addresses' => array( 'billing' => array( 'city' => 'Faro' ) ),
)
);
$this->assertInstanceOf( ContractView::class, $updated );
@@ -949,7 +1034,13 @@ class ContractsTest extends EngineIntegrationTestCase {
$this->assert_write_throws_when_child_inserts_fail(
SchemaInstaller::TABLE_CONTRACT_ITEMS,
static function () use ( $id ) {
- return Contracts::update( $id, array( 'items' => array( array( 'item_name' => 'Cocoa' ) ) ) );
+ return Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'items' => array( array( 'item_name' => 'Cocoa' ) ),
+ )
+ );
}
);
@@ -972,7 +1063,13 @@ class ContractsTest extends EngineIntegrationTestCase {
$this->assert_write_throws_when_child_inserts_fail(
SchemaInstaller::TABLE_CONTRACT_ADDRESSES,
static function () use ( $id ) {
- return Contracts::update( $id, array( 'addresses' => array( 'billing' => array( 'city' => 'Faro' ) ) ) );
+ return Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'addresses' => array( 'billing' => array( 'city' => 'Faro' ) ),
+ )
+ );
}
);
@@ -992,6 +1089,7 @@ class ContractsTest extends EngineIntegrationTestCase {
Contracts::update(
$id,
array(
+ 'extension_slug' => self::EXTENSION_SLUG,
'selling_plan_id' => null,
'next_payment_gmt' => null,
)
@@ -1003,7 +1101,15 @@ class ContractsTest extends EngineIntegrationTestCase {
}
public function test_update_on_an_unknown_contract_returns_null(): void {
- $this->assertNull( Contracts::update( 999999, array( 'status' => ContractStatus::ACTIVE ) ) );
+ $this->assertNull(
+ Contracts::update(
+ 999999,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'status' => ContractStatus::ACTIVE,
+ )
+ )
+ );
}
public function test_an_unknown_update_key_is_ignored_with_a_notice(): void {
@@ -1016,6 +1122,7 @@ class ContractsTest extends EngineIntegrationTestCase {
Contracts::update(
$id,
array(
+ 'extension_slug' => self::EXTENSION_SLUG,
'nonsense' => 1,
'payment_method' => 'dummy',
'items' => array(
@@ -1034,23 +1141,369 @@ class ContractsTest extends EngineIntegrationTestCase {
$this->assertArrayNotHasKey( 'price', $items[0] );
}
- public function test_extension_slug_is_not_an_update_key(): void {
+ /**
+ * @testdox update() writes only a contract of the named extension and never rewrites its extension.
+ */
+ public function test_update_is_scoped_to_the_owning_extension(): void {
$id = $this->create_released( array( 'extension_slug' => self::EXTENSION_SLUG ) )->get_id();
- $this->setExpectedIncorrectUsage( Contracts::class . '::update' );
- Contracts::update(
+ $this->assertNull(
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => 'other',
+ 'customer_id' => 7,
+ )
+ ),
+ 'A contract of another extension reads as missing.'
+ );
+ $this->assertSame( self::EXTENSION_SLUG, $this->view( $id )->get_extension_slug(), 'The extension slug is not rewritten.' );
+ $this->assertNull( $this->view( $id )->get_customer_id(), 'Nothing is written.' );
+ $this->assertNull( $this->get_lock_holder( $id )['operation'], 'The foreign contract is not locked.' );
+
+ $updated = Contracts::update(
$id,
array(
- 'extension_slug' => 'other',
+ 'extension_slug' => self::EXTENSION_SLUG,
'customer_id' => 7,
)
);
-
+ $this->assertNotNull( $updated );
$view = $this->view( $id );
$this->assertSame( self::EXTENSION_SLUG, $view->get_extension_slug(), 'The extension slug is not rewritten.' );
$this->assertSame( 7, $view->get_customer_id() );
}
+ /**
+ * @testdox update() requires the expected owner and reports its absence with every other shape error.
+ */
+ public function test_update_requires_the_owner(): void {
+ $id = $this->contract_with_currency();
+
+ $this->assert_validation_errors(
+ static function () use ( $id ): void {
+ Contracts::update( $id, array( 'currency' => 'usd' ) );
+ },
+ array(
+ array( ValidationErrorCode::REQUIRED, 'extension_slug' ),
+ array( ValidationErrorCode::INVALID_VALUE, 'currency' ),
+ )
+ );
+ $this->assert_validation_errors(
+ static function () use ( $id ): void {
+ Contracts::update( $id, array( 'extension_slug' => 5 ) );
+ },
+ array( array( ValidationErrorCode::INVALID_TYPE, 'extension_slug' ) )
+ );
+ }
+
+ /**
+ * @testdox A contract of another extension reads as missing to update(): also with no fields, and while another operation holds its lock.
+ */
+ public function test_a_foreign_contract_reads_as_missing_to_update(): void {
+ $id = $this->contract_with_currency();
+
+ $this->assertNull( Contracts::update( $id, array( 'extension_slug' => 'other' ) ), 'A fieldless update is scoped too.' );
+ $this->assertNotNull( Contracts::update( $id, array( 'extension_slug' => self::EXTENSION_SLUG ) ) );
+
+ $lock = $this->take_lock( $id, 'hold' );
+ try {
+ $this->assertNull(
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => 'other',
+ 'billing_total' => '9',
+ )
+ ),
+ 'No ContractLockedException for a foreign row.'
+ );
+ $this->assertSame( '0.00000000', (string) $this->view( $id )->get_billing_total(), 'Nothing is written.' );
+ } finally {
+ $lock->release();
+ }
+ }
+
+ /**
+ * @testdox The owner matches exactly: a slug differing only in case reads as another extension.
+ */
+ public function test_the_owner_match_is_case_sensitive(): void {
+ $id = $this->contract_with_currency();
+ $upper = strtoupper( self::EXTENSION_SLUG );
+
+ $this->assertNull( Contracts::lock( $id, $upper, 'hold' ) );
+ $this->assertNull(
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => $upper,
+ 'billing_total' => '9',
+ )
+ )
+ );
+ $this->assertNull( Contracts::update( $id, array( 'extension_slug' => $upper ) ), 'The fieldless path agrees.' );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'k', 'v' );
+ $this->assertFalse( Contracts::delete_meta( $id, $upper, 'k' ) );
+ $this->assertSame( 'v', Contracts::get_meta( $id, 'k', true ), 'The case variant deletes nothing.' );
+ $this->expectException( InvalidArgumentException::class );
+ Contracts::add_meta( $id, $upper, 'k', 'v' );
+ }
+
+ /**
+ * @testdox update() reports argument errors before refusing a lock for another contract.
+ */
+ public function test_update_reports_argument_errors_before_the_lock(): void {
+ $id = $this->contract_with_currency();
+ $other = $this->contract_with_currency();
+ $lock = $this->take_lock( $other, 'hold' );
+ try {
+ $this->assert_validation_errors(
+ static function () use ( $id, $lock ): void {
+ Contracts::update( $id, array( 'currency' => 'usd' ), $lock );
+ },
+ array(
+ array( ValidationErrorCode::REQUIRED, 'extension_slug' ),
+ array( ValidationErrorCode::INVALID_VALUE, 'currency' ),
+ )
+ );
+ } finally {
+ $lock->release();
+ }
+ }
+
+ /**
+ * @testdox lock() reports an empty owner alone; the label is refused in a second step.
+ */
+ public function test_lock_reports_the_owner_before_the_label(): void {
+ $id = $this->contract_with_currency();
+
+ $this->assert_validation_errors(
+ static function () use ( $id ): void {
+ Contracts::lock( $id, '', '' );
+ },
+ array( array( ValidationErrorCode::REQUIRED, 'extension_slug' ) )
+ );
+ }
+
+ /**
+ * @testdox lock() takes only a contract of the named extension and refuses an empty owner.
+ */
+ public function test_lock_is_scoped_to_the_owning_extension(): void {
+ $id = $this->contract_with_currency();
+
+ $this->assertNull( Contracts::lock( $id, 'other', 'hold' ), 'A contract of another extension reads as missing.' );
+ $this->assertNull( $this->view( $id )->get_lock_state(), 'The foreign contract is not locked.' );
+
+ $this->assert_validation_errors(
+ static function () use ( $id ): void {
+ Contracts::lock( $id, '', 'hold' );
+ },
+ array( array( ValidationErrorCode::REQUIRED, 'extension_slug' ) )
+ );
+
+ $lock = Contracts::lock( $id, self::EXTENSION_SLUG, 'hold' );
+ $this->assertNotNull( $lock );
+ $lock->release();
+ }
+
+ /**
+ * @testdox Cycle writes require the expected owner, reported with every other shape error.
+ */
+ public function test_cycle_writes_require_the_owner(): void {
+ $id = $this->contract_with_currency();
+ $cycle = $this->add_cycle_locked( $id, $this->cycle_args( array( 'status' => CycleStatus::PENDING ) ) );
+ $this->assertInstanceOf( CycleView::class, $cycle );
+ $cycle_id = (int) $cycle->get_id();
+ $lock = $this->take_lock( $id, 'renew' );
+ try {
+ $this->assert_validation_errors(
+ function () use ( $id, $lock ): void {
+ $args = $this->cycle_args( array( 'currency' => 'usd' ) );
+ unset( $args['extension_slug'] );
+ Contracts::add_cycle( $id, $args, $lock );
+ },
+ array(
+ array( ValidationErrorCode::REQUIRED, 'extension_slug' ),
+ array( ValidationErrorCode::INVALID_VALUE, 'currency' ),
+ )
+ );
+ $this->assert_validation_errors(
+ static function () use ( $cycle_id, $lock ): void {
+ Contracts::transition_cycle(
+ $cycle_id,
+ array(
+ 'extension_slug' => 5,
+ 'from_status' => CycleStatus::PENDING,
+ 'to_status' => CycleStatus::BILLED,
+ ),
+ $lock
+ );
+ },
+ array( array( ValidationErrorCode::INVALID_TYPE, 'extension_slug' ) )
+ );
+ $this->assert_validation_errors(
+ static function () use ( $cycle_id, $lock ): void {
+ Contracts::transition_cycle(
+ $cycle_id,
+ array(
+ 'from_status' => CycleStatus::PENDING,
+ 'to_status' => CycleStatus::BILLED,
+ 'order_id' => -1,
+ ),
+ $lock
+ );
+ },
+ array(
+ array( ValidationErrorCode::REQUIRED, 'extension_slug' ),
+ array( ValidationErrorCode::OUT_OF_BOUNDS, 'order_id' ),
+ )
+ );
+ $this->assertSame( 1, $this->stored_cycle_count( $id ), 'No cycle is added.' );
+ $this->assertSame( CycleStatus::PENDING, $this->cycle( $id, $cycle_id )->get_status()->get_value(), 'No cycle is moved.' );
+ } finally {
+ $lock->release();
+ }
+ }
+
+ /**
+ * @testdox Cycle writes with the owner's lock but another extension's slug add or move no cycle; with the owner's slug they land.
+ */
+ public function test_cycle_writes_with_another_owner_are_refused(): void {
+ $id = $this->contract_with_currency();
+ $cycle = $this->add_cycle_locked( $id, $this->cycle_args( array( 'status' => CycleStatus::PENDING ) ) );
+ $this->assertInstanceOf( CycleView::class, $cycle );
+ $cycle_id = (int) $cycle->get_id();
+ $lock = $this->take_lock( $id, 'renew' );
+ try {
+ $writes = array(
+ 'add_cycle' => function () use ( $id, $lock ): void {
+ Contracts::add_cycle( $id, $this->cycle_args( array( 'extension_slug' => 'other' ) ), $lock );
+ },
+ 'transition_cycle' => function () use ( $cycle_id, $lock ): void {
+ Contracts::transition_cycle( $cycle_id, $this->transition( CycleStatus::PENDING, CycleStatus::BILLED, array( 'extension_slug' => 'other' ) ), $lock );
+ },
+ );
+ foreach ( $writes as $method => $write ) {
+ try {
+ $write();
+ $this->fail( "Expected {$method}() to refuse the lock." );
+ } catch ( InvalidArgumentException $e ) {
+ $this->assertNotInstanceOf( ValidationException::class, $e, "{$method}() refuses the lock, not the arguments." );
+ $this->assertStringContainsString( 'another extension', $e->getMessage() );
+ }
+ }
+ $this->assertSame( 1, $this->stored_cycle_count( $id ), 'No cycle is added.' );
+ $this->assertSame( CycleStatus::PENDING, $this->cycle( $id, $cycle_id )->get_status()->get_value(), 'No cycle is moved.' );
+
+ $this->assertInstanceOf( CycleView::class, Contracts::add_cycle( $id, $this->cycle_args(), $lock ) );
+ $this->assertTrue( Contracts::transition_cycle( $cycle_id, $this->transition( CycleStatus::PENDING, CycleStatus::BILLED ), $lock ) );
+ } finally {
+ $lock->release();
+ }
+ }
+
+ /**
+ * @testdox A write with the owner's lock but another extension's slug is refused before anything is written.
+ */
+ public function test_a_lock_passed_with_another_owner_is_refused(): void {
+ $id = $this->contract_with_currency();
+ $lock = $this->take_lock( $id, 'hold' );
+
+ $writes = array(
+ 'update' => static function () use ( $id, $lock ): void {
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => 'other',
+ 'billing_total' => '9',
+ ),
+ $lock
+ );
+ },
+ 'add_meta' => static function () use ( $id, $lock ): void {
+ Contracts::add_meta( $id, 'other', 'k', 'v', false, $lock );
+ },
+ 'update_meta' => static function () use ( $id, $lock ): void {
+ Contracts::update_meta( $id, 'other', 'k', 'v', null, $lock );
+ },
+ 'delete_meta' => static function () use ( $id, $lock ): void {
+ Contracts::delete_meta( $id, 'other', 'k', null, $lock );
+ },
+ );
+ try {
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'k', 'kept', false, $lock );
+ foreach ( $writes as $method => $write ) {
+ try {
+ $write();
+ $this->fail( "Expected {$method}() to refuse the lock." );
+ } catch ( InvalidArgumentException $e ) {
+ $this->assertNotInstanceOf( ValidationException::class, $e, "{$method}() refuses the lock, not the arguments." );
+ $this->assertStringContainsString( 'another extension', $e->getMessage() );
+ }
+ }
+ $this->assertSame( '0.00000000', (string) $this->view( $id )->get_billing_total() );
+ $this->assertSame( array( 'kept' ), Contracts::get_meta( $id, 'k' ) );
+ } finally {
+ $lock->release();
+ }
+ }
+
+ /**
+ * @testdox Meta writes check the owner and the key in one pass, reporting both errors at once.
+ */
+ public function test_meta_writes_report_owner_and_key_errors_together(): void {
+ $id = $this->contract_with_currency();
+
+ foreach ( array( 'add_meta', 'update_meta', 'delete_meta' ) as $method ) {
+ $this->assert_validation_errors(
+ static function () use ( $id, $method ): void {
+ Contracts::{$method}( $id, '', '', 'v' );
+ },
+ array(
+ array( ValidationErrorCode::REQUIRED, 'extension_slug' ),
+ array( ValidationErrorCode::REQUIRED, 'key' ),
+ )
+ );
+ }
+ }
+
+ /**
+ * @testdox Unlocked meta writes on a contract of another extension write nothing: add and update throw, delete returns false, also while its lock is held.
+ */
+ public function test_unlocked_meta_writes_on_a_foreign_contract_write_nothing(): void {
+ $id = $this->contract_with_currency();
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'k', 'kept' );
+
+ foreach ( array( false, true ) as $locked ) {
+ $lock = $locked ? $this->take_lock( $id, 'hold' ) : null;
+ try {
+ $writes = array(
+ 'add_meta' => static function () use ( $id ): void {
+ Contracts::add_meta( $id, 'other', 'k', 'theirs' );
+ },
+ 'update_meta' => static function () use ( $id ): void {
+ Contracts::update_meta( $id, 'other', 'k', 'theirs' );
+ },
+ );
+ foreach ( $writes as $method => $write ) {
+ try {
+ $write();
+ $this->fail( "Expected {$method}() to refuse a foreign contract." );
+ } catch ( InvalidArgumentException $e ) {
+ $this->assertStringContainsString( sprintf( 'has id %d', $id ), $e->getMessage() );
+ $this->assertStringContainsString( 'other', $e->getMessage() );
+ }
+ }
+ $this->assertFalse( Contracts::delete_meta( $id, 'other', 'k' ) );
+ $this->assertSame( array( 'kept' ), Contracts::get_meta( $id, 'k' ) );
+ } finally {
+ if ( null !== $lock ) {
+ $lock->release();
+ }
+ }
+ }
+ }
+
public function test_snapshot_keys_are_ignored_with_a_notice(): void {
$this->setExpectedIncorrectUsage( Contracts::class . '::create' );
$messages = array();
@@ -1129,10 +1582,11 @@ class ContractsTest extends EngineIntegrationTestCase {
private function cycle_args( array $overrides = array() ): array {
return array_merge(
array(
- 'status' => CycleStatus::BILLED,
- 'starts_at_gmt' => '2026-01-01 00:00:00',
- 'ends_at_gmt' => '2026-02-01 00:00:00',
- 'currency' => 'USD',
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'status' => CycleStatus::BILLED,
+ 'starts_at_gmt' => '2026-01-01 00:00:00',
+ 'ends_at_gmt' => '2026-02-01 00:00:00',
+ 'currency' => 'USD',
),
$overrides
);
@@ -1569,8 +2023,9 @@ class ContractsTest extends EngineIntegrationTestCase {
private function transition( string $from, string $to, array $extra = array() ): array {
return array_merge(
array(
- 'from_status' => $from,
- 'to_status' => $to,
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'from_status' => $from,
+ 'to_status' => $to,
),
$extra
);
@@ -1585,8 +2040,8 @@ class ContractsTest extends EngineIntegrationTestCase {
$lock = $this->take_lock( $id, 'hold' );
$this->assertSame( $id, $lock->get_contract_id() );
- $this->assertNull( Contracts::lock( $id, 'cancel' ), 'A held lock is a lost race.' );
- $this->assertNull( Contracts::lock( 999999, 'hold' ), 'An unknown contract has no lock to take.' );
+ $this->assertNull( Contracts::lock( $id, self::EXTENSION_SLUG, 'cancel' ), 'A held lock is a lost race.' );
+ $this->assertNull( Contracts::lock( 999999, self::EXTENSION_SLUG, 'hold' ), 'An unknown contract has no lock to take.' );
$lock_state = $this->view( $id )->get_lock_state();
$this->assertNotNull( $lock_state );
@@ -1595,7 +2050,7 @@ class ContractsTest extends EngineIntegrationTestCase {
foreach ( array( '', str_repeat( 'x', 65 ) ) as $label ) {
try {
- Contracts::lock( $id, $label );
+ Contracts::lock( $id, self::EXTENSION_SLUG, $label );
$this->fail( 'Expected an InvalidArgumentException.' );
} catch ( InvalidArgumentException $e ) {
$this->assertStringNotContainsString( 'lock_token', $e->getMessage() );
@@ -1614,7 +2069,13 @@ class ContractsTest extends EngineIntegrationTestCase {
$this->assertNotNull( $lock_state );
$this->assertSame( 'create', $lock_state['operation'] );
try {
- Contracts::update( $default['contract']->get_id(), array( 'currency' => 'USD' ) );
+ Contracts::update(
+ $default['contract']->get_id(),
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'currency' => 'USD',
+ )
+ );
$this->fail( 'Expected an unlocked write to be refused.' );
} catch ( ContractLockedException $e ) {
$this->assertSame( 'create', $e->get_operation() );
@@ -1637,17 +2098,42 @@ class ContractsTest extends EngineIntegrationTestCase {
$this->assertSame( 'USD', $created['contract']->get_currency() );
$this->assertSame( ContractStatus::DRAFT, $this->view( $id )->get_status() );
$this->assertInstanceOf( CycleView::class, Contracts::add_cycle( $id, $this->cycle_args(), $lock ) );
- $this->assertInstanceOf( ContractView::class, Contracts::update( $id, array( 'status' => ContractStatus::ACTIVE ), $lock ) );
+ $this->assertInstanceOf(
+ ContractView::class,
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'status' => ContractStatus::ACTIVE,
+ ),
+ $lock
+ )
+ );
try {
- Contracts::update( $id, array( 'status' => ContractStatus::ON_HOLD ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'status' => ContractStatus::ON_HOLD,
+ )
+ );
$this->fail( 'Expected an unlocked write to be refused.' );
} catch ( ContractLockedException $e ) {
$this->assertSame( 'checkout', $e->get_operation() );
}
$lock->release();
- $this->assertInstanceOf( ContractView::class, Contracts::update( $id, array( 'status' => ContractStatus::ON_HOLD ) ) );
+ $this->assertInstanceOf(
+ ContractView::class,
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'status' => ContractStatus::ON_HOLD,
+ )
+ )
+ );
}
/**
@@ -1786,7 +2272,13 @@ class ContractsTest extends EngineIntegrationTestCase {
$lock = $this->take_lock( $id, 'renewal' );
try {
- Contracts::update( $id, array( 'items' => array( array( 'item_name' => 'Cocoa' ) ) ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'items' => array( array( 'item_name' => 'Cocoa' ) ),
+ )
+ );
$this->fail( 'Expected a conflict.' );
} catch ( ContractLockedException $e ) {
$this->assertSame( 'renewal', $e->get_operation() );
@@ -1794,7 +2286,13 @@ class ContractsTest extends EngineIntegrationTestCase {
$this->assertSame( array( 'Coffee' ), array_column( (array) $this->view( $id )->get_items(), 'item_name' ) );
$lock->release();
- Contracts::update( $id, array( 'items' => array( array( 'item_name' => 'Cocoa' ) ) ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'items' => array( array( 'item_name' => 'Cocoa' ) ),
+ )
+ );
$this->assertSame( array( 'Cocoa' ), array_column( (array) $this->view( $id )->get_items(), 'item_name' ) );
$this->assertSame( $id, $this->take_lock( $id, 'next' )->get_contract_id(), 'The internal lock was released.' );
}
@@ -1804,10 +2302,22 @@ class ContractsTest extends EngineIntegrationTestCase {
*/
public function test_a_refused_unlocked_update_releases_the_internal_lock(): void {
$id = $this->contract_with_currency();
- Contracts::update( $id, array( 'billing_total' => '10' ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'billing_total' => '10',
+ )
+ );
try {
- Contracts::update( $id, array( 'currency' => null ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'currency' => null,
+ )
+ );
$this->fail( 'Expected totals without a currency to be refused.' );
} catch ( InvalidArgumentException $e ) {
$this->assertNull( $this->view( $id )->get_lock_state(), 'The internal lock is released.' );
@@ -1828,7 +2338,16 @@ class ContractsTest extends EngineIntegrationTestCase {
};
add_filter( 'query', $record );
try {
- $this->assertInstanceOf( ContractView::class, Contracts::update( $id, array( 'status' => ContractStatus::ON_HOLD ) ) );
+ $this->assertInstanceOf(
+ ContractView::class,
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'status' => ContractStatus::ON_HOLD,
+ )
+ )
+ );
} finally {
remove_filter( 'query', $record );
}
@@ -1846,14 +2365,28 @@ class ContractsTest extends EngineIntegrationTestCase {
$this->take_lock( $id, 'renewal' );
try {
- Contracts::update( $id, array( 'status' => ContractStatus::ACTIVE ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'status' => ContractStatus::ACTIVE,
+ )
+ );
$this->fail( 'Expected a conflict.' );
} catch ( ContractLockedException $e ) {
$this->assertSame( 'renewal', $e->get_operation() );
}
$this->assertSame( ContractStatus::ON_HOLD, $this->view( $id )->get_status() );
- $this->assertNull( Contracts::update( 999999, array( 'status' => ContractStatus::ACTIVE ) ) );
+ $this->assertNull(
+ Contracts::update(
+ 999999,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'status' => ContractStatus::ACTIVE,
+ )
+ )
+ );
}
/**
@@ -1870,7 +2403,13 @@ class ContractsTest extends EngineIntegrationTestCase {
};
add_filter( 'query', $record );
try {
- Contracts::update( $id, array( 'items' => array( array( 'item_name' => 'Cocoa' ) ) ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'items' => array( array( 'item_name' => 'Cocoa' ) ),
+ )
+ );
$this->fail( 'Expected a conflict.' );
} catch ( ContractLockedException $e ) {
$this->assertSame( 'renewal', $e->get_operation() );
@@ -1894,7 +2433,14 @@ class ContractsTest extends EngineIntegrationTestCase {
$writes = array(
'update' => static function () use ( $id, $lock ): void {
- Contracts::update( $id, array( 'status' => ContractStatus::ON_HOLD ), $lock );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'status' => ContractStatus::ON_HOLD,
+ ),
+ $lock
+ );
},
'add_cycle' => function () use ( $id, $lock ): void {
Contracts::add_cycle( $id, $this->cycle_args(), $lock );
@@ -1903,13 +2449,13 @@ class ContractsTest extends EngineIntegrationTestCase {
Contracts::transition_cycle( $cycle_id, $this->transition( CycleStatus::PENDING, CycleStatus::BILLED ), $lock );
},
'add_meta' => static function () use ( $id, $lock ): void {
- Contracts::add_meta( $id, 'k', 'v', false, $lock );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'k', 'v', false, $lock );
},
'update_meta' => static function () use ( $id, $lock ): void {
- Contracts::update_meta( $id, 'k', 'v', null, $lock );
+ Contracts::update_meta( $id, self::EXTENSION_SLUG, 'k', 'v', null, $lock );
},
'delete_meta' => static function () use ( $id, $lock ): void {
- Contracts::delete_meta( $id, 'k', null, $lock );
+ Contracts::delete_meta( $id, self::EXTENSION_SLUG, 'k', null, $lock );
},
);
@@ -1944,10 +2490,10 @@ class ContractsTest extends EngineIntegrationTestCase {
$writes = array(
'add_meta' => static function () use ( $id, $lock ): void {
- Contracts::add_meta( $id, 'k', 'v', false, $lock );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'k', 'v', false, $lock );
},
'add_meta unique' => static function () use ( $id, $lock ): void {
- Contracts::add_meta( $id, 'k', 'v', true, $lock );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'k', 'v', true, $lock );
},
'add_cycle' => function () use ( $id, $lock ): void {
Contracts::add_cycle( $id, $this->cycle_args(), $lock );
@@ -1972,7 +2518,7 @@ class ContractsTest extends EngineIntegrationTestCase {
$this->fail_next_query( '/^\s*INSERT INTO \S*contract_meta .* FROM \S*subscription_contracts c WHERE c\.id = \d+/s' );
try {
- Contracts::add_meta( $id, 'k', 'v', true );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'k', 'v', true );
$this->fail( 'Expected the failed insert to throw.' );
} catch ( ContractLockedException $e ) {
$this->fail( 'The internal lock is still held, so the failure is not a lock conflict.' );
@@ -2053,8 +2599,8 @@ class ContractsTest extends EngineIntegrationTestCase {
public function test_meta_keeps_several_values_under_one_key(): void {
$id = $this->create_released( array( 'extension_slug' => self::EXTENSION_SLUG ) )->get_id();
- $this->assertIsInt( Contracts::add_meta( $id, 'note', 'one' ) );
- $this->assertIsInt( Contracts::add_meta( $id, 'note', array( 'two' ) ) );
+ $this->assertIsInt( Contracts::add_meta( $id, self::EXTENSION_SLUG, 'note', 'one' ) );
+ $this->assertIsInt( Contracts::add_meta( $id, self::EXTENSION_SLUG, 'note', array( 'two' ) ) );
$this->assertSame( array( 'one', array( 'two' ) ), Contracts::get_meta( $id, 'note' ) );
$this->assertSame( 'one', Contracts::get_meta( $id, 'note', true ) );
@@ -2063,48 +2609,48 @@ class ContractsTest extends EngineIntegrationTestCase {
public function test_a_unique_meta_add_refuses_an_existing_key(): void {
$id = $this->create_released( array( 'extension_slug' => self::EXTENSION_SLUG ) )->get_id();
- Contracts::add_meta( $id, 'note', 'one' );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'note', 'one' );
- $this->assertNull( Contracts::add_meta( $id, 'note', 'two', true ) );
+ $this->assertNull( Contracts::add_meta( $id, self::EXTENSION_SLUG, 'note', 'two', true ) );
$this->assertSame( array( 'one' ), Contracts::get_meta( $id, 'note' ) );
}
public function test_update_meta_with_a_previous_value_and_delete_one_value(): void {
$id = $this->create_released( array( 'extension_slug' => self::EXTENSION_SLUG ) )->get_id();
- Contracts::add_meta( $id, 'note', 'one' );
- Contracts::add_meta( $id, 'note', 'two' );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'note', 'one' );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'note', 'two' );
- $this->assertTrue( Contracts::update_meta( $id, 'note', 'three', 'two' ) );
+ $this->assertTrue( Contracts::update_meta( $id, self::EXTENSION_SLUG, 'note', 'three', 'two' ) );
$this->assertSame( array( 'one', 'three' ), Contracts::get_meta( $id, 'note' ) );
- $this->assertTrue( Contracts::delete_meta( $id, 'note', 'one' ) );
+ $this->assertTrue( Contracts::delete_meta( $id, self::EXTENSION_SLUG, 'note', 'one' ) );
$this->assertSame( array( 'three' ), Contracts::get_meta( $id, 'note' ) );
}
public function test_only_null_matches_any_meta_value(): void {
$id = $this->create_released( array( 'extension_slug' => self::EXTENSION_SLUG ) )->get_id();
- Contracts::add_meta( $id, 'note', 'one' );
- Contracts::add_meta( $id, 'note', '' );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'note', 'one' );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'note', '' );
- $this->assertTrue( Contracts::update_meta( $id, 'note', 'blank', '' ) );
+ $this->assertTrue( Contracts::update_meta( $id, self::EXTENSION_SLUG, 'note', 'blank', '' ) );
$this->assertSame( array( 'one', 'blank' ), Contracts::get_meta( $id, 'note' ), 'An empty previous value matches literally.' );
- $this->assertFalse( Contracts::delete_meta( $id, 'note', '' ) );
+ $this->assertFalse( Contracts::delete_meta( $id, self::EXTENSION_SLUG, 'note', '' ) );
$this->assertSame( array( 'one', 'blank' ), Contracts::get_meta( $id, 'note' ), 'An empty value deletes only empty values.' );
- $this->assertTrue( Contracts::delete_meta( $id, 'note' ) );
+ $this->assertTrue( Contracts::delete_meta( $id, self::EXTENSION_SLUG, 'note' ) );
$this->assertSame( array(), Contracts::get_meta( $id, 'note' ) );
}
public function test_meta_reads_for_an_unknown_contract_are_empty(): void {
- $this->assertFalse( Contracts::delete_meta( 999999, 'note' ) );
+ $this->assertFalse( Contracts::delete_meta( 999999, self::EXTENSION_SLUG, 'note' ) );
$this->assertSame( '', Contracts::get_meta( 999999, 'note', true ) );
$this->assertSame( array(), Contracts::get_meta( 999999, 'note' ) );
}
public function test_deleting_a_contract_removes_its_meta(): void {
$id = $this->create_released( array( 'extension_slug' => self::EXTENSION_SLUG ) )->get_id();
- Contracts::add_meta( $id, 'note', 'one' );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'note', 'one' );
$this->assertTrue( ( new ContractRepository() )->delete( $id ) );
@@ -2116,18 +2662,19 @@ class ContractsTest extends EngineIntegrationTestCase {
$this->expectException( InvalidArgumentException::class );
- Contracts::add_meta( $id, '', 'one' );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, '', 'one' );
}
public function test_meta_survives_a_contract_update(): void {
$id = $this->create_released( array( 'extension_slug' => self::EXTENSION_SLUG ) )->get_id();
- Contracts::add_meta( $id, 'note', 'kept' );
+ Contracts::add_meta( $id, self::EXTENSION_SLUG, 'note', 'kept' );
Contracts::update(
$id,
array(
- 'status' => ContractStatus::ACTIVE,
- 'items' => array( array( 'item_name' => 'Coffee' ) ),
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'status' => ContractStatus::ACTIVE,
+ 'items' => array( array( 'item_name' => 'Coffee' ) ),
)
);
diff --git a/packages/php/woocommerce-subscriptions-engine/tests/integration/Api/PlansTest.php b/packages/php/woocommerce-subscriptions-engine/tests/integration/Api/PlansTest.php
index 2061cf8e354..690d0f3271c 100644
--- a/packages/php/woocommerce-subscriptions-engine/tests/integration/Api/PlansTest.php
+++ b/packages/php/woocommerce-subscriptions-engine/tests/integration/Api/PlansTest.php
@@ -547,6 +547,16 @@ class PlansTest extends EngineIntegrationTestCase {
)
);
$this->assertNull( Plans::update( $id, array( 'extension_slug' => 'other-extension' ) ), 'A fieldless update is scoped too.' );
+ $this->assertNull(
+ Plans::update(
+ $id,
+ array(
+ 'extension_slug' => strtoupper( self::OWNER ),
+ 'name' => 'Hijacked',
+ )
+ ),
+ 'The owner matches exactly: a case variant is another extension.'
+ );
$this->assertSame( 0, $validated, 'The validate action never sees a plan of another extension.' );
$this->assertSame( $before, $this->stored( $id )->to_storage() );
@@ -1038,9 +1048,9 @@ class PlansTest extends EngineIntegrationTestCase {
public function test_meta_methods_round_trip(): void {
$id = $this->create();
- $this->assertIsInt( Plans::add_meta( $id, 'note', 'one' ) );
- $this->assertNull( Plans::add_meta( $id, 'note', 'two', true ) );
- $this->assertTrue( Plans::update_meta( $id, 'flag', array( 'a' => 1 ) ) );
+ $this->assertIsInt( Plans::add_meta( $id, self::OWNER, 'note', 'one' ) );
+ $this->assertNull( Plans::add_meta( $id, self::OWNER, 'note', 'two', true ) );
+ $this->assertTrue( Plans::update_meta( $id, self::OWNER, 'flag', array( 'a' => 1 ) ) );
$this->assertSame( array( 'one' ), Plans::get_meta( $id, 'note' ) );
$this->assertSame( array( 'a' => 1 ), Plans::get_meta( $id, 'flag', true ) );
$this->assertSame(
@@ -1050,7 +1060,7 @@ class PlansTest extends EngineIntegrationTestCase {
),
Plans::get_meta( $id )
);
- $this->assertTrue( Plans::delete_meta( $id, 'note' ) );
+ $this->assertTrue( Plans::delete_meta( $id, self::OWNER, 'note' ) );
$this->assertSame( '', Plans::get_meta( $id, 'note', true ) );
}
@@ -1058,17 +1068,17 @@ class PlansTest extends EngineIntegrationTestCase {
* @testdox meta reads for a missing plan are empty.
*/
public function test_meta_reads_for_a_missing_plan_are_empty(): void {
- $this->assertFalse( Plans::delete_meta( 999999, 'note' ) );
+ $this->assertFalse( Plans::delete_meta( 999999, self::OWNER, 'note' ) );
$this->assertSame( '', Plans::get_meta( 999999, 'note', true ) );
$this->assertSame( array(), Plans::get_meta( 999999 ) );
}
/**
- * @testdox meta writes do not look up the plan.
+ * @testdox meta writes scope by the owner in the write statement itself, with no read first.
*/
- public function test_meta_writes_do_not_look_up_the_plan(): void {
+ public function test_meta_writes_scope_by_owner_in_the_write_statement(): void {
$id = $this->create();
- Plans::add_meta( $id, 'note', 'one' );
+ Plans::add_meta( $id, self::OWNER, 'flag', 'off' );
$queries = array();
$capture = static function ( $query ) use ( &$queries ) {
@@ -1077,39 +1087,52 @@ class PlansTest extends EngineIntegrationTestCase {
};
add_filter( 'query', $capture );
try {
- Plans::add_meta( $id, 'note', 'two' );
- Plans::update_meta( $id, 'flag', 'on' );
+ Plans::add_meta( $id, self::OWNER, 'note', 'one' );
+ Plans::update_meta( $id, self::OWNER, 'flag', 'on' );
+ Plans::delete_meta( $id, self::OWNER, 'note' );
} finally {
remove_filter( 'query', $capture );
}
- $plans_table = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLANS );
+ $this->assertCount( 3, $queries, 'One statement per write.' );
+ $this->assertStringStartsWith( 'INSERT INTO', $queries[0] );
+ $this->assertStringStartsWith( 'UPDATE', $queries[1] );
+ $this->assertStringStartsWith( 'DELETE', $queries[2] );
foreach ( $queries as $query ) {
- $this->assertDoesNotMatchRegularExpression( '/\\b' . preg_quote( $plans_table, '/' ) . '\\b/', $query, 'A meta write reads only the meta table.' );
+ $this->assertStringContainsString( 'p.extension_slug = ', $query, 'Each write matches the owner.' );
}
- $this->assertSame( array( 'one', 'two' ), Plans::get_meta( $id, 'note' ) );
$this->assertSame( 'on', Plans::get_meta( $id, 'flag', true ) );
}
/**
- * @testdox deleting a plan removes its meta.
+ * @testdox meta writes report an empty owner and an empty key together.
*/
- public function test_deleting_a_plan_removes_its_meta(): void {
+ public function test_meta_writes_report_owner_and_key_errors_together(): void {
$id = $this->create();
- Plans::add_meta( $id, 'note', 'one' );
-
- $this->assertTrue( ( new PlanRepository() )->delete( $id ) );
- $this->assertSame( array(), Plans::get_meta( $id, 'note' ) );
+ foreach ( array( 'add_meta', 'update_meta', 'delete_meta' ) as $method ) {
+ $this->assert_validation_errors(
+ static function () use ( $id, $method ): void {
+ Plans::{$method}( $id, '', '', 'x' );
+ },
+ array(
+ array( ValidationErrorCode::REQUIRED, 'extension_slug' ),
+ array( ValidationErrorCode::REQUIRED, 'key' ),
+ )
+ );
+ }
}
/**
- * @testdox an empty meta key is rejected.
+ * @testdox deleting a plan removes its meta.
*/
- public function test_an_empty_meta_key_is_rejected(): void {
- $this->expectException( InvalidArgumentException::class );
+ public function test_deleting_a_plan_removes_its_meta(): void {
+ $id = $this->create();
+ Plans::add_meta( $id, self::OWNER, 'note', 'one' );
- Plans::add_meta( $this->create(), '', 'x' );
+ $this->assertTrue( ( new PlanRepository() )->delete( $id ) );
+
+ $this->assertSame( array(), Plans::get_meta( $id, 'note' ) );
}
/**
diff --git a/packages/php/woocommerce-subscriptions-engine/tests/integration/Api/Rest/ContractsControllerTest.php b/packages/php/woocommerce-subscriptions-engine/tests/integration/Api/Rest/ContractsControllerTest.php
index 9f12cc4f407..ee81ac44c0d 100644
--- a/packages/php/woocommerce-subscriptions-engine/tests/integration/Api/Rest/ContractsControllerTest.php
+++ b/packages/php/woocommerce-subscriptions-engine/tests/integration/Api/Rest/ContractsControllerTest.php
@@ -619,7 +619,13 @@ class ContractsControllerTest extends EngineIntegrationTestCase {
array(
'callback' => function ( ContractView $contract, array $action_args ) use ( $action ): ?ContractView {
$this->calls[] = array( $action, $contract->get_id(), $action_args );
- return Contracts::update( $contract->get_id(), array( 'status' => 'on-hold' ) );
+ return Contracts::update(
+ $contract->get_id(),
+ array(
+ 'extension_slug' => self::EXTENSION_SLUG,
+ 'status' => 'on-hold',
+ )
+ );
},
'permission' => $permission,
) + $extra
diff --git a/packages/php/woocommerce-subscriptions-engine/tests/integration/EngineIntegrationTestCase.php b/packages/php/woocommerce-subscriptions-engine/tests/integration/EngineIntegrationTestCase.php
index bcc01a3ccdb..a57d625c988 100644
--- a/packages/php/woocommerce-subscriptions-engine/tests/integration/EngineIntegrationTestCase.php
+++ b/packages/php/woocommerce-subscriptions-engine/tests/integration/EngineIntegrationTestCase.php
@@ -72,18 +72,30 @@ abstract class EngineIntegrationTestCase extends WP_UnitTestCase {
}
/**
- * Take a contract's lock through the facade, failing the test when the race is lost.
+ * Take a contract's lock through the facade as its owner, failing the test when the race is lost.
*
* @param int $contract_id Contract id.
* @param string $operation Operation label.
*/
protected function take_lock( int $contract_id, string $operation ): ContractLock {
- $lock = Contracts::lock( $contract_id, $operation );
+ $lock = Contracts::lock( $contract_id, $this->get_extension_slug( $contract_id ), $operation );
$this->assertNotNull( $lock, 'Expected to take the lock.' );
return $lock;
}
+ /**
+ * The stored owning extension of a contract, failing the test when it does not exist.
+ *
+ * @param int $contract_id Contract id.
+ */
+ protected function get_extension_slug( int $contract_id ): string {
+ $contract = Contracts::get( $contract_id );
+ $this->assertNotNull( $contract, 'Expected the contract to exist.' );
+
+ return (string) $contract->get_extension_slug();
+ }
+
/**
* A lock's private state, read through reflection (tests only).
*
@@ -146,7 +158,7 @@ abstract class EngineIntegrationTestCase extends WP_UnitTestCase {
protected function add_cycle_locked( int $contract_id, array $args ): ?CycleView {
$lock = $this->take_lock( $contract_id, 'test' );
try {
- return Contracts::add_cycle( $contract_id, $args, $lock );
+ return Contracts::add_cycle( $contract_id, $args + array( 'extension_slug' => $this->get_extension_slug( $contract_id ) ), $lock );
} finally {
$lock->release();
}
@@ -159,9 +171,10 @@ abstract class EngineIntegrationTestCase extends WP_UnitTestCase {
* @param array<string, mixed> $args Transition fields.
*/
protected function transition_cycle_locked( int $cycle_id, array $args ): bool {
- $lock = $this->take_lock( $this->get_cycle_contract_id( $cycle_id ), 'test' );
+ $contract_id = $this->get_cycle_contract_id( $cycle_id );
+ $lock = $this->take_lock( $contract_id, 'test' );
try {
- return Contracts::transition_cycle( $cycle_id, $args, $lock );
+ return Contracts::transition_cycle( $cycle_id, $args + array( 'extension_slug' => $this->get_extension_slug( $contract_id ) ), $lock );
} finally {
$lock->release();
}
@@ -493,6 +506,7 @@ abstract class EngineIntegrationTestCase extends WP_UnitTestCase {
Contracts::add_cycle(
$id,
array(
+ 'extension_slug' => (string) $plan->get_extension_slug(),
'status' => CycleStatus::BILLED,
'count' => 1,
'order_id' => $order->get_id(),
@@ -503,7 +517,14 @@ abstract class EngineIntegrationTestCase extends WP_UnitTestCase {
),
$lock
);
- Contracts::update( $id, array( 'status' => $status ), $lock );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => (string) $plan->get_extension_slug(),
+ 'status' => $status,
+ ),
+ $lock
+ );
$lock->release();
return $id;
diff --git a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Dispatch/DueDispatcherTest.php b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Dispatch/DueDispatcherTest.php
index dcf4d19dd0b..9cad2fcad76 100644
--- a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Dispatch/DueDispatcherTest.php
+++ b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Dispatch/DueDispatcherTest.php
@@ -12,12 +12,14 @@ namespace Automattic\WooCommerce\SubscriptionsEngine\Tests\Integration\Integrati
use DateTimeImmutable;
use DateTimeZone;
use EngineIntegrationTestCase;
+use InvalidArgumentException;
use RuntimeException;
use Automattic\WooCommerce\SubscriptionsEngine\Api\ContractLock;
use Automattic\WooCommerce\SubscriptionsEngine\Api\ContractLockedException;
use Automattic\WooCommerce\SubscriptionsEngine\Api\Contracts;
use Automattic\WooCommerce\SubscriptionsEngine\Api\View\ContractView;
use Automattic\WooCommerce\SubscriptionsEngine\Core\Entity\ContractStatus;
+use Automattic\WooCommerce\SubscriptionsEngine\Core\Entity\CycleStatus;
use Automattic\WooCommerce\SubscriptionsEngine\Integration\Dispatch\DueDispatcher;
use Automattic\WooCommerce\SubscriptionsEngine\Integration\Ownership\ConsumerRegistry;
use Automattic\WooCommerce\SubscriptionsEngine\Integration\Storage\SchemaInstaller;
@@ -105,7 +107,14 @@ class DueDispatcherTest extends EngineIntegrationTestCase {
$nested = ( new DueDispatcher() )->run_batch( $this->now() );
}
if ( $moved === $contract->get_id() ) {
- Contracts::update( $moved, array( 'next_payment_gmt' => '2026-03-15 00:00:00' ), $context['lock'] );
+ Contracts::update(
+ $moved,
+ array(
+ 'extension_slug' => self::OWNER,
+ 'next_payment_gmt' => '2026-03-15 00:00:00',
+ ),
+ $context['lock']
+ );
}
},
)
@@ -116,6 +125,88 @@ class DueDispatcherTest extends EngineIntegrationTestCase {
$this->assertSame( array( $first, $idle, $moved, $idle ), $this->notified_ids(), 'Only the contract left due is notified again.' );
}
+ /**
+ * @testdox The handler's lock is taken for the scanned owner: a write naming that owner lands, one naming another is refused.
+ */
+ public function test_the_handler_lock_carries_the_scanned_owner(): void {
+ $id = $this->create_due_contract();
+ $refused = array();
+ ConsumerRegistry::register(
+ self::OWNER,
+ array(
+ 'contract_due_handler' => static function ( ContractView $contract, array $context ) use ( &$refused ): void {
+ $contract_id = (int) $contract->get_id();
+ $lock = $context['lock'];
+ $cycle = array(
+ 'status' => CycleStatus::PENDING,
+ 'starts_at_gmt' => '2026-02-15 00:00:00',
+ 'ends_at_gmt' => '2026-03-15 00:00:00',
+ 'currency' => 'USD',
+ );
+ $foreign = array(
+ 'update' => static function () use ( $contract_id, $lock ): void {
+ Contracts::update(
+ $contract_id,
+ array(
+ 'extension_slug' => 'other-ext',
+ 'next_payment_gmt' => null,
+ ),
+ $lock
+ );
+ },
+ 'add_cycle' => static function () use ( $contract_id, $cycle, $lock ): void {
+ Contracts::add_cycle( $contract_id, array( 'extension_slug' => 'other-ext' ) + $cycle, $lock );
+ },
+ );
+ foreach ( $foreign as $method => $write ) {
+ try {
+ $write();
+ } catch ( InvalidArgumentException $e ) {
+ $refused[ $method ] = $e->getMessage();
+ }
+ }
+
+ $added = Contracts::add_cycle( $contract_id, array( 'extension_slug' => self::OWNER ) + $cycle, $lock );
+ if ( null === $added ) {
+ return;
+ }
+ $transition = array(
+ 'from_status' => CycleStatus::PENDING,
+ 'to_status' => CycleStatus::BILLED,
+ );
+ try {
+ Contracts::transition_cycle( (int) $added->get_id(), array( 'extension_slug' => 'other-ext' ) + $transition, $lock );
+ } catch ( InvalidArgumentException $e ) {
+ $refused['transition_cycle'] = $e->getMessage();
+ }
+ Contracts::transition_cycle( (int) $added->get_id(), array( 'extension_slug' => self::OWNER ) + $transition, $lock );
+ Contracts::add_meta( $contract_id, self::OWNER, '_handled', 'yes', false, $lock );
+ Contracts::update(
+ $contract_id,
+ array(
+ 'extension_slug' => self::OWNER,
+ 'next_payment_gmt' => null,
+ ),
+ $lock
+ );
+ },
+ )
+ );
+
+ $this->assertSame( 1, $this->sut->run_batch( $this->now() ) );
+ $this->assertSame( array( 'update', 'add_cycle', 'transition_cycle' ), array_keys( $refused ) );
+ foreach ( $refused as $message ) {
+ $this->assertStringContainsString( 'another extension', $message );
+ }
+ $cycles = Contracts::get_cycles( $id );
+ $this->assertCount( 1, $cycles, 'Only the owner\'s cycle is added.' );
+ $this->assertSame( CycleStatus::BILLED, $cycles[0]->get_status(), 'Only the owner\'s transition lands.' );
+ $this->assertSame( array( 'yes' ), Contracts::get_meta( $id, '_handled' ) );
+ $view = Contracts::get( $id );
+ $this->assertInstanceOf( ContractView::class, $view );
+ $this->assertNull( $view->get_next_payment_gmt() );
+ }
+
/**
* @testdox A contract locked before the scan is not selected.
*/
@@ -147,7 +238,13 @@ class DueDispatcherTest extends EngineIntegrationTestCase {
);
if ( $first === $contract->get_id() ) {
// Other writers act on the next contracts after the scan read them.
- Contracts::update( $second, array( 'next_payment_gmt' => '2026-02-14 12:00:00' ) );
+ Contracts::update(
+ $second,
+ array(
+ 'extension_slug' => self::OWNER,
+ 'next_payment_gmt' => '2026-02-14 12:00:00',
+ )
+ );
$this->take_lock( $third, 'cancel' );
}
},
@@ -187,7 +284,14 @@ class DueDispatcherTest extends EngineIntegrationTestCase {
'contract' => $contract,
'context' => $context,
);
- Contracts::update( $contract->get_id(), array( 'next_payment_gmt' => '2026-02-16 00:00:00' ), $context['lock'] );
+ Contracts::update(
+ $contract->get_id(),
+ array(
+ 'extension_slug' => self::OWNER,
+ 'next_payment_gmt' => '2026-02-16 00:00:00',
+ ),
+ $context['lock']
+ );
},
)
);
@@ -214,7 +318,14 @@ class DueDispatcherTest extends EngineIntegrationTestCase {
'contract' => $contract,
'context' => $context,
);
- Contracts::update( $contract->get_id(), array( 'next_payment_gmt' => null ), $context['lock'] );
+ Contracts::update(
+ $contract->get_id(),
+ array(
+ 'extension_slug' => self::OWNER,
+ 'next_payment_gmt' => null,
+ ),
+ $context['lock']
+ );
},
)
);
@@ -241,11 +352,24 @@ class DueDispatcherTest extends EngineIntegrationTestCase {
$this->assertSame( DueDispatcher::LOCK_OPERATION, $lock->get_operation() );
try {
- Contracts::update( $contract->get_id(), array( 'status' => ContractStatus::CANCELLED ) );
+ Contracts::update(
+ $contract->get_id(),
+ array(
+ 'extension_slug' => self::OWNER,
+ 'status' => ContractStatus::CANCELLED,
+ )
+ );
} catch ( ContractLockedException $e ) {
$refused = $e->get_operation();
}
- Contracts::update( $contract->get_id(), array( 'status' => ContractStatus::ON_HOLD ), $lock );
+ Contracts::update(
+ $contract->get_id(),
+ array(
+ 'extension_slug' => self::OWNER,
+ 'status' => ContractStatus::ON_HOLD,
+ ),
+ $lock
+ );
},
)
);
diff --git a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ColumnFitTest.php b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ColumnFitTest.php
new file mode 100644
index 00000000000..584512bee7a
--- /dev/null
+++ b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ColumnFitTest.php
@@ -0,0 +1,81 @@
+<?php
+/**
+ * Integration tests for ColumnFit.
+ *
+ * @package Automattic\WooCommerce\SubscriptionsEngine
+ */
+
+declare( strict_types=1 );
+
+namespace Automattic\WooCommerce\SubscriptionsEngine\Tests\Integration\Integration\Storage;
+
+use EngineIntegrationTestCase;
+use RuntimeException;
+use WP_Error;
+use Automattic\WooCommerce\SubscriptionsEngine\Integration\Storage\ColumnFit;
+use Automattic\WooCommerce\SubscriptionsEngine\Integration\Storage\SchemaInstaller;
+
+/**
+ * @covers \Automattic\WooCommerce\SubscriptionsEngine\Integration\Storage\ColumnFit
+ */
+class ColumnFitTest extends EngineIntegrationTestCase {
+
+ /**
+ * The plan meta table, whose `meta_key` is VARCHAR(255).
+ *
+ * @var string
+ */
+ private $table;
+
+ public function setUp(): void {
+ parent::setUp();
+ $this->table = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLAN_META );
+ }
+
+ /**
+ * @testdox A value at the column length, counted in characters, fits.
+ */
+ public function test_a_value_at_the_column_length_fits(): void {
+ ColumnFit::assert_fits_column( $this->table, 'meta_key', str_repeat( 'é', 255 ) );
+ $this->addToAssertionCount( 1 );
+ }
+
+ /**
+ * @testdox A value longer than the column is refused with the column and its length named.
+ */
+ public function test_a_value_longer_than_the_column_is_refused(): void {
+ $this->expectException( RuntimeException::class );
+ $this->expectExceptionMessage( 'is longer than the column allows (255).' );
+
+ ColumnFit::assert_fits_column( $this->table, 'meta_key', str_repeat( 'k', 256 ) );
+ }
+
+ /**
+ * @testdox A value with bytes invalid for the column charset is refused.
+ */
+ public function test_a_value_invalid_for_the_charset_is_refused(): void {
+ $this->expectException( RuntimeException::class );
+ $this->expectExceptionMessage( 'has characters the column cannot store.' );
+
+ ColumnFit::assert_fits_column( $this->table, 'meta_key', "key\xC3\x28" );
+ }
+
+ /**
+ * @testdox A column that cannot be checked refuses the value.
+ */
+ public function test_a_column_that_cannot_be_checked_is_refused(): void {
+ $error = static function () {
+ return new WP_Error( 'wpdb_get_table_charset_failure', 'Could not retrieve table charset.' );
+ };
+ add_filter( 'pre_get_col_charset', $error );
+
+ try {
+ ColumnFit::assert_fits_column( $this->table, 'meta_key', 'short' );
+ $this->fail( 'Expected the unchecked column to refuse the value.' );
+ } catch ( RuntimeException $e ) {
+ $this->assertStringContainsString( 'could not be checked against the column', $e->getMessage() );
+ } finally {
+ remove_filter( 'pre_get_col_charset', $error );
+ }
+ }
+}
diff --git a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ContractLockTest.php b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ContractLockTest.php
index d4d90fc7886..c520e2c2cc8 100644
--- a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ContractLockTest.php
+++ b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ContractLockTest.php
@@ -29,6 +29,11 @@ use Automattic\WooCommerce\SubscriptionsEngine\Integration\Storage\SchemaInstall
*/
class ContractLockTest extends EngineIntegrationTestCase {
+ /**
+ * Owning extension of the fixture contracts.
+ */
+ private const OWNER = 'engine-tests';
+
/**
* The System Under Test.
*
@@ -45,7 +50,7 @@ class ContractLockTest extends EngineIntegrationTestCase {
return $this->insert_released(
Contract::create(
array(
- 'extension_slug' => 'engine-tests',
+ 'extension_slug' => self::OWNER,
'status' => ContractStatus::ACTIVE,
'next_payment_gmt' => $next_payment_gmt,
)
@@ -83,7 +88,7 @@ class ContractLockTest extends EngineIntegrationTestCase {
}
private function acquire( int $id, string $operation = 'hold' ): ContractLock {
- $lock = $this->sut->acquire_lock( $id, $operation );
+ $lock = $this->sut->acquire_lock( $id, self::OWNER, $operation );
$this->assertInstanceOf( ContractLock::class, $lock );
return $lock;
@@ -100,7 +105,7 @@ class ContractLockTest extends EngineIntegrationTestCase {
$this->assertSame( $id, $lock->get_contract_id() );
$this->assertSame( 'hold', $lock->get_operation() );
- $this->assertNull( $this->sut->acquire_lock( $id, 'cancel' ) );
+ $this->assertNull( $this->sut->acquire_lock( $id, self::OWNER, 'cancel' ) );
$this->assertSame( '2035-01-01 00:10:00', $this->lock_columns( $id )['locked_until_gmt'] );
$this->assertSame(
array(
@@ -120,7 +125,7 @@ class ContractLockTest extends EngineIntegrationTestCase {
$old = $this->acquire( $id );
$this->pin_db_clock( '2035-01-01 00:09:59' );
- $this->assertNull( $this->sut->acquire_lock( $id, 'cancel' ), 'Still live one second before expiry.' );
+ $this->assertNull( $this->sut->acquire_lock( $id, self::OWNER, 'cancel' ), 'Still live one second before expiry.' );
$this->pin_db_clock( '2035-01-01 00:10:00' );
$new = $this->acquire( $id, 'cancel' );
@@ -146,9 +151,9 @@ class ContractLockTest extends EngineIntegrationTestCase {
public function test_acquire_with_an_equality_condition(): void {
$id = $this->insert_contract( '2026-06-20 00:00:00' );
- $this->assertNull( $this->sut->acquire_lock( $id, 'engine_due_dispatch', array( 'next_payment_gmt' => '2026-06-15 00:00:00' ) ) );
+ $this->assertNull( $this->sut->acquire_lock( $id, self::OWNER, 'engine_due_dispatch', array( 'next_payment_gmt' => '2026-06-15 00:00:00' ) ) );
$this->assertNull( $this->get_lock_holder( $id )['operation'] );
- $this->assertInstanceOf( ContractLock::class, $this->sut->acquire_lock( $id, 'engine_due_dispatch', array( 'next_payment_gmt' => '2026-06-20 00:00:00' ) ) );
+ $this->assertInstanceOf( ContractLock::class, $this->sut->acquire_lock( $id, self::OWNER, 'engine_due_dispatch', array( 'next_payment_gmt' => '2026-06-20 00:00:00' ) ) );
}
/**
@@ -156,11 +161,11 @@ class ContractLockTest extends EngineIntegrationTestCase {
*/
public function test_acquire_with_a_null_condition(): void {
$set = $this->insert_contract( '2026-06-20 00:00:00' );
- $this->assertNull( $this->sut->acquire_lock( $set, 'engine_due_dispatch', array( 'next_payment_gmt' => null ) ) );
+ $this->assertNull( $this->sut->acquire_lock( $set, self::OWNER, 'engine_due_dispatch', array( 'next_payment_gmt' => null ) ) );
$this->assertNull( $this->get_lock_holder( $set )['operation'] );
$cleared = $this->insert_contract( null );
- $this->assertInstanceOf( ContractLock::class, $this->sut->acquire_lock( $cleared, 'engine_due_dispatch', array( 'next_payment_gmt' => null ) ) );
+ $this->assertInstanceOf( ContractLock::class, $this->sut->acquire_lock( $cleared, self::OWNER, 'engine_due_dispatch', array( 'next_payment_gmt' => null ) ) );
}
/**
@@ -172,6 +177,7 @@ class ContractLockTest extends EngineIntegrationTestCase {
$this->assertNull(
$this->sut->acquire_lock(
$id,
+ self::OWNER,
'engine_due_dispatch',
array(
'next_payment_gmt' => '2026-06-20 00:00:00',
@@ -184,6 +190,7 @@ class ContractLockTest extends EngineIntegrationTestCase {
ContractLock::class,
$this->sut->acquire_lock(
$id,
+ self::OWNER,
'engine_due_dispatch',
array(
'next_payment_gmt' => '2026-06-20 00:00:00',
@@ -201,7 +208,7 @@ class ContractLockTest extends EngineIntegrationTestCase {
foreach ( array( array( 'lock_token' => null ), array( 'customer_id' => 1 ), array( 'status' => array( ContractStatus::ACTIVE ) ) ) as $conditions ) {
try {
- $this->sut->acquire_lock( $id, 'engine_due_dispatch', $conditions );
+ $this->sut->acquire_lock( $id, self::OWNER, 'engine_due_dispatch', $conditions );
$this->fail( 'Expected an invalid condition to throw.' );
} catch ( InvalidArgumentException $e ) {
$this->assertNull( $this->get_lock_holder( $id )['operation'] );
@@ -209,11 +216,38 @@ class ContractLockTest extends EngineIntegrationTestCase {
}
}
+ /**
+ * @testdox A contract of another extension cannot be locked, and its lock columns are untouched.
+ */
+ public function test_a_foreign_contract_cannot_be_locked(): void {
+ $id = $this->insert_contract();
+ $before = $this->lock_columns( $id );
+
+ $this->assertNull( $this->sut->acquire_lock( $id, 'other-extension', 'hold' ) );
+ $this->assertSame( $before, $this->lock_columns( $id ) );
+ $this->assertNull( $this->sut->acquire_lock( $id, 'other-extension', 'engine_due_dispatch', array( 'next_payment_gmt' => '2026-06-15 00:00:00' ) ), 'A met condition does not lift the owner scope.' );
+ $this->assertInstanceOf( ContractLock::class, $this->sut->acquire_lock( $id, self::OWNER, 'hold' ) );
+ }
+
+ /**
+ * @testdox The internal lock reads a contract of another extension as missing, also while another operation holds its lock.
+ */
+ public function test_the_internal_lock_reads_a_foreign_contract_as_missing(): void {
+ $id = $this->insert_contract();
+
+ $this->assertNull( $this->sut->acquire_internal_lock( $id, 'other-extension' ) );
+ $this->assertInstanceOf( ContractLock::class, $this->sut->acquire_lock( $id, self::OWNER, 'hold' ) );
+ $this->assertNull( $this->sut->acquire_internal_lock( $id, 'other-extension' ), 'No ContractLockedException for a foreign row.' );
+
+ $this->expectException( ContractLockedException::class );
+ $this->sut->acquire_internal_lock( $id, self::OWNER );
+ }
+
/**
* @testdox An unknown contract cannot be locked and reads as missing.
*/
public function test_unknown_contract(): void {
- $this->assertNull( $this->sut->acquire_lock( 999999, 'hold' ) );
+ $this->assertNull( $this->sut->acquire_lock( 999999, self::OWNER, 'hold' ) );
$this->assertFalse( $this->get_lock_holder( 999999 )['exists'] );
}
@@ -237,15 +271,15 @@ class ContractLockTest extends EngineIntegrationTestCase {
foreach ( array( '', str_repeat( 'o', 65 ), 'Hold', 'hold now', '<b>hold</b>', "hold\n", 'hôld' ) as $operation ) {
try {
- $this->sut->acquire_lock( $id, $operation );
+ $this->sut->acquire_lock( $id, self::OWNER, $operation );
$this->fail( 'Expected an invalid label to be refused.' );
} catch ( InvalidArgumentException $e ) {
$this->assertNull( $this->get_lock_holder( $id )['operation'] );
}
}
- $this->assertInstanceOf( ContractLock::class, $this->sut->acquire_lock( $id, str_repeat( 'o', 64 ) ) );
- $this->assertInstanceOf( ContractLock::class, $this->sut->acquire_lock( $this->insert_contract(), 'renewal_retry-2' ) );
+ $this->assertInstanceOf( ContractLock::class, $this->sut->acquire_lock( $id, self::OWNER, str_repeat( 'o', 64 ) ) );
+ $this->assertInstanceOf( ContractLock::class, $this->sut->acquire_lock( $this->insert_contract(), self::OWNER, 'renewal_retry-2' ) );
}
/**
@@ -269,7 +303,7 @@ class ContractLockTest extends EngineIntegrationTestCase {
$this->lock_columns( $id )
);
$this->assertTrue( $this->lock_state( $lock )->is_released() );
- $this->assertInstanceOf( ContractLock::class, $this->sut->acquire_lock( $id, 'cancel' ), 'A released contract can be locked again at once.' );
+ $this->assertInstanceOf( ContractLock::class, $this->sut->acquire_lock( $id, self::OWNER, 'cancel' ), 'A released contract can be locked again at once.' );
}
/**
@@ -413,7 +447,7 @@ class ContractLockTest extends EngineIntegrationTestCase {
$this->age_lock_trust( $old );
try {
- $this->sut->add_meta( $id, '_stale', 'x', false, $old );
+ $this->sut->add_meta( $id, self::OWNER, '_stale', 'x', false, $old );
$this->fail( 'Expected the taken-over lock to be refused.' );
} catch ( ContractLockedException $e ) {
$this->assertSame( 'cancel', $e->get_operation() );
@@ -428,7 +462,7 @@ class ContractLockTest extends EngineIntegrationTestCase {
public function test_a_lock_built_outside_the_repository_is_doing_it_wrong(): void {
$this->setExpectedIncorrectUsage( ContractLock::class . '::__construct' );
- $lock = new ContractLock( 7, 'hold', str_repeat( 'a', 32 ) );
+ $lock = new ContractLock( 7, self::OWNER, 'hold', str_repeat( 'a', 32 ) );
$this->assertSame( 7, $lock->get_contract_id() );
}
@@ -461,9 +495,9 @@ class ContractLockTest extends EngineIntegrationTestCase {
$this->assertInstanceOf( Contract::class, $contract );
$contract->set_status( ContractStatus::ON_HOLD );
$this->assertTrue( $this->sut->update_fields( $contract, array( 'status' ), $lock ) );
- $this->assertIsInt( $this->sut->add_meta( $id, 'note', 'x', false, $lock ) );
+ $this->assertIsInt( $this->sut->add_meta( $id, self::OWNER, 'note', 'x', false, $lock ) );
$lock->release();
- $this->assertNotNull( $this->sut->insert( Contract::create( array( 'extension_slug' => 'engine-tests' ) ), 'checkout' ) );
+ $this->assertNotNull( $this->sut->insert( Contract::create( array( 'extension_slug' => self::OWNER ) ), 'checkout' ) );
} finally {
remove_action( 'doing_it_wrong_run', $count );
}
@@ -480,11 +514,11 @@ class ContractLockTest extends EngineIntegrationTestCase {
$this->acquire( $id );
$this->setExpectedIncorrectUsage( ContractLock::class . '::__construct' );
- $forged = new ContractLock( $id, 'forged', str_repeat( 'f', 32 ) );
+ $forged = new ContractLock( $id, self::OWNER, 'forged', str_repeat( 'f', 32 ) );
$this->age_lock_trust( $forged );
try {
- $this->sut->add_meta( $id, '_forged', 'x', false, $forged );
+ $this->sut->add_meta( $id, self::OWNER, '_forged', 'x', false, $forged );
$this->fail( 'Expected the forged lock to be refused.' );
} catch ( ContractLockedException $e ) {
$this->assertSame( 'hold', $e->get_operation() );
@@ -501,7 +535,13 @@ class ContractLockTest extends EngineIntegrationTestCase {
$this->refuse_next_write( 'UPDATE `' . SchemaInstaller::get_table_name( SchemaInstaller::TABLE_CONTRACTS ) . '` SET' );
try {
- Contracts::update( $id, array( 'status' => ContractStatus::ON_HOLD ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => self::OWNER,
+ 'status' => ContractStatus::ON_HOLD,
+ )
+ );
$this->fail( 'Expected the refused write to throw.' );
} catch ( ContractLockedException $e ) {
$this->assertSame( 'engine_contract_update', $e->get_operation() );
@@ -523,28 +563,28 @@ class ContractLockTest extends EngineIntegrationTestCase {
};
add_filter( 'query', $count );
try {
- $this->assertIsInt( $this->sut->add_meta( $id, '_note', 'a' ) );
- $this->assertTrue( $this->sut->update_meta( $id, '_note', 'b' ) );
- $this->assertFalse( $this->sut->update_meta( $id, '_note', 'b' ), 'The stored value: nothing to change.' );
- $this->assertTrue( $this->sut->delete_meta( $id, '_note' ) );
- $this->assertFalse( $this->sut->delete_meta( $id, '_absent' ) );
+ $this->assertIsInt( $this->sut->add_meta( $id, self::OWNER, '_note', 'a' ) );
+ $this->assertTrue( $this->sut->update_meta( $id, self::OWNER, '_note', 'b' ) );
+ $this->assertFalse( $this->sut->update_meta( $id, self::OWNER, '_note', 'b' ), 'The stored value: nothing to change.' );
+ $this->assertTrue( $this->sut->delete_meta( $id, self::OWNER, '_note' ) );
+ $this->assertFalse( $this->sut->delete_meta( $id, self::OWNER, '_absent' ) );
} finally {
remove_filter( 'query', $count );
}
$this->assertSame( 5, $acquires, 'Each write takes the internal lock.' );
$this->assertNull( $this->get_lock_holder( $id )['operation'], 'Each write releases it.' );
- $this->sut->add_meta( $id, '_anchor', 'a' );
+ $this->sut->add_meta( $id, self::OWNER, '_anchor', 'a' );
$this->acquire( $id, 'renewal' );
$writes = array(
'add_meta' => function () use ( $id ): void {
- $this->sut->add_meta( $id, '_anchor', 'b' );
+ $this->sut->add_meta( $id, self::OWNER, '_anchor', 'b' );
},
'update_meta' => function () use ( $id ): void {
- $this->sut->update_meta( $id, '_anchor', 'b' );
+ $this->sut->update_meta( $id, self::OWNER, '_anchor', 'b' );
},
'delete_meta' => function () use ( $id ): void {
- $this->sut->delete_meta( $id, '_anchor' );
+ $this->sut->delete_meta( $id, self::OWNER, '_anchor' );
},
);
foreach ( $writes as $name => $write ) {
@@ -557,10 +597,10 @@ class ContractLockTest extends EngineIntegrationTestCase {
}
$this->assertSame( array( 'a' ), $this->sut->get_meta( $id, '_anchor' ) );
- $this->assertFalse( $this->sut->delete_meta( 999999, '_anchor' ), 'A delete on a missing contract deletes nothing.' );
+ $this->assertFalse( $this->sut->delete_meta( 999999, self::OWNER, '_anchor' ), 'A delete on a missing contract deletes nothing.' );
foreach ( array( 'add_meta', 'update_meta' ) as $method ) {
try {
- $this->sut->$method( 999999, '_anchor', 'x' );
+ $this->sut->$method( 999999, self::OWNER, '_anchor', 'x' );
$this->fail( "Expected {$method} on a missing contract to throw." );
} catch ( InvalidArgumentException $e ) {
$this->assertStringContainsString( '999999', $e->getMessage() );
@@ -581,10 +621,10 @@ class ContractLockTest extends EngineIntegrationTestCase {
$writes = array(
'update_meta' => function () use ( $id, $lock ): void {
- $this->sut->update_meta( $id, '_note', 'x', null, $lock );
+ $this->sut->update_meta( $id, self::OWNER, '_note', 'x', null, $lock );
},
'delete_meta' => function () use ( $id, $lock ): void {
- $this->sut->delete_meta( $id, '_note', null, $lock );
+ $this->sut->delete_meta( $id, self::OWNER, '_note', null, $lock );
},
'replace_items' => function () use ( $id, $lock ): void {
$this->call_private( $this->sut, 'replace_items', $id, array( array( 'item_name' => 'Changed' ) ), $lock );
diff --git a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ContractMetaTest.php b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ContractMetaTest.php
index c0ae4f060d4..16febf3e694 100644
--- a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ContractMetaTest.php
+++ b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ContractMetaTest.php
@@ -24,6 +24,11 @@ use Automattic\WooCommerce\SubscriptionsEngine\Integration\Storage\SchemaInstall
*/
class ContractMetaTest extends EngineIntegrationTestCase {
+ /**
+ * Owning extension of the fixture contracts.
+ */
+ private const OWNER = 'acme-subs';
+
/**
* The System Under Test.
*
@@ -41,12 +46,12 @@ class ContractMetaTest extends EngineIntegrationTestCase {
public function setUp(): void {
parent::setUp();
$this->sut = new ContractRepository();
- $this->id = $this->insert_released( Contract::create( array( 'extension_slug' => 'acme-subs' ) ) );
+ $this->id = $this->insert_released( Contract::create( array( 'extension_slug' => self::OWNER ) ) );
}
public function test_add_meta_keeps_every_value_under_one_key_in_order(): void {
- $first = $this->sut->add_meta( $this->id, 'note', 'one' );
- $second = $this->sut->add_meta( $this->id, 'note', 'two' );
+ $first = $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+ $second = $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
$this->assertIsInt( $first );
$this->assertIsInt( $second );
@@ -56,9 +61,9 @@ class ContractMetaTest extends EngineIntegrationTestCase {
}
public function test_unique_add_on_an_existing_key_adds_nothing(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
- $this->assertNull( $this->sut->add_meta( $this->id, 'note', 'two', true ) );
+ $this->assertNull( $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two', true ) );
$this->assertSame( array( 'one' ), $this->sut->get_meta( $this->id, 'note' ) );
}
@@ -68,24 +73,24 @@ class ContractMetaTest extends EngineIntegrationTestCase {
public function test_unique_add_is_idempotent(): void {
global $wpdb;
- $this->assertIsInt( $this->sut->add_meta( $this->id, 'note', 'one', true ) );
- $this->assertNull( $this->sut->add_meta( $this->id, 'note', 'two', true ) );
+ $this->assertIsInt( $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one', true ) );
+ $this->assertNull( $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two', true ) );
$lock = $this->lock();
- $this->assertNull( $this->sut->add_meta( $this->id, 'note', 'three', true, $lock ) );
+ $this->assertNull( $this->sut->add_meta( $this->id, self::OWNER, 'note', 'three', true, $lock ) );
$lock->release();
- $this->assertIsInt( $this->sut->add_meta( $this->id, 'fresh', 'x', true ) );
+ $this->assertIsInt( $this->sut->add_meta( $this->id, self::OWNER, 'fresh', 'x', true ) );
$queries = $wpdb->num_queries;
- $this->assertIsInt( $this->sut->add_meta( $this->id, 'single', 'x', true ) );
+ $this->assertIsInt( $this->sut->add_meta( $this->id, self::OWNER, 'single', 'x', true ) );
$this->assertSame( 4, $wpdb->num_queries - $queries, 'Unlocked: the internal lock acquire, the key read, the insert, the release.' );
$lock = $this->lock();
$queries = $wpdb->num_queries;
$statements = $this->statements_during(
function () use ( $lock ): void {
- $this->assertIsInt( $this->sut->add_meta( $this->id, 'single-locked', 'x', true, $lock ) );
+ $this->assertIsInt( $this->sut->add_meta( $this->id, self::OWNER, 'single-locked', 'x', true, $lock ) );
}
);
$this->assertSame( 2, $wpdb->num_queries - $queries, 'With a trusted lock: the key read, then the insert.' );
@@ -99,12 +104,12 @@ class ContractMetaTest extends EngineIntegrationTestCase {
* @testdox update_meta on an absent key adds once; a repeat updates instead of duplicating.
*/
public function test_update_meta_does_not_duplicate(): void {
- $this->assertTrue( $this->sut->update_meta( $this->id, 'note', 'one' ) );
- $this->assertTrue( $this->sut->update_meta( $this->id, 'note', 'two' ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'one' ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'two' ) );
$lock = $this->lock();
- $this->assertTrue( $this->sut->update_meta( $this->id, 'locked', 'one', null, $lock ) );
- $this->assertTrue( $this->sut->update_meta( $this->id, 'locked', 'two', null, $lock ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'locked', 'one', null, $lock ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'locked', 'two', null, $lock ) );
$lock->release();
$this->assertSame( array( 'two' ), $this->sut->get_meta( $this->id, 'note' ) );
@@ -115,7 +120,7 @@ class ContractMetaTest extends EngineIntegrationTestCase {
* @testdox A failed meta read throws instead of reading as an absent key.
*/
public function test_a_failed_meta_read_throws(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
$this->fail_next_query( '/^\s*SELECT\s+meta_key,\s*meta_value/i' );
@@ -124,25 +129,25 @@ class ContractMetaTest extends EngineIntegrationTestCase {
}
public function test_update_meta_adds_when_absent(): void {
- $this->assertTrue( $this->sut->update_meta( $this->id, 'note', 'one' ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'one' ) );
$this->assertSame( array( 'one' ), $this->sut->get_meta( $this->id, 'note' ) );
}
public function test_update_meta_rewrites_every_row_for_the_key(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
- $this->sut->add_meta( $this->id, 'note', 'two' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
- $this->assertTrue( $this->sut->update_meta( $this->id, 'note', 'three' ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'three' ) );
$this->assertSame( array( 'three', 'three' ), $this->sut->get_meta( $this->id, 'note' ) );
}
public function test_update_meta_with_a_previous_value_rewrites_only_matching_rows(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
- $this->sut->add_meta( $this->id, 'note', 'two' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
- $this->assertTrue( $this->sut->update_meta( $this->id, 'note', 'three', 'two' ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'three', 'two' ) );
$this->assertSame( array( 'one', 'three' ), $this->sut->get_meta( $this->id, 'note' ) );
- $this->assertFalse( $this->sut->update_meta( $this->id, 'note', 'four', 'missing' ) );
+ $this->assertFalse( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'four', 'missing' ) );
}
/**
@@ -151,15 +156,15 @@ class ContractMetaTest extends EngineIntegrationTestCase {
public function test_update_meta_on_an_existing_key_is_one_statement(): void {
global $wpdb;
- $this->sut->add_meta( $this->id, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
$queries = $wpdb->num_queries;
- $this->assertTrue( $this->sut->update_meta( $this->id, 'note', 'two' ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'two' ) );
$this->assertSame( 3, $wpdb->num_queries - $queries, 'Unlocked: the internal lock acquire, the update, the release.' );
$lock = $this->lock();
$queries = $wpdb->num_queries;
- $this->assertTrue( $this->sut->update_meta( $this->id, 'note', 'three', null, $lock ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'three', null, $lock ) );
$this->assertSame( 1, $wpdb->num_queries - $queries, 'With a trusted lock.' );
$lock->release();
@@ -167,34 +172,34 @@ class ContractMetaTest extends EngineIntegrationTestCase {
}
public function test_update_meta_to_the_same_value_reports_no_change(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
- $this->assertFalse( $this->sut->update_meta( $this->id, 'note', 'one' ) );
+ $this->assertFalse( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'one' ) );
}
public function test_delete_meta_with_a_value_removes_only_that_row(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
- $this->sut->add_meta( $this->id, 'note', 'two' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
- $this->assertTrue( $this->sut->delete_meta( $this->id, 'note', 'one' ) );
+ $this->assertTrue( $this->sut->delete_meta( $this->id, self::OWNER, 'note', 'one' ) );
$this->assertSame( array( 'two' ), $this->sut->get_meta( $this->id, 'note' ) );
}
public function test_delete_meta_without_a_value_removes_every_row_for_the_key(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
- $this->sut->add_meta( $this->id, 'note', 'two' );
- $this->sut->add_meta( $this->id, 'other', 'kept' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'other', 'kept' );
- $this->assertTrue( $this->sut->delete_meta( $this->id, 'note' ) );
+ $this->assertTrue( $this->sut->delete_meta( $this->id, self::OWNER, 'note' ) );
$this->assertSame( array(), $this->sut->get_meta( $this->id, 'note' ) );
$this->assertSame( array( 'kept' ), $this->sut->get_meta( $this->id, 'other' ) );
- $this->assertFalse( $this->sut->delete_meta( $this->id, 'note' ) );
+ $this->assertFalse( $this->sut->delete_meta( $this->id, self::OWNER, 'note' ) );
}
public function test_get_meta_without_a_key_groups_every_key(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
- $this->sut->add_meta( $this->id, 'flag', 'yes' );
- $this->sut->add_meta( $this->id, 'note', 'two' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'flag', 'yes' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
$this->assertSame(
array(
@@ -211,10 +216,10 @@ class ContractMetaTest extends EngineIntegrationTestCase {
'b' => array( 'c' ),
);
- $this->sut->add_meta( $this->id, 'payload', $value );
+ $this->sut->add_meta( $this->id, self::OWNER, 'payload', $value );
$this->assertSame( $value, $this->sut->get_meta( $this->id, 'payload', true ) );
- $this->assertTrue( $this->sut->delete_meta( $this->id, 'payload', $value ) );
+ $this->assertTrue( $this->sut->delete_meta( $this->id, self::OWNER, 'payload', $value ) );
}
public function test_missing_key_reads_as_empty(): void {
@@ -224,45 +229,23 @@ class ContractMetaTest extends EngineIntegrationTestCase {
}
public function test_meta_is_scoped_to_its_contract(): void {
- $other = $this->insert_released( Contract::create( array( 'extension_slug' => 'acme-subs' ) ) );
- $this->sut->add_meta( $other, 'note', 'theirs' );
+ $other = $this->insert_released( Contract::create( array( 'extension_slug' => self::OWNER ) ) );
+ $this->sut->add_meta( $other, self::OWNER, 'note', 'theirs' );
$this->assertSame( array(), $this->sut->get_meta( $this->id, 'note' ) );
}
- /**
- * @dataProvider provide_write_methods
- *
- * @param string $method Write method name.
- */
- public function test_an_empty_key_is_rejected_on_writes( string $method ): void {
- $this->expectException( InvalidArgumentException::class );
-
- $this->sut->{$method}( $this->id, '', 'value' );
- }
-
- /**
- * @return array<string, array{0: string}>
- */
- public function provide_write_methods(): array {
- return array(
- 'add' => array( 'add_meta' ),
- 'update' => array( 'update_meta' ),
- 'delete' => array( 'delete_meta' ),
- );
- }
-
/**
* @testdox Each meta write lands with the live lock.
*/
public function test_meta_writes_with_the_lock_land(): void {
$lock = $this->lock();
- $this->assertIsInt( $this->sut->add_meta( $this->id, 'note', 'one', false, $lock ) );
- $this->assertTrue( $this->sut->update_meta( $this->id, 'note', 'two', null, $lock ) );
- $this->assertTrue( $this->sut->update_meta( $this->id, 'other', 'added', null, $lock ) );
+ $this->assertIsInt( $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one', false, $lock ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'two', null, $lock ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'other', 'added', null, $lock ) );
$this->assertSame( array( 'two' ), $this->sut->get_meta( $this->id, 'note' ) );
- $this->assertTrue( $this->sut->delete_meta( $this->id, 'note', null, $lock ) );
+ $this->assertTrue( $this->sut->delete_meta( $this->id, self::OWNER, 'note', null, $lock ) );
$this->assertSame( array(), $this->sut->get_meta( $this->id, 'note' ) );
}
@@ -270,21 +253,21 @@ class ContractMetaTest extends EngineIntegrationTestCase {
* @testdox Unlocked meta writes are refused while another lock lives and land after release.
*/
public function test_unlocked_meta_writes_are_refused_while_locked(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
$lock = $this->lock();
$writes = array(
'add' => function (): void {
- $this->sut->add_meta( $this->id, 'note', 'two' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
},
'update' => function (): void {
- $this->sut->update_meta( $this->id, 'note', 'two' );
+ $this->sut->update_meta( $this->id, self::OWNER, 'note', 'two' );
},
'add via update' => function (): void {
- $this->sut->update_meta( $this->id, 'fresh', 'two' );
+ $this->sut->update_meta( $this->id, self::OWNER, 'fresh', 'two' );
},
'delete' => function (): void {
- $this->sut->delete_meta( $this->id, 'note' );
+ $this->sut->delete_meta( $this->id, self::OWNER, 'note' );
},
);
@@ -299,9 +282,9 @@ class ContractMetaTest extends EngineIntegrationTestCase {
$this->assertSame( array( 'note' => array( 'one' ) ), $this->sut->get_meta( $this->id ), 'Nothing was written.' );
$lock->release();
- $this->assertTrue( $this->sut->update_meta( $this->id, 'note', 'two' ) );
- $this->assertIsInt( $this->sut->add_meta( $this->id, 'note', 'three' ) );
- $this->assertTrue( $this->sut->delete_meta( $this->id, 'note', 'two' ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'two' ) );
+ $this->assertIsInt( $this->sut->add_meta( $this->id, self::OWNER, 'note', 'three' ) );
+ $this->assertTrue( $this->sut->delete_meta( $this->id, self::OWNER, 'note', 'two' ) );
$this->assertSame( array( 'three' ), $this->sut->get_meta( $this->id, 'note' ) );
}
@@ -311,25 +294,42 @@ class ContractMetaTest extends EngineIntegrationTestCase {
public function test_a_meta_add_for_an_unknown_contract_throws(): void {
foreach ( array( 'add_meta', 'update_meta' ) as $method ) {
try {
- $this->sut->{$method}( 999999, 'note', 'orphan' );
+ $this->sut->{$method}( 999999, self::OWNER, 'note', 'orphan' );
$this->fail( "Expected {$method}() to refuse an unknown contract." );
} catch ( InvalidArgumentException $e ) {
$this->assertSame( array(), $this->sut->get_meta( 999999 ) );
}
}
- $this->assertFalse( $this->sut->delete_meta( 999999, 'note' ) );
+ $this->assertFalse( $this->sut->delete_meta( 999999, self::OWNER, 'note' ) );
}
/**
- * @testdox A meta write with a lock for another contract is refused.
+ * @testdox A meta write with a lock for another contract of another extension is refused for the contract first.
*/
public function test_a_meta_write_with_another_contracts_lock_is_refused(): void {
- $other = $this->insert_released( Contract::create( array( 'extension_slug' => 'acme-subs' ) ) );
- $lock = $this->sut->acquire_lock( $other, 'hold' );
+ $other = $this->insert_released( Contract::create( array( 'extension_slug' => 'another-extension' ) ) );
+ $lock = $this->sut->acquire_lock( $other, 'another-extension', 'hold' );
$this->assertInstanceOf( ContractLock::class, $lock );
- $this->expectException( InvalidArgumentException::class );
- $this->sut->add_meta( $this->id, 'note', 'one', false, $lock );
+ $writes = array(
+ 'add_meta' => function () use ( $lock ): void {
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one', false, $lock );
+ },
+ 'update_meta' => function () use ( $lock ): void {
+ $this->sut->update_meta( $this->id, self::OWNER, 'note', 'one', null, $lock );
+ },
+ 'delete_meta' => function () use ( $lock ): void {
+ $this->sut->delete_meta( $this->id, self::OWNER, 'note', null, $lock );
+ },
+ );
+ foreach ( $writes as $method => $write ) {
+ try {
+ $write();
+ $this->fail( "Expected {$method}() to refuse the lock." );
+ } catch ( InvalidArgumentException $e ) {
+ $this->assertStringContainsString( "The lock belongs to contract {$other}, not contract {$this->id}.", $e->getMessage(), $method );
+ }
+ }
}
/**
@@ -338,7 +338,7 @@ class ContractMetaTest extends EngineIntegrationTestCase {
public function test_a_failed_meta_write_throws(): void {
global $wpdb;
- $this->sut->add_meta( $this->id, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
$break = static function ( string $query ): string {
return 1 === preg_match( '/^\s*(UPDATE|DELETE)\s/i', $query ) && false !== strpos( $query, 'contract_meta' ) ? 'SELECT * FROM table_that_does_not_exist_for_tests' : $query;
@@ -348,7 +348,7 @@ class ContractMetaTest extends EngineIntegrationTestCase {
try {
foreach ( array( 'update_meta', 'delete_meta' ) as $method ) {
try {
- $this->sut->{$method}( $this->id, 'note', 'two' );
+ $this->sut->{$method}( $this->id, self::OWNER, 'note', 'two' );
$this->fail( "Expected {$method}() to throw." );
} catch ( RuntimeException $e ) {
$this->assertStringContainsString( 'contract meta', $e->getMessage() );
@@ -360,8 +360,34 @@ class ContractMetaTest extends EngineIntegrationTestCase {
}
}
+ /**
+ * @testdox A meta key longer than the 255-character column is refused and stores nothing, in ASCII or multibyte characters; a 255-character key is stored.
+ */
+ public function test_an_over_long_meta_key_is_refused(): void {
+ foreach ( array( 'k', 'é' ) as $char ) {
+ $too_long = str_repeat( $char, 256 );
+ foreach ( array( 'add_meta', 'update_meta' ) as $method ) {
+ try {
+ $this->sut->{$method}( $this->id, self::OWNER, $too_long, 'value' );
+ $this->fail( "Expected {$method}() to refuse an over-long key of '{$char}'." );
+ } catch ( RuntimeException $e ) {
+ // The message tells this refusal apart from a ContractLockedException, also a RuntimeException.
+ $this->assertStringContainsString( 'is longer than the column allows', $e->getMessage(), $method );
+ }
+ }
+ $this->assertSame( array(), $this->sut->get_meta( $this->id ) );
+
+ $longest = str_repeat( $char, 255 );
+ $this->assertIsInt( $this->sut->add_meta( $this->id, self::OWNER, $longest, 'one' ) );
+ $this->sut->delete_meta( $this->id, self::OWNER, $longest );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, $longest, 'two' ) );
+ $this->assertSame( array( $longest => array( 'two' ) ), $this->sut->get_meta( $this->id ) );
+ $this->sut->delete_meta( $this->id, self::OWNER, $longest );
+ }
+ }
+
private function lock(): ContractLock {
- $lock = $this->sut->acquire_lock( $this->id, 'hold' );
+ $lock = $this->sut->acquire_lock( $this->id, self::OWNER, 'hold' );
$this->assertInstanceOf( ContractLock::class, $lock );
return $lock;
diff --git a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ContractRepositoryTest.php b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ContractRepositoryTest.php
index 114a8192af9..78c7db3955f 100644
--- a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ContractRepositoryTest.php
+++ b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/ContractRepositoryTest.php
@@ -302,7 +302,7 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
*/
public function test_meta_survives_a_whole_entity_update(): void {
$id = $this->insert_released( $this->make_contract() );
- $this->sut->add_meta( $id, 'source_channel', 'pdp' );
+ $this->sut->add_meta( $id, $this->get_extension_slug( $id ), 'source_channel', 'pdp' );
$contract = $this->sut->find( $id );
$this->assertInstanceOf( Contract::class, $contract );
@@ -902,7 +902,15 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
$id = $this->insert_released( $this->make_contract() );
for ( $i = 0; $i < 2; $i++ ) {
- $this->assertNotNull( Contracts::update( $id, array( 'status' => ContractStatus::ACTIVE ) ) );
+ $this->assertNotNull(
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => $this->get_extension_slug( $id ),
+ 'status' => ContractStatus::ACTIVE,
+ )
+ )
+ );
}
$stored = $this->sut->find( $id );
@@ -1246,7 +1254,7 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
*/
public function test_a_lost_contract_write_changes_neither_status_nor_due_moment(): void {
$id = $this->insert_contract_due_at( '2026-06-15 00:00:00', ContractStatus::ACTIVE );
- $lock = $this->sut->acquire_lock( $id, 'test' );
+ $lock = $this->sut->acquire_lock( $id, $this->get_extension_slug( $id ), 'test' );
$this->assertNotNull( $lock );
$contract = $this->sut->find( $id );
@@ -1280,7 +1288,7 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
*/
public function test_due_moment_writes_leave_the_lock_alone(): void {
$id = $this->insert_contract_due_at( '2026-06-15 00:00:00', ContractStatus::ACTIVE );
- $lock = $this->sut->acquire_lock( $id, 'engine_due_dispatch' );
+ $lock = $this->sut->acquire_lock( $id, $this->get_extension_slug( $id ), 'engine_due_dispatch' );
$this->assertNotNull( $lock );
$before = $this->get_lock_holder( $id );
@@ -1968,7 +1976,7 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
* @param string $operation Operation label.
*/
private function lock( int $contract_id, string $operation = 'test' ): ContractLock {
- $lock = $this->sut->acquire_lock( $contract_id, $operation );
+ $lock = $this->sut->acquire_lock( $contract_id, $this->get_extension_slug( $contract_id ), $operation );
$this->assertInstanceOf( ContractLock::class, $lock );
return $lock;
@@ -2057,7 +2065,13 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
$this->assertSame( ContractStatus::ON_HOLD, $this->stored_status( $id ) );
try {
- Contracts::update( $id, array( 'status' => ContractStatus::CANCELLED ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => $this->get_extension_slug( $id ),
+ 'status' => ContractStatus::CANCELLED,
+ )
+ );
$this->fail( 'Expected an unlocked write to be refused while the lock lives.' );
} catch ( ContractLockedException $e ) {
$this->assertSame( 'hold', $e->get_operation() );
@@ -2065,7 +2079,15 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
$this->assertSame( ContractStatus::ON_HOLD, $this->stored_status( $id ) );
$this->pin_db_clock( '2035-01-01 00:10:00' );
- $this->assertNotNull( Contracts::update( $id, array( 'status' => ContractStatus::CANCELLED ) ) );
+ $this->assertNotNull(
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => $this->get_extension_slug( $id ),
+ 'status' => ContractStatus::CANCELLED,
+ )
+ )
+ );
$this->assertSame( ContractStatus::CANCELLED, $this->stored_status( $id ) );
try {
@@ -2115,7 +2137,16 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
$this->sut->delete( $id );
$this->assertFalse( $this->sut->update_fields( $contract, array( 'status' ), $lock ) );
- $this->assertNull( Contracts::update( $id, array( 'status' => ContractStatus::ON_HOLD ) ), 'Unlocked, the update reads the contract as missing.' );
+ $this->assertNull(
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => 'lite',
+ 'status' => ContractStatus::ON_HOLD,
+ )
+ ),
+ 'Unlocked, the update reads the contract as missing.'
+ );
}
/**
@@ -2127,8 +2158,9 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
$lock = $this->lock( $id, 'hold' );
$args = array(
- 'status' => ContractStatus::ON_HOLD,
- 'items' => $this->two_items(),
+ 'extension_slug' => $this->get_extension_slug( $id ),
+ 'status' => ContractStatus::ON_HOLD,
+ 'items' => $this->two_items(),
);
try {
Contracts::update( $id, $args );
@@ -2201,7 +2233,7 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
$this->sut->delete( $id );
try {
- $this->sut->add_meta( $id, 'note', 'x', true, $lock );
+ $this->sut->add_meta( $id, 'lite', 'note', 'x', true, $lock );
$this->fail( 'Expected the add to a missing contract to throw.' );
} catch ( \InvalidArgumentException $e ) {
$this->assertStringContainsString( (string) $id, $e->getMessage() );
@@ -2452,7 +2484,16 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
array( 'contract_id' => $id ),
function () use ( $id, $release ): void {
$this->fail_next_query( $release );
- $this->assertNotNull( Contracts::update( $id, array( 'items' => $this->two_items() ) ), 'The landed write is reported.' );
+ $this->assertNotNull(
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => $this->get_extension_slug( $id ),
+ 'items' => $this->two_items(),
+ )
+ ),
+ 'The landed write is reported.'
+ );
}
);
$this->assertNotSame( array(), $logs, 'The failed release is logged.' );
@@ -2466,7 +2507,13 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
$this->fail_next_query( '/^\s*INSERT\s+INTO\s+\S*contract_items/i' );
$this->fail_next_query( $release );
try {
- Contracts::update( $id, array( 'items' => $this->two_items() ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => $this->get_extension_slug( $id ),
+ 'items' => $this->two_items(),
+ )
+ );
$this->fail( 'Expected the failed item insert to throw.' );
} catch ( \RuntimeException $e ) {
$this->assertStringContainsString( 'item', $e->getMessage(), 'The write failure surfaces, not the release failure.' );
@@ -2544,14 +2591,28 @@ class ContractRepositoryTest extends EngineIntegrationTestCase {
$this->assertCount( 1, $this->item_ids( $id ), 'Items are written under the lock.' );
try {
- Contracts::update( $id, array( 'status' => ContractStatus::ON_HOLD ) );
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => $this->get_extension_slug( $id ),
+ 'status' => ContractStatus::ON_HOLD,
+ )
+ );
$this->fail( 'Expected an unlocked write to be refused.' );
} catch ( ContractLockedException $e ) {
$this->assertSame( 'checkout', $e->get_operation() );
}
$lock->release();
- $this->assertNotNull( Contracts::update( $id, array( 'status' => ContractStatus::ON_HOLD ) ) );
+ $this->assertNotNull(
+ Contracts::update(
+ $id,
+ array(
+ 'extension_slug' => $this->get_extension_slug( $id ),
+ 'status' => ContractStatus::ON_HOLD,
+ )
+ )
+ );
}
/**
diff --git a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/PlanMetaTest.php b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/PlanMetaTest.php
index deb113d6ae2..4e62c1bb526 100644
--- a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/PlanMetaTest.php
+++ b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/PlanMetaTest.php
@@ -12,6 +12,7 @@ namespace Automattic\WooCommerce\SubscriptionsEngine\Tests\Integration\Integrati
use EngineIntegrationTestCase;
use InvalidArgumentException;
+use RuntimeException;
use Automattic\WooCommerce\SubscriptionsEngine\Core\Entity\Plan;
use Automattic\WooCommerce\SubscriptionsEngine\Integration\Storage\PlanRepository;
use Automattic\WooCommerce\SubscriptionsEngine\Integration\Storage\SchemaInstaller;
@@ -21,6 +22,11 @@ use Automattic\WooCommerce\SubscriptionsEngine\Integration\Storage\SchemaInstall
*/
class PlanMetaTest extends EngineIntegrationTestCase {
+ /**
+ * Owning extension of the fixture plans.
+ */
+ private const OWNER = 'acme-subs';
+
/**
* The System Under Test.
*
@@ -42,7 +48,7 @@ class PlanMetaTest extends EngineIntegrationTestCase {
Plan::create(
array(
'name' => 'Meta plan',
- 'extension_slug' => 'acme-subs',
+ 'extension_slug' => self::OWNER,
)
)
);
@@ -52,8 +58,8 @@ class PlanMetaTest extends EngineIntegrationTestCase {
* @testdox add_meta keeps every value under one key in order.
*/
public function test_add_meta_keeps_every_value_under_one_key_in_order(): void {
- $first = $this->sut->add_meta( $this->id, 'note', 'one' );
- $second = $this->sut->add_meta( $this->id, 'note', 'two' );
+ $first = $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+ $second = $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
$this->assertIsInt( $first );
$this->assertIsInt( $second );
@@ -66,9 +72,9 @@ class PlanMetaTest extends EngineIntegrationTestCase {
* @testdox unique add on an existing key adds nothing.
*/
public function test_unique_add_on_an_existing_key_adds_nothing(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
- $this->assertNull( $this->sut->add_meta( $this->id, 'note', 'two', true ) );
+ $this->assertNull( $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two', true ) );
$this->assertSame( array( 'one' ), $this->sut->get_meta( $this->id, 'note' ) );
}
@@ -76,7 +82,7 @@ class PlanMetaTest extends EngineIntegrationTestCase {
* @testdox update_meta adds the key when absent.
*/
public function test_update_meta_adds_when_absent(): void {
- $this->assertTrue( $this->sut->update_meta( $this->id, 'note', 'one' ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'one' ) );
$this->assertSame( array( 'one' ), $this->sut->get_meta( $this->id, 'note' ) );
}
@@ -84,10 +90,10 @@ class PlanMetaTest extends EngineIntegrationTestCase {
* @testdox update_meta rewrites every row for the key.
*/
public function test_update_meta_rewrites_every_row_for_the_key(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
- $this->sut->add_meta( $this->id, 'note', 'two' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
- $this->assertTrue( $this->sut->update_meta( $this->id, 'note', 'three' ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'three' ) );
$this->assertSame( array( 'three', 'three' ), $this->sut->get_meta( $this->id, 'note' ) );
}
@@ -95,21 +101,21 @@ class PlanMetaTest extends EngineIntegrationTestCase {
* @testdox update_meta with a previous value rewrites only matching rows.
*/
public function test_update_meta_with_a_previous_value_rewrites_only_matching_rows(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
- $this->sut->add_meta( $this->id, 'note', 'two' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
- $this->assertTrue( $this->sut->update_meta( $this->id, 'note', 'three', 'two' ) );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'three', 'two' ) );
$this->assertSame( array( 'one', 'three' ), $this->sut->get_meta( $this->id, 'note' ) );
- $this->assertFalse( $this->sut->update_meta( $this->id, 'note', 'four', 'missing' ) );
+ $this->assertFalse( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'four', 'missing' ) );
}
/**
* @testdox update_meta to the same value reports no change.
*/
public function test_update_meta_to_the_same_value_reports_no_change(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
- $this->assertFalse( $this->sut->update_meta( $this->id, 'note', 'one' ) );
+ $this->assertFalse( $this->sut->update_meta( $this->id, self::OWNER, 'note', 'one' ) );
$this->assertSame( array( 'one' ), $this->sut->get_meta( $this->id, 'note' ) );
}
@@ -117,10 +123,10 @@ class PlanMetaTest extends EngineIntegrationTestCase {
* @testdox delete_meta with a value removes only that row.
*/
public function test_delete_meta_with_a_value_removes_only_that_row(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
- $this->sut->add_meta( $this->id, 'note', 'two' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
- $this->assertTrue( $this->sut->delete_meta( $this->id, 'note', 'one' ) );
+ $this->assertTrue( $this->sut->delete_meta( $this->id, self::OWNER, 'note', 'one' ) );
$this->assertSame( array( 'two' ), $this->sut->get_meta( $this->id, 'note' ) );
}
@@ -128,23 +134,23 @@ class PlanMetaTest extends EngineIntegrationTestCase {
* @testdox delete_meta without a value removes every row for the key.
*/
public function test_delete_meta_without_a_value_removes_every_row_for_the_key(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
- $this->sut->add_meta( $this->id, 'note', 'two' );
- $this->sut->add_meta( $this->id, 'other', 'kept' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'other', 'kept' );
- $this->assertTrue( $this->sut->delete_meta( $this->id, 'note' ) );
+ $this->assertTrue( $this->sut->delete_meta( $this->id, self::OWNER, 'note' ) );
$this->assertSame( array(), $this->sut->get_meta( $this->id, 'note' ) );
$this->assertSame( array( 'kept' ), $this->sut->get_meta( $this->id, 'other' ) );
- $this->assertFalse( $this->sut->delete_meta( $this->id, 'note' ) );
+ $this->assertFalse( $this->sut->delete_meta( $this->id, self::OWNER, 'note' ) );
}
/**
* @testdox get_meta without a key groups every key.
*/
public function test_get_meta_without_a_key_groups_every_key(): void {
- $this->sut->add_meta( $this->id, 'note', 'one' );
- $this->sut->add_meta( $this->id, 'flag', 'yes' );
- $this->sut->add_meta( $this->id, 'note', 'two' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'flag', 'yes' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two' );
$this->assertSame(
array(
@@ -164,10 +170,10 @@ class PlanMetaTest extends EngineIntegrationTestCase {
'b' => array( 'c' ),
);
- $this->sut->add_meta( $this->id, 'payload', $value );
+ $this->sut->add_meta( $this->id, self::OWNER, 'payload', $value );
$this->assertSame( $value, $this->sut->get_meta( $this->id, 'payload', true ) );
- $this->assertTrue( $this->sut->delete_meta( $this->id, 'payload', $value ) );
+ $this->assertTrue( $this->sut->delete_meta( $this->id, self::OWNER, 'payload', $value ) );
}
/**
@@ -186,11 +192,11 @@ class PlanMetaTest extends EngineIntegrationTestCase {
$plan = Plan::create(
array(
'name' => 'Other',
- 'extension_slug' => 'acme-subs',
+ 'extension_slug' => self::OWNER,
)
);
$other = $this->sut->insert( $plan );
- $this->sut->add_meta( $other, 'note', 'theirs' );
+ $this->sut->add_meta( $other, self::OWNER, 'note', 'theirs' );
$this->assertSame( array(), $this->sut->get_meta( $this->id, 'note' ) );
}
@@ -199,7 +205,7 @@ class PlanMetaTest extends EngineIntegrationTestCase {
* @testdox meta survives a plan update.
*/
public function test_meta_survives_a_plan_update(): void {
- $this->sut->add_meta( $this->id, 'note', 'kept' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'kept' );
$plan = $this->sut->find( $this->id );
$this->assertInstanceOf( Plan::class, $plan );
@@ -213,7 +219,7 @@ class PlanMetaTest extends EngineIntegrationTestCase {
* @testdox delete removes the plan's meta.
*/
public function test_delete_removes_the_plan_meta(): void {
- $this->sut->add_meta( $this->id, 'note', 'gone' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'gone' );
$this->assertTrue( $this->sut->delete( $this->id ) );
$this->assertSame( array(), $this->sut->get_meta( $this->id ) );
@@ -236,7 +242,7 @@ class PlanMetaTest extends EngineIntegrationTestCase {
* @testdox delete throws when the plan row fails to delete, and keeps the plan and its meta.
*/
public function test_delete_throws_when_the_plan_row_delete_fails(): void {
- $this->sut->add_meta( $this->id, 'note', 'kept' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'kept' );
$this->assert_write_throws_on_a_failed_query(
'DELETE FROM `' . SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLANS ) . '`',
@@ -254,19 +260,25 @@ class PlanMetaTest extends EngineIntegrationTestCase {
* @testdox a failed meta write throws.
* @dataProvider provide_failing_meta_writes
*
- * @param string $statement Start of the failing SQL statement.
+ * @param string $statement Start of the failing SQL statement, `{meta}` / `{plans}` for the tables.
* @param string $message Expected message fragment.
* @param string $method Repository method.
* @param array<int, mixed> $args Arguments after the plan id.
*/
public function test_a_failed_meta_write_throws( string $statement, string $message, string $method, array $args ): void {
- $this->sut->add_meta( $this->id, 'existing', 'one' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'existing', 'one' );
$this->assert_write_throws_on_a_failed_query(
- $statement . ' `' . SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLAN_META ) . '`',
+ strtr(
+ $statement,
+ array(
+ '{meta}' => SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLAN_META ),
+ '{plans}' => SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLANS ),
+ )
+ ),
$message,
function () use ( $method, $args ): void {
- $this->sut->{$method}( $this->id, ...$args );
+ $this->sut->{$method}( $this->id, self::OWNER, ...$args );
}
);
}
@@ -276,9 +288,53 @@ class PlanMetaTest extends EngineIntegrationTestCase {
*/
public function provide_failing_meta_writes(): array {
return array(
- 'add' => array( 'INSERT INTO', 'Failed to add plan meta', 'add_meta', array( 'note', 'x' ) ),
- 'update' => array( 'UPDATE', 'Failed to update plan meta', 'update_meta', array( 'existing', 'two' ) ),
- 'delete' => array( 'DELETE FROM', 'Failed to delete plan meta', 'delete_meta', array( 'existing' ) ),
+ 'add' => array( 'INSERT INTO {meta} ', 'Failed to add plan meta', 'add_meta', array( 'note', 'x' ) ),
+ 'update' => array( 'UPDATE {meta} m ', 'Failed to update plan meta', 'update_meta', array( 'existing', 'two' ) ),
+ 'update classifying read' => array( 'SELECT EXISTS ( SELECT 1 FROM {meta} m ', 'Failed to read plan meta', 'update_meta', array( 'absent', 'x' ) ),
+ 'update add branch' => array( 'INSERT INTO {meta} ', 'Failed to add plan meta', 'update_meta', array( 'absent', 'x' ) ),
+ 'unique add existence read' => array( 'SELECT p.id FROM {plans} p ', 'Failed to read plan', 'add_meta', array( 'existing', 'x', true ) ),
+ 'delete' => array( 'DELETE m FROM {meta} ', 'Failed to delete plan meta', 'delete_meta', array( 'existing' ) ),
+ );
+ }
+
+ /**
+ * @testdox a failed plan update or existence read throws.
+ * @dataProvider provide_failing_plan_update_queries
+ *
+ * @param string $statement Start of the failing SQL statement, `{plans}` for the table.
+ * @param string $message Expected message fragment.
+ */
+ public function test_a_failed_plan_update_throws( string $statement, string $message ): void {
+ $plans = SchemaInstaller::get_table_name( SchemaInstaller::TABLE_PLANS );
+ $plan = $this->sut->find( $this->id );
+ $this->assertInstanceOf( Plan::class, $plan );
+
+ // The UPDATE changes no row, so update_fields() reads whether the plan exists.
+ $change_nothing = static function ( string $query ) use ( $plans ): string {
+ return 0 === strpos( $query, "UPDATE {$plans} p " ) ? $query . ' AND 0 = 1' : $query;
+ };
+ add_filter( 'query', $change_nothing );
+
+ try {
+ $this->assert_write_throws_on_a_failed_query(
+ str_replace( '{plans}', $plans, $statement ),
+ $message,
+ function () use ( $plan ): void {
+ $this->sut->update_fields( $plan, array( 'name' ) );
+ }
+ );
+ } finally {
+ remove_filter( 'query', $change_nothing );
+ }
+ }
+
+ /**
+ * @return array<string, array{0: string, 1: string}>
+ */
+ public function provide_failing_plan_update_queries(): array {
+ return array(
+ 'update' => array( 'UPDATE {plans} p ', 'Failed to update plan' ),
+ 'existence read after it' => array( 'SELECT p.id FROM {plans} p ', 'Failed to read plan' ),
);
}
@@ -313,7 +369,7 @@ class PlanMetaTest extends EngineIntegrationTestCase {
* @testdox a delete under another extension slug keeps the plan and its meta.
*/
public function test_a_delete_under_another_extension_slug_keeps_the_plan_and_its_meta(): void {
- $this->sut->add_meta( $this->id, 'note', 'kept' );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'kept' );
$this->assertFalse( $this->sut->delete( $this->id, 'another-extension' ) );
$this->assertNotNull( $this->sut->find( $this->id ) );
@@ -321,25 +377,104 @@ class PlanMetaTest extends EngineIntegrationTestCase {
}
/**
- * @testdox an empty key is rejected on writes.
- * @dataProvider provide_write_methods
- *
- * @param string $method Write method name.
+ * @testdox add_meta and update_meta on a missing plan or a plan of another extension throw and write no orphan row.
*/
- public function test_an_empty_key_is_rejected_on_writes( string $method ): void {
- $this->expectException( InvalidArgumentException::class );
+ public function test_writes_on_a_missing_or_foreign_plan_write_nothing(): void {
+ foreach ( array( array( 999999, self::OWNER ), array( $this->id, 'another-extension' ) ) as list( $plan_id, $owner ) ) {
+ foreach ( array( 'add_meta', 'update_meta' ) as $method ) {
+ try {
+ $this->sut->{$method}( $plan_id, $owner, 'note', 'orphan' );
+ $this->fail( "Expected {$method}() to refuse plan {$plan_id} of {$owner}." );
+ } catch ( InvalidArgumentException $e ) {
+ $this->assertStringContainsString( sprintf( 'has id %d', $plan_id ), $e->getMessage() );
+ $this->assertStringContainsString( $owner, $e->getMessage() );
+ }
+ }
+ $this->assertSame( array(), $this->sut->get_meta( $plan_id ), 'No meta row is written.' );
+ }
+ }
- $this->sut->{$method}( $this->id, '', 'value' );
+ /**
+ * @testdox a unique add returns null for a taken key on an own plan and throws for a plan of another extension with that key.
+ */
+ public function test_a_unique_add_tells_a_taken_key_from_a_foreign_plan(): void {
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'one' );
+
+ $this->assertNull( $this->sut->add_meta( $this->id, self::OWNER, 'note', 'two', true ) );
+ $this->assertIsInt( $this->sut->add_meta( $this->id, self::OWNER, 'fresh', 'x', true ), 'An absent key is added by the NOT EXISTS insert.' );
+
+ try {
+ $this->sut->add_meta( $this->id, 'another-extension', 'note', 'theirs', true );
+ $this->fail( 'Expected a unique add on a foreign plan to throw.' );
+ } catch ( InvalidArgumentException $e ) {
+ $this->assertSame( array( 'one' ), $this->sut->get_meta( $this->id, 'note' ), 'Nothing is written.' );
+ }
}
/**
- * @return array<string, array{0: string}>
+ * @testdox update_meta and delete_meta on a plan of another extension change nothing.
*/
- public function provide_write_methods(): array {
- return array(
- 'add' => array( 'add_meta' ),
- 'update' => array( 'update_meta' ),
- 'delete' => array( 'delete_meta' ),
- );
+ public function test_update_and_delete_on_a_foreign_plan_change_nothing(): void {
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'kept' );
+
+ try {
+ $this->sut->update_meta( $this->id, 'another-extension', 'note', 'theirs' );
+ $this->fail( 'Expected update_meta() to refuse a foreign plan.' );
+ } catch ( InvalidArgumentException $e ) {
+ $this->assertSame( array( 'kept' ), $this->sut->get_meta( $this->id, 'note' ) );
+ }
+ $this->assertFalse( $this->sut->delete_meta( $this->id, 'another-extension', 'note' ) );
+ $this->assertSame( array( 'kept' ), $this->sut->get_meta( $this->id, 'note' ) );
+ }
+
+ /**
+ * @testdox the plan owner matches exactly: a slug differing only in case reads as another extension.
+ */
+ public function test_the_plan_owner_match_is_case_sensitive(): void {
+ $upper = strtoupper( self::OWNER );
+ $this->sut->add_meta( $this->id, self::OWNER, 'note', 'kept' );
+
+ $this->assertNull( $this->sut->find( $this->id, $upper ) );
+ $this->assertFalse( $this->sut->exists( $this->id, $upper ) );
+ $this->assertFalse( $this->sut->delete_meta( $this->id, $upper, 'note' ) );
+ foreach ( array( 'add_meta', 'update_meta' ) as $method ) {
+ try {
+ $this->sut->{$method}( $this->id, $upper, 'note', 'theirs' );
+ $this->fail( "Expected {$method}() to refuse a case variant of the owner." );
+ } catch ( InvalidArgumentException $e ) {
+ $this->assertStringContainsString( $upper, $e->getMessage() );
+ }
+ }
+ $this->assertSame( array( 'kept' ), $this->sut->get_meta( $this->id, 'note' ) );
+
+ $plan = $this->sut->find( $this->id );
+ $this->assertInstanceOf( Plan::class, $plan );
+ $plan->set_name( 'Renamed' );
+ $this->assertTrue( $this->sut->update_fields( $plan, array( 'name' ) ), 'The exact owner still writes.' );
+ }
+
+ /**
+ * @testdox a meta key longer than the 255-character column is refused and stores nothing, in ASCII or multibyte characters; a 255-character key is stored.
+ */
+ public function test_an_over_long_meta_key_is_refused(): void {
+ foreach ( array( 'k', 'é' ) as $char ) {
+ $too_long = str_repeat( $char, 256 );
+ foreach ( array( 'add_meta', 'update_meta' ) as $method ) {
+ try {
+ $this->sut->{$method}( $this->id, self::OWNER, $too_long, 'value' );
+ $this->fail( "Expected {$method}() to refuse an over-long key of '{$char}'." );
+ } catch ( RuntimeException $e ) {
+ $this->assertStringContainsString( 'is longer than the column allows', $e->getMessage(), $method );
+ }
+ }
+ $this->assertSame( array(), $this->sut->get_meta( $this->id ) );
+
+ $longest = str_repeat( $char, 255 );
+ $this->assertIsInt( $this->sut->add_meta( $this->id, self::OWNER, $longest, 'one' ) );
+ $this->sut->delete_meta( $this->id, self::OWNER, $longest );
+ $this->assertTrue( $this->sut->update_meta( $this->id, self::OWNER, $longest, 'two' ) );
+ $this->assertSame( array( $longest => array( 'two' ) ), $this->sut->get_meta( $this->id ) );
+ $this->sut->delete_meta( $this->id, self::OWNER, $longest );
+ }
}
}
diff --git a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/PlanRepositoryTest.php b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/PlanRepositoryTest.php
index 68c1f649d60..cbb7abe23d9 100644
--- a/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/PlanRepositoryTest.php
+++ b/packages/php/woocommerce-subscriptions-engine/tests/integration/Integration/Storage/PlanRepositoryTest.php
@@ -237,6 +237,37 @@ class PlanRepositoryTest extends EngineIntegrationTestCase {
$this->assertSame( $before->to_storage(), $after->to_storage() );
}
+ /**
+ * @testdox update_fields refuses a name too long for its column, as insert does, and writes nothing.
+ */
+ public function test_update_fields_refuses_a_name_too_long_for_its_column(): void {
+ $repo = new PlanRepository();
+ $long = str_repeat( 'n', 256 );
+
+ try {
+ $this->insert_plan( $repo, $long );
+ $this->fail( 'Expected insert to refuse the name.' );
+ } catch ( \RuntimeException $e ) {
+ $this->assertStringContainsString( 'Failed to insert plan', $e->getMessage() );
+ }
+
+ $id = $this->insert_plan( $repo, 'Short' );
+ $plan = $repo->find( $id );
+ $this->assertInstanceOf( Plan::class, $plan );
+ $plan->set_name( $long );
+
+ try {
+ $repo->update_fields( $plan, array( 'name' ) );
+ $this->fail( 'Expected update_fields to refuse the name.' );
+ } catch ( \RuntimeException $e ) {
+ $this->assertStringContainsString( 'longer than the column allows', $e->getMessage() );
+ }
+
+ $stored = $repo->find( $id );
+ $this->assertInstanceOf( Plan::class, $stored );
+ $this->assertSame( 'Short', $stored->get_name() );
+ }
+
/**
* @testdox update_fields without an id throws.
*/
diff --git a/packages/php/woocommerce-subscriptions-engine/tests/unit/Api/ContractLockTest.php b/packages/php/woocommerce-subscriptions-engine/tests/unit/Api/ContractLockTest.php
index 2ff15868fe4..84eed3e9e19 100644
--- a/packages/php/woocommerce-subscriptions-engine/tests/unit/Api/ContractLockTest.php
+++ b/packages/php/woocommerce-subscriptions-engine/tests/unit/Api/ContractLockTest.php
@@ -37,7 +37,7 @@ class ContractLockTest extends TestCase {
private function issue(): ContractLock {
$factory = Closure::bind(
static function ( string $token ): ContractLock {
- return new ContractLock( 7, 'hold', $token );
+ return new ContractLock( 7, 'acme', 'hold', $token );
},
null,
ContractRepository::class
@@ -162,7 +162,7 @@ class ContractLockTest extends TestCase {
* @testdox A stamp trusts the lock for at most 60 s, less when it ends sooner, and not at all with 60 s or less left.
*/
public function test_trust_window_follows_the_last_stamp(): void {
- $state = new ContractLockState( self::TOKEN );
+ $state = new ContractLockState( self::TOKEN, 'acme' );
$this->assertTrue( $state->is_trusted() );
$this->assertLessThanOrEqual( 60.0, $this->trust_left( $state ) );
$this->assertGreaterThan( 59.0, $this->trust_left( $state ) );
@@ -185,7 +185,7 @@ class ContractLockTest extends TestCase {
* @testdox A lock is near expiry by the local estimate when 120 s or less remain.
*/
public function test_near_expiry_estimate(): void {
- $state = new ContractLockState( self::TOKEN );
+ $state = new ContractLockState( self::TOKEN, 'acme' );
$this->assertFalse( $state->is_near_expiry() );
$state->stamp( 120 );
@@ -195,11 +195,18 @@ class ContractLockTest extends TestCase {
$this->assertFalse( $state->is_near_expiry() );
}
+ /**
+ * @testdox The state keeps the owner the lock was taken for.
+ */
+ public function test_state_keeps_the_owner(): void {
+ $this->assertSame( 'acme', ( new ContractLockState( self::TOKEN, 'acme' ) )->get_extension_slug() );
+ }
+
/**
* @testdox A released lock is never trusted, even after a stamp.
*/
public function test_a_released_lock_is_never_trusted(): void {
- $state = new ContractLockState( self::TOKEN );
+ $state = new ContractLockState( self::TOKEN, 'acme' );
$state->mark_released();
$this->assertTrue( $state->is_released() );
@@ -213,7 +220,7 @@ class ContractLockTest extends TestCase {
* @testdox The extend allowance is recorded once used.
*/
public function test_extend_allowance_flag(): void {
- $state = new ContractLockState( self::TOKEN );
+ $state = new ContractLockState( self::TOKEN, 'acme' );
$this->assertFalse( $state->has_extended() );
$state->mark_extended();