3.x #759
3.x #759
Conversation
* updated not operator
* updated index unit tests
…ss the database adapter and validator classes
Object(json) attribute support for postgres
WalkthroughThis PR introduces comprehensive support for object-type (JSONB/JSON) attributes and indexes across the database layer. Changes include adding an abstract Changes
Sequence DiagramsequenceDiagram
participant Client as Client Code
participant DB as Database
participant Adapter as Adapter<br/>(e.g., Postgres)
participant Validator as Validator
rect rgb(200, 220, 255)
Note over Client,Validator: Object Attribute Creation
Client->>DB: createAttribute(name, VAR_OBJECT)
DB->>Adapter: getSupportForObject()
Adapter-->>DB: true/false
DB->>Validator: validateAttribute()
Validator->>Validator: checkObjectType()
Validator-->>DB: valid
DB-->>Client: Attribute created
end
rect rgb(220, 200, 255)
Note over Client,Validator: Object Query Execution
Client->>DB: find([Query::equal('meta', {...})])
DB->>Adapter: getSQLCondition()
Adapter->>Adapter: handleObjectQueries()
Adapter->>Adapter: buildJSONBClause()
Adapter-->>DB: SQL fragment
DB-->>Client: Results
end
rect rgb(200, 255, 220)
Note over Client,Validator: Object Index Creation
Client->>DB: createIndex(type=INDEX_OBJECT, attributes=['meta'])
DB->>Validator: validateIndex()
Validator->>Validator: checkObjectIndexes()
Validator-->>DB: valid
Adapter->>Adapter: createGINIndex()
Adapter-->>DB: Index created
DB-->>Client: Success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Database/Database.php (1)
1994-2054: Restore JSON filter when bulk-creating object attributesNice catch adding
self::ATTRIBUTE_FILTER_TYPESso single attribute creation automatically injects the right filter, but the bulk path increateAttributes()still leavesfiltersas provided by the caller. For new"object"attributes created via this API we’ll now persist raw PHP arrays instead of JSON, which will either blow up on adapters expecting JSONB/JSON strings (Postgres) or silently store malformed data. This is a functional regression for every adapter that reportsgetSupportForObject() === true. Please mirror the logic you added increateAttribute()before the call tovalidateAttribute()inside the bulk loop.Apply this diff inside the foreach loop in
createAttributes()after we ensure$attribute['filters']is set:if (!isset($attribute['filters'])) { $attribute['filters'] = []; } + if (in_array($attribute['type'], self::ATTRIBUTE_FILTER_TYPES)) { + $attribute['filters'] = array_values( + array_unique(array_merge($attribute['filters'], [$attribute['type']])) + ); + } + $attributeDocument = $this->validateAttribute( $collection, $attribute['$id'],
🧹 Nitpick comments (2)
tests/unit/Validator/ObjectTest.php (1)
62-69: Consider testing additional edge cases for empty/null values.The test verifies empty arrays and invalid strings, but doesn't cover edge cases like
null,false, or0which the currentempty()check in the validator would accept. Consider adding test cases to explicitly document and verify the intended behavior for these values.Add test cases:
public function testEmptyCases(): void { $validator = new ObjectValidator(); $this->assertTrue($validator->isValid([])); $this->assertFalse($validator->isValid('sldfjsdlfj')); + + // Document behavior for null and other empty-ish values + $this->assertTrue($validator->isValid(null)); + $this->assertTrue($validator->isValid(false)); + $this->assertTrue($validator->isValid(0)); }src/Database/Validator/Index.php (1)
542-578: Minor: Grammar issue in error message.Line 564 has a subject-verb agreement error: "Object index do not support" should be "Object index does not support" or "Object indexes do not support".
Apply this fix:
if (!empty($orders)) { - $this->message = 'Object index do not support explicit orders. Remove the orders to create this index.'; + $this->message = 'Object indexes do not support explicit orders. Remove the orders to create this index.'; return false; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/Database/Adapter.php(1 hunks)src/Database/Adapter/MariaDB.php(1 hunks)src/Database/Adapter/Mongo.php(1 hunks)src/Database/Adapter/Pool.php(1 hunks)src/Database/Adapter/Postgres.php(7 hunks)src/Database/Adapter/SQL.php(1 hunks)src/Database/Adapter/SQLite.php(1 hunks)src/Database/Database.php(14 hunks)src/Database/Query.php(2 hunks)src/Database/Validator/Index.php(4 hunks)src/Database/Validator/ObjectValidator.php(1 hunks)src/Database/Validator/Query/Filter.php(2 hunks)src/Database/Validator/Structure.php(1 hunks)tests/e2e/Adapter/Base.php(2 hunks)tests/e2e/Adapter/Scopes/AttributeTests.php(2 hunks)tests/e2e/Adapter/Scopes/DocumentTests.php(2 hunks)tests/e2e/Adapter/Scopes/ObjectAttributeTests.php(1 hunks)tests/e2e/Adapter/Scopes/VectorTests.php(1 hunks)tests/unit/Validator/IndexTest.php(1 hunks)tests/unit/Validator/ObjectTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-03T02:04:17.803Z
Learnt from: abnegate
Repo: utopia-php/database PR: 721
File: tests/e2e/Adapter/Scopes/DocumentTests.php:6418-6439
Timestamp: 2025-10-03T02:04:17.803Z
Learning: In tests/e2e/Adapter/Scopes/DocumentTests::testSchemalessDocumentInvalidInteralAttributeValidation (PHP), when the adapter reports getSupportForAttributes() === false (schemaless), the test should not expect exceptions from createDocuments for “invalid” internal attributes; remove try/catch and ensure the test passes without exceptions, keeping at least one assertion.
Applied to files:
tests/e2e/Adapter/Base.phptests/e2e/Adapter/Scopes/ObjectAttributeTests.phpsrc/Database/Database.phptests/e2e/Adapter/Scopes/AttributeTests.phpsrc/Database/Validator/Query/Filter.phptests/e2e/Adapter/Scopes/VectorTests.phptests/e2e/Adapter/Scopes/DocumentTests.phptests/unit/Validator/IndexTest.php
📚 Learning: 2025-10-29T12:27:57.071Z
Learnt from: ArnabChatterjee20k
Repo: utopia-php/database PR: 747
File: src/Database/Adapter/Mongo.php:1449-1453
Timestamp: 2025-10-29T12:27:57.071Z
Learning: In src/Database/Adapter/Mongo.php, when getSupportForAttributes() returns false (schemaless mode), the updateDocument method intentionally uses a raw document without $set operator for replacement-style updates, as confirmed by the repository maintainer ArnabChatterjee20k.
Applied to files:
src/Database/Adapter/Mongo.php
📚 Learning: 2025-07-30T19:17:53.630Z
Learnt from: ArnabChatterjee20k
Repo: utopia-php/database PR: 642
File: src/Database/Validator/PartialStructure.php:43-52
Timestamp: 2025-07-30T19:17:53.630Z
Learning: In PartialStructure validator, when filtering for required attributes validation using the $requiredAttributes parameter, $this->attributes should be used instead of the merged $attributes array because this validation is specifically for internal attributes like $createdAt and $updatedAt that are defined in the base Structure class, not collection-specific attributes.
Applied to files:
src/Database/Validator/Structure.phptests/e2e/Adapter/Scopes/AttributeTests.php
📚 Learning: 2025-08-14T06:35:30.429Z
Learnt from: ArnabChatterjee20k
Repo: utopia-php/database PR: 661
File: tests/e2e/Adapter/Scopes/SpatialTests.php:180-186
Timestamp: 2025-08-14T06:35:30.429Z
Learning: Query::distance method in Utopia\Database\Query expects an array of values parameter, where each value is a [geometry, distance] pair. So the correct format is Query::distance('attribute', [[[lat, lng], distance]]) where the outer array contains the values and each value is [geometry, distance].
Applied to files:
src/Database/Validator/Query/Filter.php
🧬 Code graph analysis (17)
src/Database/Adapter/SQL.php (1)
src/Database/Database.php (1)
Database(37-8700)
src/Database/Adapter.php (5)
src/Database/Adapter/MariaDB.php (1)
getSupportForObject(2138-2141)src/Database/Adapter/Mongo.php (1)
getSupportForObject(2793-2796)src/Database/Adapter/Pool.php (1)
getSupportForObject(588-591)src/Database/Adapter/SQLite.php (1)
getSupportForObject(1011-1014)src/Database/Adapter/Postgres.php (1)
getSupportForObject(2179-2182)
src/Database/Adapter/Mongo.php (5)
src/Database/Adapter.php (1)
getSupportForObject(1080-1080)src/Database/Adapter/MariaDB.php (1)
getSupportForObject(2138-2141)src/Database/Adapter/Pool.php (1)
getSupportForObject(588-591)src/Database/Adapter/SQLite.php (1)
getSupportForObject(1011-1014)src/Database/Adapter/Postgres.php (1)
getSupportForObject(2179-2182)
src/Database/Validator/ObjectValidator.php (1)
src/Database/Database.php (1)
Database(37-8700)
src/Database/Adapter/SQLite.php (5)
src/Database/Adapter.php (1)
getSupportForObject(1080-1080)src/Database/Adapter/MariaDB.php (1)
getSupportForObject(2138-2141)src/Database/Adapter/Mongo.php (1)
getSupportForObject(2793-2796)src/Database/Adapter/Pool.php (1)
getSupportForObject(588-591)src/Database/Adapter/Postgres.php (1)
getSupportForObject(2179-2182)
src/Database/Validator/Structure.php (2)
src/Database/Database.php (1)
Database(37-8700)src/Database/Validator/ObjectValidator.php (1)
ObjectValidator(7-49)
tests/unit/Validator/ObjectTest.php (1)
src/Database/Validator/ObjectValidator.php (2)
ObjectValidator(7-49)isValid(22-32)
tests/e2e/Adapter/Scopes/ObjectAttributeTests.php (6)
src/Database/Adapter.php (1)
Adapter(16-1445)src/Database/Database.php (8)
Database(37-8700)createAttribute(1922-1978)createDocument(4544-4648)find(7220-7365)updateDocument(5105-5340)getDocument(3807-3936)count(7462-7513)createIndex(3604-3754)src/Database/Validator/Index.php (1)
Index(10-579)src/Database/Query.php (10)
Query(8-1181)getAttribute(173-176)equal(452-455)contains(528-531)notEqual(464-471)select(601-604)notContains(540-543)limit(644-647)vectorCosine(1165-1168)vectorEuclidean(1177-1180)src/Database/Adapter/Postgres.php (5)
getSupportForObject(2179-2182)createAttribute(445-474)createDocument(982-1081)updateDocument(1095-1327)createIndex(866-923)src/Database/Adapter/SQL.php (4)
createAttribute(247-261)find(2965-3170)getDocument(364-430)count(3182-3266)
src/Database/Database.php (6)
src/Database/Adapter.php (1)
getSupportForObject(1080-1080)src/Database/Adapter/MariaDB.php (1)
getSupportForObject(2138-2141)src/Database/Adapter/Mongo.php (1)
getSupportForObject(2793-2796)src/Database/Adapter/Pool.php (1)
getSupportForObject(588-591)src/Database/Adapter/SQLite.php (1)
getSupportForObject(1011-1014)src/Database/Adapter/Postgres.php (1)
getSupportForObject(2179-2182)
tests/e2e/Adapter/Scopes/AttributeTests.php (1)
src/Database/Database.php (4)
createCollection(1553-1692)createAttribute(1922-1978)Database(37-8700)getCollection(1746-1762)
src/Database/Validator/Query/Filter.php (1)
src/Database/Database.php (1)
Database(37-8700)
src/Database/Query.php (1)
src/Database/Database.php (1)
Database(37-8700)
src/Database/Adapter/Pool.php (6)
src/Database/Adapter.php (1)
getSupportForObject(1080-1080)src/Database/Adapter/MariaDB.php (1)
getSupportForObject(2138-2141)src/Database/Adapter/Mongo.php (1)
getSupportForObject(2793-2796)src/Database/Adapter/SQLite.php (1)
getSupportForObject(1011-1014)src/Database/Adapter/Postgres.php (1)
getSupportForObject(2179-2182)src/Database/Mirror.php (1)
delegate(88-103)
tests/unit/Validator/IndexTest.php (2)
src/Database/Database.php (1)
Database(37-8700)src/Database/Validator/Index.php (1)
Index(10-579)
src/Database/Validator/Index.php (1)
src/Database/Database.php (1)
Database(37-8700)
src/Database/Adapter/Postgres.php (7)
src/Database/Database.php (2)
Database(37-8700)count(7462-7513)src/Database/Query.php (4)
Query(8-1181)getMethod(165-168)getValues(181-184)isObjectAttribute(987-990)src/Database/Adapter.php (2)
count(827-827)getSupportForObject(1080-1080)src/Database/Adapter/Mongo.php (2)
count(2095-2182)getSupportForObject(2793-2796)src/Database/Adapter/Pool.php (2)
count(273-276)getSupportForObject(588-591)src/Database/Adapter/MariaDB.php (1)
getSupportForObject(2138-2141)src/Database/Adapter/SQLite.php (1)
getSupportForObject(1011-1014)
src/Database/Adapter/MariaDB.php (5)
src/Database/Adapter.php (1)
getSupportForObject(1080-1080)src/Database/Adapter/Mongo.php (1)
getSupportForObject(2793-2796)src/Database/Adapter/Pool.php (1)
getSupportForObject(588-591)src/Database/Adapter/SQLite.php (1)
getSupportForObject(1011-1014)src/Database/Adapter/Postgres.php (1)
getSupportForObject(2179-2182)
🪛 PHPMD (2.15.0)
tests/e2e/Adapter/Scopes/ObjectAttributeTests.php
82-82: Avoid unused local variables such as '$doc2'. (undefined)
(UnusedLocalVariable)
325-325: Avoid unused local variables such as '$doc7'. (undefined)
(UnusedLocalVariable)
339-339: Avoid unused local variables such as '$doc8'. (undefined)
(UnusedLocalVariable)
442-442: Avoid unused local variables such as '$doc11'. (undefined)
(UnusedLocalVariable)
575-575: Avoid unused local variables such as '$doc1'. (undefined)
(UnusedLocalVariable)
588-588: Avoid unused local variables such as '$doc2'. (undefined)
(UnusedLocalVariable)
961-961: Avoid unused local variables such as '$docA'. (undefined)
(UnusedLocalVariable)
986-986: Avoid unused local variables such as '$docB'. (undefined)
(UnusedLocalVariable)
1008-1008: Avoid unused local variables such as '$docC'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (19)
src/Database/Validator/Query/Filter.php (2)
165-168: LGTM! Object value validation correctly deferred.The approach of skipping detailed per-value validation for
VAR_OBJECTattributes is sound, as object values are arbitrary JSON structures that will be validated during encoding/decoding. This aligns with the spatial type handling pattern.
239-248: LGTM! Contains query validation correctly extended for objects.The modification appropriately allows
containsandnotContainsqueries on object-type attributes, consistent with JSONB query capabilities. The error message accurately reflects the supported types.src/Database/Adapter/SQL.php (1)
1178-1185: LGTM! Object width calculation is correct.The 20-byte allocation for
VAR_OBJECTattributes correctly accounts for the JSONB/JSON pointer overhead, consistent with other externally-stored types (arrays, LINESTRING, POLYGON, MEDIUMTEXT). The comment accurately describes the storage model.src/Database/Adapter/Mongo.php (1)
2793-2796: LGTM! Object support correctly reported as not implemented.While MongoDB has native object/document support via BSON, returning
falsecorrectly indicates that the new object attribute feature is not implemented for the Mongo adapter in this PR. This is consistent with MariaDB and SQLite, while only PostgreSQL implements the feature.tests/e2e/Adapter/Scopes/VectorTests.php (1)
2714-2714: Typo fix LGTM.Method renamed to testVectorUpsert; name now matches behavior.
src/Database/Adapter/MariaDB.php (1)
2138-2141: Capability surface aligned; no object support (correct).Method satisfies Adapter::getSupportForObject() and advertises no support, which matches getSQLType not handling VAR_OBJECT.
Please confirm Adapter.php declares this abstract and Pool delegates it (looks present), and CI covers skipping object tests for MariaDB.
src/Database/Query.php (2)
984-991: Accessor LGTM.isObjectAttribute() is a clear helper for adapter logic.
Please confirm Postgres adapter uses this to switch to JSONB operators and other adapters gate with getSupportForObject() == false.
466-470: notEqual normalization: confirm downstream array binding for maps.Wrapping maps (non-list arrays) as a single value is sensible, but adapters must not bind raw arrays to PDO (will error). Ensure adapter SQL encodes object/map values (e.g., JSON) when attributeType is object; otherwise notEqual(['key'=>...]) will bind an array.
Run a quick check for handling object attribute binds in adapters:
src/Database/Adapter/SQLite.php (1)
1011-1014: SQLite correctly reports no object support.Matches current capabilities and keeps API consistent across adapters.
tests/e2e/Adapter/Base.php (1)
12-12: E2E scope inclusion LGTM.ObjectAttributeTests wired into Base. Ensure tests skip when adapter getSupportForObject() is false.
Confirm ObjectAttributeTests guards each test with adapter capability checks (mirroring vector tests).
Also applies to: 36-36
tests/e2e/Adapter/Scopes/DocumentTests.php (2)
1893-1895: Expectation message update looks correct.Including “object” in the error text matches new object-typed attribute support.
3374-3376: Expectation message update looks correct.“notContains … not an array, string, or object” aligns with the expanded type support.
src/Database/Adapter.php (1)
1075-1081: All adapters correctly implement getSupportForObject()—no action needed.Verification confirms that all concrete adapter classes properly implement the new abstract method:
- Pool.php (concrete): ✓ implements at line 588
- Mongo.php (concrete): ✓ implements at line 2793
- Postgres.php (concrete): ✓ implements
getSupportForObject()- MariaDB.php (concrete): ✓ implements
getSupportForObject()SQL.php is abstract and correctly defers implementation to its concrete subclasses. There are no missing implementations, and the BC considerations are properly managed across the codebase.
src/Database/Adapter/Pool.php (1)
588-591: LGTM! Proper delegation implementation.The method correctly delegates to the underlying adapter using the established pattern, consistent with other capability methods in this class.
tests/e2e/Adapter/Scopes/AttributeTests.php (1)
1569-1569: LGTM! Error message correctly updated for object support.The error message now accurately reflects that
containsqueries can be used on array, string, or object attributes, consistent with the broader object-type support introduced in this PR.src/Database/Validator/Index.php (2)
31-31: LGTM! Feature flag properly integrated.The
supportForObjectIndexesparameter follows the established pattern for other feature flags in this validator, with appropriate documentation and a safe default value.Also applies to: 46-46
137-139: LGTM! Validation check properly integrated.The object index validation is correctly integrated into the validation chain, consistent with other index type validations.
tests/unit/Validator/IndexTest.php (1)
241-326: LGTM! Comprehensive test coverage for object index validation.The test thoroughly validates all object index scenarios:
- Valid single-attribute object index
- Invalid non-object attribute
- Invalid multiple attributes
- Invalid explicit orders
- Feature flag enforcement
Good use of named parameters to improve readability when constructing the validator.
src/Database/Validator/ObjectValidator.php (1)
22-32: Confirm intentional null acceptance and explicit handling for edge cases.The
empty($value)check acceptsnullintentionally for optional object attributes (confirmed by integration tests showingrequired=false, default=null). However, it also acceptsfalse,0, and'', which are untested and likely unintended. The non-array validation path (line 430) lacks the explicit null-skip present in array validation (line 417), creating inconsistency.Clarify the intent: if
nullis acceptable for optional fields, document this explicitly. If the permissiveness for other falsy values is unintended, replaceempty($value)with explicit type checks likeis_null($value) || (is_array($value) && !array_is_list($value)).
| case Query::TYPE_NOT_CONTAINS: { | ||
| $isNot = $query->getMethod() === Query::TYPE_NOT_CONTAINS; | ||
| $conditions = []; | ||
| foreach ($query->getValues() as $key => $value) { | ||
| if (count($value) === 1) { | ||
| $jsonKey = array_key_first($value); | ||
| $jsonValue = $value[$jsonKey]; | ||
|
|
||
| // If scalar (e.g. "skills" => "typescript"), | ||
| // wrap it to express array containment: {"skills": ["typescript"]} | ||
| // If it's already an object/associative array (e.g. "config" => ["lang" => "en"]), | ||
| // keep as-is to express object containment. | ||
| if (!\is_array($jsonValue)) { | ||
| $value[$jsonKey] = [$jsonValue]; | ||
| } | ||
| } | ||
| $binds[":{$placeholder}_{$key}"] = json_encode($value); | ||
| $fragment = "{$alias}.{$attribute} @> :{$placeholder}_{$key}::jsonb"; | ||
| $conditions[] = $isNot ? "NOT (" . $fragment . ")" : $fragment; | ||
| } | ||
| $separator = $isNot ? ' AND ' : ' OR '; | ||
| return empty($conditions) ? '' : '(' . implode($separator, $conditions) . ')'; |
There was a problem hiding this comment.
Fix nested array containment support.
Try inserting a document with meta.profile.skills = ['php', 'go'] and running Query::contains('meta', [['profile' => ['skills' => 'php']]]). The SQL we emit becomes meta @> '{"profile":{"skills":"php"}}'::jsonb, which PostgreSQL will never match because "skills" is stored as an array, not a scalar. The helper only wraps scalars when the fragment consists of a single top-level key, so any nested array (and even top-level arrays combined with other keys) silently stops matching. As a result contains() can no longer express “profile.skills contains X”, which is a correctness regression for realistic payloads. Please normalize these fragments recursively (or drop the partial scalar-to-array heuristic and require callers to supply the proper JSON shape) so nested array lookups continue to work. Happy to help iterate on the fix.
🤖 Prompt for AI Agents
In src/Database/Adapter/Postgres.php around lines 1688-1709, the current
scalar-to-array wrapping only handles single top-level keys and fails to
normalize nested values (so queries like contains('meta',
[['profile'=>['skills'=>'php']]]) emit fragments that won't match stored
arrays). Replace the partial heuristic with a recursive normalizer: traverse the
value tree; for associative arrays (array with string keys) iterate each key and
if the value is scalar wrap it into a single-element array, otherwise recurse;
for numeric-indexed lists (array_is_list) treat them as explicit arrays (do not
convert them to objects) but recurse into their elements if those elements are
associative; after normalization json_encode the resulting structure for the
bind. Implement this recursion and use it in place of the current top-level-only
wrapping so nested array containment queries produce correct JSONB fragments.
| case Database::VAR_OBJECT: | ||
| $validators[] = new ObjectValidator(); | ||
| break; | ||
|
|
There was a problem hiding this comment.
Tighten ObjectValidator semantics: avoid accepting null and non-object JSON.
Delegating to ObjectValidator is fine, but the current implementation (per snippets) treats any valid JSON (arrays/primitives) and even null/empty as valid “object” values. That weakens type guarantees and could let invalid data through.
Please update ObjectValidator::isValid to:
- Accept only associative arrays (non-list) when value is array.
- For strings, require JSON that decodes to an associative array (non-list); reject primitives/arrays.
- Reject null explicitly (optional attributes are already handled by Structure).
Example adjustment (ObjectValidator.php):
public function isValid(mixed $value): bool
{
if (is_string($value)) {
$decoded = json_decode($value, true);
if (json_last_error() !== JSON_ERROR_NONE) {
return false;
}
return is_array($decoded) && !array_is_list($decoded);
}
if (is_array($value)) {
return !array_is_list($value); // associative only
}
return false; // disallow null, scalars, and objects by default
}Consider tests covering:
- Reject '"foo"', '123', '[]', null.
- Accept '{}', ['k' => 'v'], '{"k":"v"}'.
🤖 Prompt for AI Agents
In src/Database/Validator/Structure.php around lines 376 to 379, the
ObjectValidator currently accepts nulls, list-arrays, primitives or JSON that
decodes to non-associative arrays; tighten semantics by updating
ObjectValidator::isValid to: reject null and all non-array/non-string inputs;
for arrays accept only associative arrays (use !array_is_list); for strings
json_decode with true, verify no json error and that the result is an
associative array (not a list); return false for all scalars and malformed JSON.
Add unit tests to reject '"foo"', '123', '[]', null and to accept '{}',
['k'=>'v'], '{"k":"v"}'.
| public function testCreateDatetimeAddingAutoFilter(): void | ||
| { | ||
| /** @var Database $database */ | ||
| $database = static::getDatabase(); | ||
|
|
||
| $database->createCollection('datetime_fail'); | ||
| $database->createCollection('datetime_auto_filter'); | ||
|
|
||
| /** Test for FAILURE */ | ||
| $this->expectException(Exception::class); | ||
| $database->createAttribute('datetime_fail', 'date_fail', Database::VAR_DATETIME, 0, false); | ||
| $database->createAttribute('datetime_auto', 'date_auto', Database::VAR_DATETIME, 0, false, filters:['json']); | ||
| $collection = $database->getCollection('datetime_auto_filter'); | ||
| $attribute = $collection->getAttribute('attributes')[0]; | ||
| $this->assertEquals([Database::VAR_DATETIME,'json'], $attribute['filters']); | ||
| $database->updateAttribute('datetime_auto', 'date_auto', Database::VAR_DATETIME, 0, false, filters:[]); | ||
| $collection = $database->getCollection('datetime_auto_filter'); | ||
| $attribute = $collection->getAttribute('attributes')[0]; | ||
| $this->assertEquals([Database::VAR_DATETIME,'json'], $attribute['filters']); | ||
| $database->deleteCollection('datetime_auto_filter'); | ||
| } |
There was a problem hiding this comment.
Critical: Collection name mismatch will cause test failure.
The test references inconsistent collection names:
- Line 1758: Creates collection
'datetime_auto_filter' - Line 1761: Creates attribute on
'datetime_auto'(wrong collection) - Line 1765: Updates attribute on
'datetime_auto'(wrong collection)
This will cause the attribute operations to fail since they're targeting a non-existent collection.
Apply this fix:
public function testCreateDatetimeAddingAutoFilter(): void
{
/** @var Database $database */
$database = static::getDatabase();
$database->createCollection('datetime_auto_filter');
$this->expectException(Exception::class);
- $database->createAttribute('datetime_auto', 'date_auto', Database::VAR_DATETIME, 0, false, filters:['json']);
+ $database->createAttribute('datetime_auto_filter', 'date_auto', Database::VAR_DATETIME, 0, false, filters:['json']);
$collection = $database->getCollection('datetime_auto_filter');
$attribute = $collection->getAttribute('attributes')[0];
$this->assertEquals([Database::VAR_DATETIME,'json'], $attribute['filters']);
- $database->updateAttribute('datetime_auto', 'date_auto', Database::VAR_DATETIME, 0, false, filters:[]);
+ $database->updateAttribute('datetime_auto_filter', 'date_auto', Database::VAR_DATETIME, 0, false, filters:[]);
$collection = $database->getCollection('datetime_auto_filter');
$attribute = $collection->getAttribute('attributes')[0];
$this->assertEquals([Database::VAR_DATETIME,'json'], $attribute['filters']);
$database->deleteCollection('datetime_auto_filter');
}🤖 Prompt for AI Agents
In tests/e2e/Adapter/Scopes/AttributeTests.php around lines 1753 to 1770, the
test creates the collection 'datetime_auto_filter' but then calls
createAttribute and updateAttribute on 'datetime_auto' (wrong collection);
change those attribute calls to use 'datetime_auto_filter' so the attribute
operations target the created collection, then leave the subsequent assertions
and deleteCollection call as-is.
Summary by CodeRabbit