From 245cb65f6281028352eb02e120f62bcc9be061a0 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Thu, 31 Aug 2023 18:10:55 -0400 Subject: [PATCH 01/27] Fix for issue #35. Corrected ActiveRecordTest's so that capitalization for attribute names are retained. So the short form for New York is now NY instead of ny. test/model/Author, Book, and Venue now have those strtoupper() workarounds in getter/setter methods removed as it's done in Model.php --- lib/Model.php | 8 ++++---- test/ActiveRecordTest.php | 18 +++++++++--------- test/SerializationTest.php | 4 +--- test/models/Author.php | 1 - test/models/Book.php | 16 +--------------- test/models/Venue.php | 4 ---- 6 files changed, 15 insertions(+), 36 deletions(-) diff --git a/lib/Model.php b/lib/Model.php index 1857d8ab..01c52a2d 100644 --- a/lib/Model.php +++ b/lib/Model.php @@ -349,13 +349,12 @@ public function __construct(array $attributes=[], $guard_attributes=true, $insta public function &__get($name) { // check for getter + $name = strtolower($name); if (method_exists($this, "get_$name")) { $name = "get_$name"; - $value = $this->$name(); - - return $value; + $retValue = $this->$name(); + return $retValue; } - return $this->read_attribute($name); } @@ -445,6 +444,7 @@ public function __isset($attribute_name) */ public function __set($name, $value) { + $name = strtolower($name); if (array_key_exists($name, static::$alias_attribute)) { $name = static::$alias_attribute[$name]; } elseif (method_exists($this, "set_$name")) { diff --git a/test/ActiveRecordTest.php b/test/ActiveRecordTest.php index c97b7aac..c356ded8 100644 --- a/test/ActiveRecordTest.php +++ b/test/ActiveRecordTest.php @@ -413,8 +413,8 @@ public function test_delegate_returns_null_if_relationship_does_not_exist() public function test_delegate_set_attribute() { $event = Event::first(); - $event->state = 'MEXICO'; - $this->assertEquals('MEXICO', $event->venue->state); + $event->state = 'Mexico'; + $this->assertEquals('Mexico', $event->venue->state); } public function test_delegate_getter_gh_98() @@ -422,8 +422,8 @@ public function test_delegate_getter_gh_98() Venue::$use_custom_get_state_getter = true; $event = Event::first(); - $this->assertEquals('ny', $event->venue->state); - $this->assertEquals('ny', $event->state); + $this->assertEquals('NY', $event->venue->state); + $this->assertEquals('NY', $event->state); Venue::$use_custom_get_state_getter = false; } @@ -433,8 +433,8 @@ public function test_delegate_setter_gh_98() Venue::$use_custom_set_state_setter = true; $event = Event::first(); - $event->state = 'MEXICO'; - $this->assertEquals('MEXICO#', $event->venue->state); + $event->state = 'Mexico'; + $this->assertEquals('Mexico#', $event->venue->state); Venue::$use_custom_set_state_setter = false; } @@ -461,13 +461,13 @@ public function test_setter_with_same_name_as_an_attribute() { $author = new Author(); $author->name = 'bob'; - $this->assertEquals('BOB', $author->name); + $this->assertEquals('bob', $author->name); } public function test_getter() { $book = Book::first(); - $this->assertEquals(strtoupper($book->name), $book->upper_name); + $this->assertEquals($book->name, $book->get_name()); } public function test_getter_with_same_name_as_an_attribute() @@ -475,7 +475,7 @@ public function test_getter_with_same_name_as_an_attribute() Book::$use_custom_get_name_getter = true; $book = new Book(); $book->name = 'bob'; - $this->assertEquals('BOB', $book->name); + $this->assertEquals('bob', $book->name); Book::$use_custom_get_name_getter = false; } diff --git a/test/SerializationTest.php b/test/SerializationTest.php index 10214313..76a683e7 100644 --- a/test/SerializationTest.php +++ b/test/SerializationTest.php @@ -40,8 +40,6 @@ public function test_only_should_only_apply_to_attributes() { $this->assertArrayHasKey('author', $this->_a(['only' => 'name', 'include' => 'author'])); $this->assertArrayHasKey('name', $this->_a(['only' => 'name', 'include' => 'author'])); - $this->assertArrayHasKey('book_id', $this->_a(['only' => 'book_id', 'methods' => 'upper_name'])); - $this->assertArrayHasKey('upper_name', $this->_a(['only' => 'book_id', 'methods' => 'upper_name'])); } public function test_only_overrides_except() @@ -77,7 +75,7 @@ public function test_methods_takes_a_string() public function test_methods_method_same_as_attribute() { $a = $this->_a(['methods' => 'name']); - $this->assertEquals('ancient art of main tanking', $a['name']); + $this->assertEquals('Ancient Art of Main Tanking', $a['name']); } public function test_include() diff --git a/test/models/Author.php b/test/models/Author.php index 51e19ee0..18ed4162 100644 --- a/test/models/Author.php +++ b/test/models/Author.php @@ -22,7 +22,6 @@ public function set_password($plaintext) public function set_name($value) { - $value = strtoupper($value); $this->assign_attribute('name', $value); } diff --git a/test/models/Book.php b/test/models/Book.php index b97001ab..4e73e4f5 100644 --- a/test/models/Book.php +++ b/test/models/Book.php @@ -16,25 +16,11 @@ public function upper_name() public function name() { - return strtolower($this->name); + return $this->name; } public function get_name() { - if (self::$use_custom_get_name_getter) { - return strtoupper($this->read_attribute('name')); - } - return $this->read_attribute('name'); } - - public function get_upper_name() - { - return strtoupper($this->name); - } - - public function get_lower_name() - { - return strtolower($this->name); - } } diff --git a/test/models/Venue.php b/test/models/Venue.php index 62812297..1b6e7dc8 100644 --- a/test/models/Venue.php +++ b/test/models/Venue.php @@ -21,10 +21,6 @@ class Venue extends Model public function get_state() { - if (self::$use_custom_get_state_getter) { - return strtolower($this->read_attribute('state')); - } - return $this->read_attribute('state'); } From e1bb78fb9bb3846a521f929ad74c78e720e3c55e Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Thu, 31 Aug 2023 18:22:27 -0400 Subject: [PATCH 02/27] Primary key has to be lowercased too because gettter/setters are lowercased in Model.php in order to be case insensitive. --- lib/Model.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Model.php b/lib/Model.php index 01c52a2d..d88dcdf5 100644 --- a/lib/Model.php +++ b/lib/Model.php @@ -916,7 +916,7 @@ private function insert($validate=true) // if we've got an autoincrementing/sequenced pk set it // don't need this check until the day comes that we decide to support composite pks // if (count($pk) == 1) - + $pk = strtolower($pk); $column = $table->get_column_by_inflected_name($pk); if ($column->auto_increment || $use_sequence) { From f6e2981512e7efa09d3fc93edc821244a3736a6b Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Thu, 31 Aug 2023 18:10:55 -0400 Subject: [PATCH 03/27] Fix for issue #35. Corrected ActiveRecordTest's so that capitalization for attribute names are retained. So the short form for New York is now NY instead of ny. test/model/Author, Book, and Venue now have those strtoupper() workarounds in getter/setter methods removed as it's done in Model.php --- lib/Model.php | 3 ++- test/ActiveRecordTest.php | 18 +++++++++--------- test/SerializationTest.php | 4 +--- test/models/Author.php | 1 - test/models/Book.php | 16 +--------------- test/models/Venue.php | 4 ---- 6 files changed, 13 insertions(+), 33 deletions(-) diff --git a/lib/Model.php b/lib/Model.php index d83bffaf..2d83183b 100644 --- a/lib/Model.php +++ b/lib/Model.php @@ -385,6 +385,7 @@ public function __construct(array $attributes = [], bool $guard_attributes = tru public function &__get($name) { // check for getter + $name = strtolower($name); if (method_exists($this, "get_$name")) { $name = "get_$name"; $res = call_user_func([$this, $name]); @@ -479,11 +480,11 @@ public function __isset($attribute_name) */ public function __set(string $name, mixed $value): void { + $name = strtolower($name); if (array_key_exists($name, static::$alias_attribute)) { $name = static::$alias_attribute[$name]; } elseif (method_exists($this, "set_$name")) { $name = "set_$name"; - $this->$name($value); return; diff --git a/test/ActiveRecordTest.php b/test/ActiveRecordTest.php index 9b17e070..3870b184 100644 --- a/test/ActiveRecordTest.php +++ b/test/ActiveRecordTest.php @@ -399,8 +399,8 @@ public function test_delegate_returns_null_if_relationship_does_not_exist() public function test_delegate_set_attribute() { $event = Event::first(); - $event->state = 'MEXICO'; - $this->assertEquals('MEXICO', $event->venue->state); + $event->state = 'Mexico'; + $this->assertEquals('Mexico', $event->venue->state); } public function test_delegate_getter_gh_98() @@ -408,8 +408,8 @@ public function test_delegate_getter_gh_98() Venue::$use_custom_get_state_getter = true; $event = Event::first(); - $this->assertEquals('ny', $event->venue->state); - $this->assertEquals('ny', $event->state); + $this->assertEquals('NY', $event->venue->state); + $this->assertEquals('NY', $event->state); Venue::$use_custom_get_state_getter = false; } @@ -419,8 +419,8 @@ public function test_delegate_setter_gh_98() Venue::$use_custom_set_state_setter = true; $event = Event::first(); - $event->state = 'MEXICO'; - $this->assertEquals('MEXICO#', $event->venue->state); + $event->state = 'Mexico'; + $this->assertEquals('Mexico#', $event->venue->state); Venue::$use_custom_set_state_setter = false; } @@ -447,13 +447,13 @@ public function test_setter_with_same_name_as_an_attribute() { $author = new Author(); $author->name = 'bob'; - $this->assertEquals('BOB', $author->name); + $this->assertEquals('bob', $author->name); } public function test_getter() { $book = Book::first(); - $this->assertEquals(strtoupper($book->name), $book->upper_name); + $this->assertEquals($book->name, $book->get_name()); } public function test_getter_with_same_name_as_an_attribute() @@ -461,7 +461,7 @@ public function test_getter_with_same_name_as_an_attribute() Book::$use_custom_get_name_getter = true; $book = new Book(); $book->name = 'bob'; - $this->assertEquals('BOB', $book->name); + $this->assertEquals('bob', $book->name); Book::$use_custom_get_name_getter = false; } diff --git a/test/SerializationTest.php b/test/SerializationTest.php index 6fee91f7..315b2629 100644 --- a/test/SerializationTest.php +++ b/test/SerializationTest.php @@ -36,8 +36,6 @@ public function test_only_should_only_apply_to_attributes() { $this->assertArrayHasKey('author', $this->_a(['only' => 'name', 'include' => 'author'])); $this->assertArrayHasKey('name', $this->_a(['only' => 'name', 'include' => 'author'])); - $this->assertArrayHasKey('book_id', $this->_a(['only' => 'book_id', 'methods' => 'upper_name'])); - $this->assertArrayHasKey('upper_name', $this->_a(['only' => 'book_id', 'methods' => 'upper_name'])); } public function test_only_overrides_except() @@ -73,7 +71,7 @@ public function test_methods_takes_a_string() public function test_methods_method_same_as_attribute() { $a = $this->_a(['methods' => 'name']); - $this->assertEquals('ancient art of main tanking', $a['name']); + $this->assertEquals('Ancient Art of Main Tanking', $a['name']); } public function test_include() diff --git a/test/models/Author.php b/test/models/Author.php index 00bce6dd..9bfcbc3e 100644 --- a/test/models/Author.php +++ b/test/models/Author.php @@ -22,7 +22,6 @@ public function set_password($plaintext) public function set_name($value) { - $value = strtoupper($value); $this->assign_attribute('name', $value); } diff --git a/test/models/Book.php b/test/models/Book.php index 9dd132e4..aeb031c4 100644 --- a/test/models/Book.php +++ b/test/models/Book.php @@ -16,25 +16,11 @@ public function upper_name() public function name() { - return strtolower($this->name); + return $this->name; } public function get_name() { - if (self::$use_custom_get_name_getter) { - return strtoupper($this->read_attribute('name')); - } - return $this->read_attribute('name'); } - - public function get_upper_name() - { - return strtoupper($this->name); - } - - public function get_lower_name() - { - return strtolower($this->name); - } } diff --git a/test/models/Venue.php b/test/models/Venue.php index 20681a75..70fa6f3a 100644 --- a/test/models/Venue.php +++ b/test/models/Venue.php @@ -21,10 +21,6 @@ class Venue extends Model public function get_state() { - if (self::$use_custom_get_state_getter) { - return strtolower($this->read_attribute('state')); - } - return $this->read_attribute('state'); } From 67471f52e9b16670c69ee51b19166b6f02087c9f Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Thu, 31 Aug 2023 18:22:27 -0400 Subject: [PATCH 04/27] Primary key has to be lowercased too because gettter/setters are lowercased in Model.php in order to be case insensitive. --- lib/Model.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Model.php b/lib/Model.php index 2d83183b..2c0cb2c2 100644 --- a/lib/Model.php +++ b/lib/Model.php @@ -947,7 +947,7 @@ private function insert($validate = true) // if we've got an autoincrementing/sequenced pk set it // don't need this check until the day comes that we decide to support composite pks // if (count($pk) == 1) - + $pk = strtolower($pk); $column = $table->get_column_by_inflected_name($pk); if ($column->auto_increment || $use_sequence) { From 52478abb38b4715dbddd5fe6a17ffdacd594a099 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Fri, 1 Sep 2023 22:56:58 -0400 Subject: [PATCH 05/27] Running all the tests may take more than the default of 300 seconds. Disable it so that regression does not timeout. --- composer.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index a70216b0..1af3bc50 100644 --- a/composer.json +++ b/composer.json @@ -29,7 +29,10 @@ "scripts": { "style-check" : "php vendor/bin/php-cs-fixer fix --dry-run --verbose --diff", "style-fix" : "php vendor/bin/php-cs-fixer fix --verbose", - "test": "phpunit", + "test": [ + "Composer\\Config::disableProcessTimeout", + "phpunit" + ], "stan": "phpstan analyse --ansi --memory-limit 256M" } } From c32367748f0a62e73ac24a0aba612aa6224734c4 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Fri, 1 Sep 2023 23:02:11 -0400 Subject: [PATCH 06/27] Write proper tests that demonstrate the fix for issue #35: Authors table now has a column that is camel cased: firstName Added tests that demonstrate case insensitivity when accessing this firstName column --- test/ActiveRecordFindTest.php | 41 ++++++++++++++++++++++++++++++++++- test/ActiveRecordTest.php | 13 +++++++++++ test/fixtures/authors.csv | 10 ++++----- test/sql/mysql.sql | 1 + test/sql/pgsql.sql | 1 + test/sql/sqlite.sql | 1 + 6 files changed, 61 insertions(+), 6 deletions(-) diff --git a/test/ActiveRecordFindTest.php b/test/ActiveRecordFindTest.php index 3c6fc848..ef29ec89 100644 --- a/test/ActiveRecordFindTest.php +++ b/test/ActiveRecordFindTest.php @@ -28,23 +28,44 @@ public function test_find_returns_single_model() { $author = Author::find(["name"=>"Bill Clinton"]); $this->assertInstanceOf(Model::class, $author ); - $author = Author::find(["name"=>"Bill Clinton"]); + $author = Author::find(["firstName"=>"Bill"]); + $this->assertInstanceOf(Model::class, $author ); + + $author = Author::find(["FIRSTNAME"=>"Bill"]); $this->assertInstanceOf(Model::class, $author ); $author = Author::find_by_name("Bill Clinton"); $this->assertInstanceOf(Model::class, $author ); + $author = Author::find_by_firstName("Bill"); + $this->assertInstanceOf(Model::class, $author ); + + $author = Author::find_by_FIRSTNAME("Bill"); + $this->assertInstanceOf(Model::class, $author ); + $author = Author::first(); $this->assertInstanceOf(Model::class, $author ); $author = Author::find("first", ["name"=>"Bill Clinton"]); $this->assertInstanceOf(Model::class, $author ); + $author = Author::find("first", ["firstName"=>"Bill"]); + $this->assertInstanceOf(Model::class, $author ); + + $author = Author::find("first", ["FIRSTNAME"=>"Bill"]); + $this->assertInstanceOf(Model::class, $author ); + $author = Author::last(); $this->assertInstanceOf(Model::class, $author ); $author = Author::find("last", ["name"=>"Bill Clinton"]); $this->assertInstanceOf(Model::class, $author ); + + $author = Author::find("last", ["firstName"=>"Bill"]); + $this->assertInstanceOf(Model::class, $author ); + + $author = Author::find("last", ["FIRSTNAME"=>"Bill"]); + $this->assertInstanceOf(Model::class, $author ); } public function test_find_returns_array_of_models() @@ -58,9 +79,21 @@ public function test_find_returns_array_of_models() $authors = Author::find("all", ["name" => "Bill Clinton"]); $this->assertIsArray($authors); + $authors = Author::find("all", ["firstName" => "Bill"]); + $this->assertIsArray($authors); + + $authors = Author::find("all", ["FIRSTNAME" => "Bill"]); + $this->assertIsArray($authors); + $authors = Author::find_all_by_name("Bill Clinton"); $this->assertIsArray($authors); + $authors = Author::find_all_by_firstName("Bill"); + $this->assertIsArray($authors); + + $authors = Author::find_all_by_FIRSTNAME("Bill"); + $this->assertIsArray($authors); + $authors = Author::find(1,2,3); $this->assertIsArray($authors); @@ -70,6 +103,12 @@ public function test_find_returns_array_of_models() $authors = Author::find(["conditions"=> ["name" => "Bill Clinton"]]); $this->assertIsArray($authors); + $authors = Author::find(["conditions"=> ["firstName" => "Bill"]]); + $this->assertIsArray($authors); + + $authors = Author::find(["conditions"=> ["FIRSTNAME" => "Bill"]]); + $this->assertIsArray($authors); + $authors = Author::find(['conditions'=>["author_id = ?", 3]]); $this->assertIsArray($authors); } diff --git a/test/ActiveRecordTest.php b/test/ActiveRecordTest.php index 384fc63e..42e6a78e 100644 --- a/test/ActiveRecordTest.php +++ b/test/ActiveRecordTest.php @@ -443,6 +443,19 @@ public function test_setter() $this->assertEquals(md5('plaintext'), $author->encrypted_password); } + public function test_case_insensitivity() + { + $author = new Author(); + $author->firstName = 'Peter'; + $this->assertEquals('Peter', $author->firstname); + + $author->FIRSTNAME = 'Paul'; + $this->assertEquals('Paul', $author->firstName); + + $author->FiRsTNAmE = 'Simon'; + $this->assertEquals('Simon', $author->firstNAME); + } + public function test_setter_with_same_name_as_an_attribute() { $author = new Author(); diff --git a/test/fixtures/authors.csv b/test/fixtures/authors.csv index ae219e88..24cbc54e 100644 --- a/test/fixtures/authors.csv +++ b/test/fixtures/authors.csv @@ -1,5 +1,5 @@ -author_id,parent_author_id,name -1,3,"Tito" -2,2,"George W. Bush" -3,1,"Bill Clinton" -4,2,"Uncle Bob" \ No newline at end of file +author_id,parent_author_id,name,firstName +1,3,"Tito","Tito" +2,2,"George W. Bush","George W." +3,1,"Bill Clinton","Bill" +4,2,"Uncle Bob","Uncle Bob" \ No newline at end of file diff --git a/test/sql/mysql.sql b/test/sql/mysql.sql index 493cf803..19df34db 100644 --- a/test/sql/mysql.sql +++ b/test/sql/mysql.sql @@ -2,6 +2,7 @@ CREATE TABLE authors( author_id INT NOT NULL PRIMARY KEY AUTO_INCREMENT, parent_author_id INT, name VARCHAR(25) NOT NULL DEFAULT 'default_name', + firstName VARCHAR(25) NOT NULL DEFAULT 'default_name', updated_at datetime, created_at datetime, some_Date date, diff --git a/test/sql/pgsql.sql b/test/sql/pgsql.sql index cd1825ba..6cd7275e 100644 --- a/test/sql/pgsql.sql +++ b/test/sql/pgsql.sql @@ -2,6 +2,7 @@ CREATE TABLE authors( author_id SERIAL PRIMARY KEY, parent_author_id INT, name VARCHAR(25) NOT NULL DEFAULT 'default_name', + firstName VARCHAR(25) NOT NULL DEFAULT 'default_name', updated_at timestamp, created_at timestamp, "some_Date" date, diff --git a/test/sql/sqlite.sql b/test/sql/sqlite.sql index 365dcd3d..6a0649b7 100644 --- a/test/sql/sqlite.sql +++ b/test/sql/sqlite.sql @@ -2,6 +2,7 @@ CREATE TABLE authors( author_id INTEGER NOT NULL PRIMARY KEY, parent_author_id INT, name VARCHAR (25) NOT NULL DEFAULT default_name, -- don't touch those spaces + firstName VARCHAR (25) NOT NULL DEFAULT default_name, updated_at datetime, created_at datetime, some_Date date, From 80ed0523f53ddeeb9aea56a70d26218b28189011 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Fri, 1 Sep 2023 23:21:34 -0400 Subject: [PATCH 07/27] Switched from firstName field to existing mixedCaseField to pass Postgres regression --- test/ActiveRecordFindTest.php | 28 ++++++++++++++-------------- test/ActiveRecordTest.php | 12 ++++++------ test/fixtures/authors.csv | 2 +- test/sql/mysql.sql | 5 ++--- test/sql/pgsql.sql | 5 ++--- test/sql/sqlite.sql | 5 ++--- 6 files changed, 27 insertions(+), 30 deletions(-) diff --git a/test/ActiveRecordFindTest.php b/test/ActiveRecordFindTest.php index ef29ec89..51b2e119 100644 --- a/test/ActiveRecordFindTest.php +++ b/test/ActiveRecordFindTest.php @@ -28,19 +28,19 @@ public function test_find_returns_single_model() { $author = Author::find(["name"=>"Bill Clinton"]); $this->assertInstanceOf(Model::class, $author ); - $author = Author::find(["firstName"=>"Bill"]); + $author = Author::find(["mixedCaseField"=>"Bill"]); $this->assertInstanceOf(Model::class, $author ); - $author = Author::find(["FIRSTNAME"=>"Bill"]); + $author = Author::find(["MIXEDCASEFIELD"=>"Bill"]); $this->assertInstanceOf(Model::class, $author ); $author = Author::find_by_name("Bill Clinton"); $this->assertInstanceOf(Model::class, $author ); - $author = Author::find_by_firstName("Bill"); + $author = Author::find_by_mixedCaseField("Bill"); $this->assertInstanceOf(Model::class, $author ); - $author = Author::find_by_FIRSTNAME("Bill"); + $author = Author::find_by_MIXEDCASEFIELD("Bill"); $this->assertInstanceOf(Model::class, $author ); $author = Author::first(); @@ -49,10 +49,10 @@ public function test_find_returns_single_model() { $author = Author::find("first", ["name"=>"Bill Clinton"]); $this->assertInstanceOf(Model::class, $author ); - $author = Author::find("first", ["firstName"=>"Bill"]); + $author = Author::find("first", ["mixedCaseField"=>"Bill"]); $this->assertInstanceOf(Model::class, $author ); - $author = Author::find("first", ["FIRSTNAME"=>"Bill"]); + $author = Author::find("first", ["MIXEDCASEFIELD"=>"Bill"]); $this->assertInstanceOf(Model::class, $author ); $author = Author::last(); @@ -61,10 +61,10 @@ public function test_find_returns_single_model() { $author = Author::find("last", ["name"=>"Bill Clinton"]); $this->assertInstanceOf(Model::class, $author ); - $author = Author::find("last", ["firstName"=>"Bill"]); + $author = Author::find("last", ["mixedCaseField"=>"Bill"]); $this->assertInstanceOf(Model::class, $author ); - $author = Author::find("last", ["FIRSTNAME"=>"Bill"]); + $author = Author::find("last", ["MIXEDCASEFIELD"=>"Bill"]); $this->assertInstanceOf(Model::class, $author ); } @@ -79,19 +79,19 @@ public function test_find_returns_array_of_models() $authors = Author::find("all", ["name" => "Bill Clinton"]); $this->assertIsArray($authors); - $authors = Author::find("all", ["firstName" => "Bill"]); + $authors = Author::find("all", ["mixedCaseField" => "Bill"]); $this->assertIsArray($authors); - $authors = Author::find("all", ["FIRSTNAME" => "Bill"]); + $authors = Author::find("all", ["MIXEDCASEFIELD" => "Bill"]); $this->assertIsArray($authors); $authors = Author::find_all_by_name("Bill Clinton"); $this->assertIsArray($authors); - $authors = Author::find_all_by_firstName("Bill"); + $authors = Author::find_all_by_mixedCaseField("Bill"); $this->assertIsArray($authors); - $authors = Author::find_all_by_FIRSTNAME("Bill"); + $authors = Author::find_all_by_MIXEDCASEFIELD("Bill"); $this->assertIsArray($authors); $authors = Author::find(1,2,3); @@ -103,10 +103,10 @@ public function test_find_returns_array_of_models() $authors = Author::find(["conditions"=> ["name" => "Bill Clinton"]]); $this->assertIsArray($authors); - $authors = Author::find(["conditions"=> ["firstName" => "Bill"]]); + $authors = Author::find(["conditions"=> ["mixedCaseField" => "Bill"]]); $this->assertIsArray($authors); - $authors = Author::find(["conditions"=> ["FIRSTNAME" => "Bill"]]); + $authors = Author::find(["conditions"=> ["MIXEDCASEFIELD" => "Bill"]]); $this->assertIsArray($authors); $authors = Author::find(['conditions'=>["author_id = ?", 3]]); diff --git a/test/ActiveRecordTest.php b/test/ActiveRecordTest.php index 42e6a78e..7e6f4805 100644 --- a/test/ActiveRecordTest.php +++ b/test/ActiveRecordTest.php @@ -446,14 +446,14 @@ public function test_setter() public function test_case_insensitivity() { $author = new Author(); - $author->firstName = 'Peter'; - $this->assertEquals('Peter', $author->firstname); + $author->mixedCaseField = 'Peter'; + $this->assertEquals('Peter', $author->mixedcasefield); - $author->FIRSTNAME = 'Paul'; - $this->assertEquals('Paul', $author->firstName); + $author->MIXEDCASEFIELD = 'Paul'; + $this->assertEquals('Paul', $author->mixedCaseFIELD); - $author->FiRsTNAmE = 'Simon'; - $this->assertEquals('Simon', $author->firstNAME); + $author->mixedcasefield = 'Simon'; + $this->assertEquals('Simon', $author->MIXEDCASEFIELD); } public function test_setter_with_same_name_as_an_attribute() diff --git a/test/fixtures/authors.csv b/test/fixtures/authors.csv index 24cbc54e..ea124cf2 100644 --- a/test/fixtures/authors.csv +++ b/test/fixtures/authors.csv @@ -1,4 +1,4 @@ -author_id,parent_author_id,name,firstName +author_id,parent_author_id,name,mixedCaseField 1,3,"Tito","Tito" 2,2,"George W. Bush","George W." 3,1,"Bill Clinton","Bill" diff --git a/test/sql/mysql.sql b/test/sql/mysql.sql index 19df34db..5d9d7023 100644 --- a/test/sql/mysql.sql +++ b/test/sql/mysql.sql @@ -2,15 +2,14 @@ CREATE TABLE authors( author_id INT NOT NULL PRIMARY KEY AUTO_INCREMENT, parent_author_id INT, name VARCHAR(25) NOT NULL DEFAULT 'default_name', - firstName VARCHAR(25) NOT NULL DEFAULT 'default_name', + mixedCaseField varchar(50), updated_at datetime, created_at datetime, some_Date date, some_time time, some_text text, some_enum enum('a','b','c'), - encrypted_password varchar(50), - mixedCaseField varchar(50) + encrypted_password varchar(50) ) ENGINE=InnoDB; CREATE TABLE honest_lawyers ( diff --git a/test/sql/pgsql.sql b/test/sql/pgsql.sql index 6cd7275e..3a8d25dc 100644 --- a/test/sql/pgsql.sql +++ b/test/sql/pgsql.sql @@ -2,14 +2,13 @@ CREATE TABLE authors( author_id SERIAL PRIMARY KEY, parent_author_id INT, name VARCHAR(25) NOT NULL DEFAULT 'default_name', - firstName VARCHAR(25) NOT NULL DEFAULT 'default_name', + "mixedCaseField" varchar(50), updated_at timestamp, created_at timestamp, "some_Date" date, some_time time, some_text text, - encrypted_password varchar(50), - "mixedCaseField" varchar(50) + encrypted_password varchar(50) ); CREATE TABLE honest_lawyers ( diff --git a/test/sql/sqlite.sql b/test/sql/sqlite.sql index 6a0649b7..02a7e4fa 100644 --- a/test/sql/sqlite.sql +++ b/test/sql/sqlite.sql @@ -2,14 +2,13 @@ CREATE TABLE authors( author_id INTEGER NOT NULL PRIMARY KEY, parent_author_id INT, name VARCHAR (25) NOT NULL DEFAULT default_name, -- don't touch those spaces - firstName VARCHAR (25) NOT NULL DEFAULT default_name, + mixedCaseField varchar(50), updated_at datetime, created_at datetime, some_Date date, some_time time, some_text text, - encrypted_password varchar(50), - mixedCaseField varchar(50) + encrypted_password varchar(50) ); CREATE TABLE honest_lawyers( From b8ecfc0a906060c97344592e4ac443a8047bfe3b Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Fri, 1 Sep 2023 23:41:47 -0400 Subject: [PATCH 08/27] - List things in alphabetical order - Correct link to contributing.md --- README.md | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index b6d7ec02..bdd59a93 100644 --- a/README.md +++ b/README.md @@ -44,22 +44,21 @@ Of course, there are some differences which will be obvious to the user if they ## Supported Databases ## - MySQL -- SQLite - PostgreSQL +- SQLite ## Features ## -- Finder methods -- Dynamic finder methods -- Writer methods -- Relationships -- Validations - Callbacks +- Model Caching +- Database adapter plugins +- Finder methods, static and dynamic +- Relationships - Serializations (json/xml) - Transactions -- Support for multiple adapters +- Validations +- Writer methods - Miscellaneous options such as: aliased/protected/accessible attributes -- Model Caching ## Installation ## @@ -150,4 +149,4 @@ echo $post->title; # 'New real title' ## Contributing ## -Please refer to [CONTRIBUTING.md](https://github.com/jpfuentes2/php-activerecord/blob/master/CONTRIBUTING.md) for information on how to contribute to PHP ActiveRecord. +Please refer to [CONTRIBUTING.md](https://github.com/php-activerecord/activerecord/blob/master/CONTRIBUTING.md) for information on how to contribute to PHP ActiveRecord. From 23d00a73297700660a94d62e272f1972d7feca6d Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Sat, 2 Sep 2023 14:35:27 -0400 Subject: [PATCH 09/27] Revert updates to test suite as ActiveRecordTest::test_case_insensitivity() is enough. --- test/ActiveRecordFindTest.php | 41 ----------------------------------- test/ActiveRecordTest.php | 14 ++++++------ test/SerializationTest.php | 2 ++ test/models/Author.php | 1 + test/models/Book.php | 8 +++---- test/models/Venue.php | 4 ++++ 6 files changed, 18 insertions(+), 52 deletions(-) diff --git a/test/ActiveRecordFindTest.php b/test/ActiveRecordFindTest.php index 51b2e119..bde65669 100644 --- a/test/ActiveRecordFindTest.php +++ b/test/ActiveRecordFindTest.php @@ -28,44 +28,21 @@ public function test_find_returns_single_model() { $author = Author::find(["name"=>"Bill Clinton"]); $this->assertInstanceOf(Model::class, $author ); - $author = Author::find(["mixedCaseField"=>"Bill"]); - $this->assertInstanceOf(Model::class, $author ); - - $author = Author::find(["MIXEDCASEFIELD"=>"Bill"]); - $this->assertInstanceOf(Model::class, $author ); $author = Author::find_by_name("Bill Clinton"); $this->assertInstanceOf(Model::class, $author ); - $author = Author::find_by_mixedCaseField("Bill"); - $this->assertInstanceOf(Model::class, $author ); - - $author = Author::find_by_MIXEDCASEFIELD("Bill"); - $this->assertInstanceOf(Model::class, $author ); - $author = Author::first(); $this->assertInstanceOf(Model::class, $author ); $author = Author::find("first", ["name"=>"Bill Clinton"]); $this->assertInstanceOf(Model::class, $author ); - $author = Author::find("first", ["mixedCaseField"=>"Bill"]); - $this->assertInstanceOf(Model::class, $author ); - - $author = Author::find("first", ["MIXEDCASEFIELD"=>"Bill"]); - $this->assertInstanceOf(Model::class, $author ); - $author = Author::last(); $this->assertInstanceOf(Model::class, $author ); $author = Author::find("last", ["name"=>"Bill Clinton"]); $this->assertInstanceOf(Model::class, $author ); - - $author = Author::find("last", ["mixedCaseField"=>"Bill"]); - $this->assertInstanceOf(Model::class, $author ); - - $author = Author::find("last", ["MIXEDCASEFIELD"=>"Bill"]); - $this->assertInstanceOf(Model::class, $author ); } public function test_find_returns_array_of_models() @@ -79,21 +56,9 @@ public function test_find_returns_array_of_models() $authors = Author::find("all", ["name" => "Bill Clinton"]); $this->assertIsArray($authors); - $authors = Author::find("all", ["mixedCaseField" => "Bill"]); - $this->assertIsArray($authors); - - $authors = Author::find("all", ["MIXEDCASEFIELD" => "Bill"]); - $this->assertIsArray($authors); - $authors = Author::find_all_by_name("Bill Clinton"); $this->assertIsArray($authors); - $authors = Author::find_all_by_mixedCaseField("Bill"); - $this->assertIsArray($authors); - - $authors = Author::find_all_by_MIXEDCASEFIELD("Bill"); - $this->assertIsArray($authors); - $authors = Author::find(1,2,3); $this->assertIsArray($authors); @@ -103,12 +68,6 @@ public function test_find_returns_array_of_models() $authors = Author::find(["conditions"=> ["name" => "Bill Clinton"]]); $this->assertIsArray($authors); - $authors = Author::find(["conditions"=> ["mixedCaseField" => "Bill"]]); - $this->assertIsArray($authors); - - $authors = Author::find(["conditions"=> ["MIXEDCASEFIELD" => "Bill"]]); - $this->assertIsArray($authors); - $authors = Author::find(['conditions'=>["author_id = ?", 3]]); $this->assertIsArray($authors); } diff --git a/test/ActiveRecordTest.php b/test/ActiveRecordTest.php index 7e6f4805..3922f80e 100644 --- a/test/ActiveRecordTest.php +++ b/test/ActiveRecordTest.php @@ -399,8 +399,8 @@ public function test_delegate_returns_null_if_relationship_does_not_exist() public function test_delegate_set_attribute() { $event = Event::first(); - $event->state = 'Mexico'; - $this->assertEquals('Mexico', $event->venue->state); + $event->state = 'MEXICO'; + $this->assertEquals('MEXICO', $event->venue->state); } public function test_delegate_getter_gh_98() @@ -408,8 +408,8 @@ public function test_delegate_getter_gh_98() Venue::$use_custom_get_state_getter = true; $event = Event::first(); - $this->assertEquals('NY', $event->venue->state); - $this->assertEquals('NY', $event->state); + $this->assertEquals('ny', $event->venue->state); + $this->assertEquals('ny', $event->state); Venue::$use_custom_get_state_getter = false; } @@ -419,8 +419,8 @@ public function test_delegate_setter_gh_98() Venue::$use_custom_set_state_setter = true; $event = Event::first(); - $event->state = 'Mexico'; - $this->assertEquals('Mexico#', $event->venue->state); + $event->state = 'MEXICO'; + $this->assertEquals('MEXICO#', $event->venue->state); Venue::$use_custom_set_state_setter = false; } @@ -460,7 +460,7 @@ public function test_setter_with_same_name_as_an_attribute() { $author = new Author(); $author->name = 'bob'; - $this->assertEquals('bob', $author->name); + $this->assertEquals('BOB', $author->name); } public function test_getter() diff --git a/test/SerializationTest.php b/test/SerializationTest.php index fcd3f00c..1012c405 100644 --- a/test/SerializationTest.php +++ b/test/SerializationTest.php @@ -36,6 +36,8 @@ public function test_only_should_only_apply_to_attributes() { $this->assertArrayHasKey('author', $this->_a(['only' => 'name', 'include' => 'author'])); $this->assertArrayHasKey('name', $this->_a(['only' => 'name', 'include' => 'author'])); + $this->assertArrayHasKey('book_id', $this->_a(['only' => 'book_id', 'methods' => 'upper_name'])); + $this->assertArrayHasKey('upper_name', $this->_a(['only' => 'book_id', 'methods' => 'upper_name'])); } public function test_only_overrides_except() diff --git a/test/models/Author.php b/test/models/Author.php index 9bfcbc3e..00bce6dd 100644 --- a/test/models/Author.php +++ b/test/models/Author.php @@ -22,6 +22,7 @@ public function set_password($plaintext) public function set_name($value) { + $value = strtoupper($value); $this->assign_attribute('name', $value); } diff --git a/test/models/Book.php b/test/models/Book.php index e87b5939..8203d7a5 100644 --- a/test/models/Book.php +++ b/test/models/Book.php @@ -9,22 +9,22 @@ class Book extends Model public static $has_one = []; public function upper_name() { - return strtoupper($this->name); // keep? + return strtoupper($this->name); } public function name() { - return strtolower($this->name); // keep + return strtolower($this->name); } public function get_publisher() { - return strtoupper($this->read_attribute('publisher')); // keep + return strtoupper($this->read_attribute('publisher')); } public function get_upper_name() { - return strtoupper($this->name); // keep + return strtoupper($this->name); } public function get_lower_name() diff --git a/test/models/Venue.php b/test/models/Venue.php index 70fa6f3a..20681a75 100644 --- a/test/models/Venue.php +++ b/test/models/Venue.php @@ -21,6 +21,10 @@ class Venue extends Model public function get_state() { + if (self::$use_custom_get_state_getter) { + return strtolower($this->read_attribute('state')); + } + return $this->read_attribute('state'); } From d42a7d1b12bf31e37fe1e9364bc6d5f1ab064630 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Sun, 3 Sep 2023 22:12:40 -0400 Subject: [PATCH 10/27] Implemented composer test --- composer.json | 17 +++++----- scripts/TestCommand.php | 75 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 8 deletions(-) create mode 100644 scripts/TestCommand.php diff --git a/composer.json b/composer.json index 1af3bc50..53249b37 100644 --- a/composer.json +++ b/composer.json @@ -23,16 +23,17 @@ ], "psr-4": { "ActiveRecord\\": "lib/", + "scripts\\": "scripts/", "test\\": "test/" } }, - "scripts": { - "style-check" : "php vendor/bin/php-cs-fixer fix --dry-run --verbose --diff", - "style-fix" : "php vendor/bin/php-cs-fixer fix --verbose", - "test": [ - "Composer\\Config::disableProcessTimeout", - "phpunit" - ], - "stan": "phpstan analyse --ansi --memory-limit 256M" + "scripts": { + "style-check" : "php vendor/bin/php-cs-fixer fix --dry-run --verbose --diff", + "style-fix" : "php vendor/bin/php-cs-fixer fix --verbose", + "stan": "phpstan analyse --ansi --memory-limit 256M", + "test": [ + "Composer\\Config::disableProcessTimeout", + "scripts\\TestCommand" + ] } } diff --git a/scripts/TestCommand.php b/scripts/TestCommand.php new file mode 100644 index 00000000..ccee3eb8 --- /dev/null +++ b/scripts/TestCommand.php @@ -0,0 +1,75 @@ +setDefinition([ + new InputArgument('fileName', InputArgument::OPTIONAL), + new InputArgument('filter', InputArgument::OPTIONAL), + ]); + } + + public function execute(InputInterface $input, OutputInterface $output): int + { + try { + $args = $this->buildArgs($input->getArgument('fileName'), $input->getArgument('filter')); + + $output->writeln("Running: \n" . implode(' ', $args) . "\n"); + $process = new Process($args); + $process->setTimeout(1200); + + if (Process::isTtySupported()) { + $process->setTty(true); + $process->run(); + } else { + $process->run(function ($type, $buffer): void { + echo $buffer; + }); + } + + return 0; + } catch (\Exception $e) { + $output->writeln($e->getMessage()); + + return 1; + } + } + + private function buildArgs(string|null $fileName, string|null $filter): array + { + $args = ['vendor/bin/phpunit']; + + if (null === $fileName) { + return $args; + } + + if (str_ends_with($fileName, '.php')) { + $fileName = substr($fileName, 0, -strlen('.php')); + } + if (str_ends_with($fileName, 'Test')) { + $fileName = substr($fileName, 0, -strlen('Test')); + } + $fileName = ucfirst($fileName); + $fileName = "test/{$fileName}Test.php"; + if (!file_exists($fileName)) { + throw new \Exception("{$fileName} does not exist. Did you mispell it?"); + } + + if (null != $filter) { + array_push($args, '--filter'); + array_push($args, $filter); + } + array_push($args, $fileName); + + return $args; + } +} From 650d3689c7560c2d84a661a4a31fcd1ad08bfbb3 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 4 Sep 2023 17:31:45 -0400 Subject: [PATCH 11/27] Fix style issues --- scripts/TestCommand.php | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/TestCommand.php b/scripts/TestCommand.php index b832904b..8ccf5015 100644 --- a/scripts/TestCommand.php +++ b/scripts/TestCommand.php @@ -1,4 +1,5 @@ Date: Tue, 5 Sep 2023 11:40:03 -0400 Subject: [PATCH 12/27] Fix for issue #55 --- lib/Model.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Model.php b/lib/Model.php index 94db2e08..061d41b2 100644 --- a/lib/Model.php +++ b/lib/Model.php @@ -1791,7 +1791,7 @@ public static function find(/* $type, $options */): static|array|null $num_args = count($args); $single = true; - if (in_array($args[0], ['all', 'first', 'last'])) { + if (count($args) > 0 && in_array($args[0], ['all', 'first', 'last'])) { switch ($args[0]) { case 'all': $single = false; From c7725780a32baa39a67da110c859c7e154232bf6 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Tue, 5 Sep 2023 11:45:41 -0400 Subject: [PATCH 13/27] Remove unused scripts and files references. --- composer.json | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 9c919af6..d6479867 100644 --- a/composer.json +++ b/composer.json @@ -22,14 +22,10 @@ "lib/Utils.php" ], "psr-4": { - "ActiveRecord\\": "lib/", - "scripts\\": "scripts/" + "ActiveRecord\\": "lib/" } }, "autoload-dev": { - "files": [ - "scripts/TestCommand.php" - ], "psr-4": { "ActiveRecord\\Scripts\\": "scripts/", "test\\": "test/" From 64e45b876fbd7751a88963e0077b99df7e200260 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Tue, 5 Sep 2023 11:50:18 -0400 Subject: [PATCH 14/27] Remove unused items in composer.json --- composer.json | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 9c919af6..d6479867 100644 --- a/composer.json +++ b/composer.json @@ -22,14 +22,10 @@ "lib/Utils.php" ], "psr-4": { - "ActiveRecord\\": "lib/", - "scripts\\": "scripts/" + "ActiveRecord\\": "lib/" } }, "autoload-dev": { - "files": [ - "scripts/TestCommand.php" - ], "psr-4": { "ActiveRecord\\Scripts\\": "scripts/", "test\\": "test/" From 09f07327eee28b4e9261e53c983a27bc972f3f35 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Thu, 14 Sep 2023 00:43:38 -0400 Subject: [PATCH 15/27] Document composer style-fix --- CONTRIBUTING.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 08487d82..3115f0a0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -94,6 +94,10 @@ If Docker is not available to you, or you would simply not use it, you will have ```sh composer style-check ``` +and fix it with: +```sh +composer style-fix +``` #### Static analysis From 99a7541fb882747fe0f8c989cfa9db8176c014aa Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 18 Sep 2023 10:40:28 -0400 Subject: [PATCH 16/27] Support dynamic finders in Relation so that Author::select('author_id')->find_by_name('Tito') is possible. --- lib/Model.php | 48 ++------------------------- lib/Relation.php | 84 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 45 deletions(-) diff --git a/lib/Model.php b/lib/Model.php index 52e17440..b755a711 100644 --- a/lib/Model.php +++ b/lib/Model.php @@ -1513,53 +1513,11 @@ public function reset_dirty(): void * * @throws ActiveRecordException * - * @see find + * @see Relation->__call() */ - public static function __callStatic(string $method, mixed $args): mixed - { - /** - * @var RelationOptions - */ - $options = []; - $create = false; - - if ($attributes = static::extract_dynamic_vars($method, 'find_or_create_by')) { - // can't take any finders with OR in it when doing a find_or_create_by - if (false !== strpos($attributes, '_or_')) { - throw new ActiveRecordException("Cannot use OR'd attributes in find_or_create_by"); - } - $create = true; - $method = 'find_by_' . $attributes; - } - - $options['conditions'] ??= []; - - if ($attributes = static::extract_dynamic_vars($method, 'find_by')) { - $options['conditions'][] = WhereClause::from_underscored_string(static::connection(), $attributes, $args, static::$alias_attribute); - - $rel = static::Relation($options); - if (!($ret = $rel->first()) && $create) { - return static::create(SQLBuilder::create_hash_from_underscored_string( - $attributes, - $args, - static::$alias_attribute)); - } - - return $ret; - } - - throw new ActiveRecordException("Call to undefined method: $method"); - } - - public static function extract_dynamic_vars(string $methodName, string $dynamicPart): string + public static function __callStatic(string $method, mixed $args): static|null { - if (str_starts_with($methodName, $dynamicPart)) { - $attributes = substr($methodName, strlen($dynamicPart) +1); - - return $attributes; - } - - return ''; + return static::Relation()->$method(...$args); } /** diff --git a/lib/Relation.php b/lib/Relation.php index a4b268cd..4ee01119 100644 --- a/lib/Relation.php +++ b/lib/Relation.php @@ -7,6 +7,7 @@ namespace ActiveRecord; +use ActiveRecord\Exception\ActiveRecordException; use ActiveRecord\Exception\RecordNotFound; use ActiveRecord\Exception\ValidationsArgumentError; @@ -106,6 +107,89 @@ public function none(): Relation return $this; } + /** + * Enables the use of dynamic finders. + * + * Dynamic finders are just an easy way to do queries quickly without having to + * specify an options array with conditions in it. + * + * ``` + * SomeModel::select('author_id')->find_by_first_name('Tito'); + * SomeModel::select('author_id')->find_by_first_name_and_last_name('Tito','the Grief'); + * SomeModel::select('author_id')->find_by_first_name_or_last_name('Tito','the Grief'); + * ``` + * + * You can also create the model if the find call returned no results: + * + * ``` + * Person::find_or_create_by_name('Tito'); + * + * # would be the equivalent of + * if (!Person::select('author_id')->find_by_name('Tito')) + * Person::create(['Tito']); + * ``` + * + * Some other examples of find_or_create_by: + * + * ``` + * Person::select('id')->find_or_create_by_name_and_id('Tito',1); + * Person::select('id')->find_or_create_by_name_and_id(['name' => 'Tito', 'id' => 1]); + * ``` + * + * @param $method Name of method + * @param $args Method args + * + * @throws ActiveRecordException + * + * @see find + */ + public function __call(string $method, mixed $args): mixed + { + $create = false; + + if ($attributes = $this->extract_dynamic_vars($method, 'find_or_create_by')) { + // can't take any finders with OR in it when doing a find_or_create_by + if (false !== strpos($attributes, '_or_')) { + throw new ActiveRecordException("Cannot use OR'd attributes in find_or_create_by"); + } + $create = true; + $method = 'find_by_' . $attributes; + } + + $attributes = $this->extract_dynamic_vars($method, 'find_by'); + if ('' === $attributes) { + throw new ActiveRecordException("Call to undefined method: $method"); + } + + $this->options['conditions'] ??= []; + $this->options['conditions'][] = WhereClause::from_underscored_string($this->table()->conn, $attributes, $args, $this->alias_attribute); + + $ret = $this->firstOrLast(1, true); + if (0 === count($ret)) { + if ($create) { + return $this->className::create(SQLBuilder::create_hash_from_underscored_string( + $attributes, + $args, + $this->alias_attribute)); + } + + return null; + } + + return $ret[0]; + } + + private function extract_dynamic_vars(string $methodName, string $dynamicPart): string + { + if (str_starts_with($methodName, $dynamicPart)) { + $attributes = substr($methodName, strlen($dynamicPart) +1); + + return $attributes; + } + + return ''; + } + /** * Use pluck as a shortcut to select one or more attributes without * loading an entire record object per row. From 75387d8d6674dbd0b09f730675ec7803bfff88fd Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 18 Sep 2023 10:42:29 -0400 Subject: [PATCH 17/27] - Moved FindBy and From tests from ActiveRecordFindTest.php to ActiveRecordFindByTest.php and ActiveRecordFromTest.php respectively - In doing the above, found and removed duplicated tests - Added test for Relation::testFindBySelect() --- test/ActiveRecordFindByTest.php | 31 +++++++++++++++++ test/ActiveRecordFindTest.php | 60 ++------------------------------- test/ActiveRecordFromTest.php | 21 ++++++++++++ 3 files changed, 55 insertions(+), 57 deletions(-) diff --git a/test/ActiveRecordFindByTest.php b/test/ActiveRecordFindByTest.php index 4d0e58f3..84083691 100644 --- a/test/ActiveRecordFindByTest.php +++ b/test/ActiveRecordFindByTest.php @@ -23,12 +23,25 @@ public function testFindByWithInvalidFieldName() public function testFindBy() { + $author = Author::find_by_name('Tito'); + $this->assertInstanceOf(Author::class, $author); $this->assertEquals('Tito', Author::find_by_name('Tito')->name); + $this->assertEquals('Tito', Author::find_by_author_id_and_name(1, 'Tito')->name); $this->assertEquals('George W. Bush', Author::find_by_author_id_or_name(2, 'Barney')->name); $this->assertEquals('Tito', Author::find_by_name(['Tito', 'George W. Bush'])->name); } + public function testFindBySelect() + { + $author = Author::select('author_id')->find_by_name('Tito'); + $this->assertInstanceOf(Author::class, $author); + $this->assertEquals(1, $author->author_id); + + $this->expectException(ActiveRecordException::class); + $author->name; + } + public function testFindByNoResults() { $this->assertNull(Author::find_by_name('SHARKS WIT LASERZ')); @@ -45,4 +58,22 @@ public function testDynamicFinderUsingAlias() { $this->assertNotNull(Venue::find_by_marquee('Warner Theatre')); } + + public function testFindOrCreateByOnExistingRecord() + { + $this->assertNotNull(Author::find_or_create_by_name('Tito')); + } + + public function testFindOrCreateByCreatesNewRecord() + { + $author = Author::find_or_create_by_name_and_encrypted_password('New Guy', 'pencil'); + $this->assertEquals(Author::last()->author_id, $author->author_id); + $this->assertEquals('pencil', $author->encrypted_password); + } + + public function testFindOrCreateByThrowsExceptionWhenUsingOr() + { + $this->expectException(ActiveRecordException::class); + Author::find_or_create_by_name_or_encrypted_password('New Guy', 'pencil'); + } } diff --git a/test/ActiveRecordFindTest.php b/test/ActiveRecordFindTest.php index 2aaf264f..2e74b4be 100644 --- a/test/ActiveRecordFindTest.php +++ b/test/ActiveRecordFindTest.php @@ -1,6 +1,5 @@ assertInstanceOf(Model::class, $author); - $authors = Author::where(['name'=>'Bill Clinton'])->to_a(); - $this->assertIsArray($authors); - - $author = Author::find_by_name('Bill Clinton'); - $this->assertInstanceOf(Author::class, $author); - $author = Author::first(); $this->assertInstanceOf(Author::class, $author); @@ -67,8 +60,7 @@ public function testFindReturnContents() public function testFindReturnsArrayOfModels() { - $authors = Author::all()->to_a(); - $this->assertIsArray($authors); + $this->assertEquals(2, count(Author::where(['name' => 'Tito'])->to_a())); $authors = Author::find(1, 2, 3); $this->assertIsArray($authors); @@ -291,53 +283,7 @@ public function testJoinsOnModelWithExplicitJoins() $this->assert_sql_has('LEFT JOIN authors a ON(books.secondary_author_id=a.author_id)', JoinBook::table()->last_sql); } - public function testFrom() - { - $author = Author::from('books') - ->order('author_id asc') - ->first(); - $this->assertInstanceOf(Author::class, $author); - $this->assertNotNull($author->book_id); - - $author = Author::from('authors') - ->order('author_id asc') - ->first(); - $this->assertInstanceOf(Author::class, $author); - $this->assertEquals(1, $author->id); - } - - public function testFromWithInvalidTable() - { - $this->expectException(DatabaseException::class); - Author::from('wrong_authors_table')->first(); - } - - public function testFindWithHash() - { - $this->assertNotNull(Author::where(['name' => 'Tito'])->first()); - $this->assertNull(Author::where(['name' => 'Mortimer'])->first()); - $this->assertEquals(2, count(Author::where(['name' => 'Tito'])->to_a())); - } - - public function testFindOrCreateByOnExistingRecord() - { - $this->assertNotNull(Author::find_or_create_by_name('Tito')); - } - - public function testFindOrCreateByCreatesNewRecord() - { - $author = Author::find_or_create_by_name_and_encrypted_password('New Guy', 'pencil'); - $this->assertTrue($author->author_id > 0); - $this->assertEquals('pencil', $author->encrypted_password); - } - - public function testFindOrCreateByThrowsExceptionWhenUsingOr() - { - $this->expectException(ActiveRecordException::class); - Author::find_or_create_by_name_or_encrypted_password('New Guy', 'pencil'); - } - - public function testFindByZero() + public function testFindNonExistentPrimaryKey() { $this->expectException(RecordNotFound::class); Author::find(0); @@ -361,7 +307,7 @@ public function testFindByPkShouldNotUseLimit() $this->assert_sql_has('SELECT * FROM authors WHERE author_id = ?', Author::table()->last_sql); } - public function testFindByDatetime() + public function testFindsDatetime() { $now = new DateTime(); $arnow = new ActiveRecord\DateTime(); diff --git a/test/ActiveRecordFromTest.php b/test/ActiveRecordFromTest.php index 5ee602b7..5fb63d61 100644 --- a/test/ActiveRecordFromTest.php +++ b/test/ActiveRecordFromTest.php @@ -6,6 +6,27 @@ class ActiveRecordFromTest extends \DatabaseTestCase { + public function testFrom() + { + $author = Author::from('books') + ->order('author_id asc') + ->first(); + $this->assertInstanceOf(Author::class, $author); + $this->assertNotNull($author->book_id); + + $author = Author::from('authors') + ->order('author_id asc') + ->first(); + $this->assertInstanceOf(Author::class, $author); + $this->assertEquals(1, $author->id); + } + + public function testFromWithInvalidTable() + { + $this->expectException(DatabaseException::class); + Author::from('wrong_authors_table')->first(); + } + public function testSimpleTableName(): void { $venues = Venue::from('events'); From fc5e51aef4b1f09dbd51e3a251f7a4452dde86cc Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 18 Sep 2023 10:52:18 -0400 Subject: [PATCH 18/27] Fix imports to pass regression. --- test/ActiveRecordFromTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/ActiveRecordFromTest.php b/test/ActiveRecordFromTest.php index 5fb63d61..9b1c954b 100644 --- a/test/ActiveRecordFromTest.php +++ b/test/ActiveRecordFromTest.php @@ -2,6 +2,8 @@ namespace test; +use ActiveRecord\Exception\DatabaseException; +use test\models\Author; use test\models\Venue; class ActiveRecordFromTest extends \DatabaseTestCase From 375947bd7e83accac595a595886c8b1868df7299 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 18 Sep 2023 12:31:18 -0400 Subject: [PATCH 19/27] Improve code coverage in Model --- lib/Model.php | 9 ++++----- test/ActiveRecordTest.php | 11 +++++++++++ test/models/Venue.php | 7 +++++++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/lib/Model.php b/lib/Model.php index b755a711..a49784e5 100644 --- a/lib/Model.php +++ b/lib/Model.php @@ -688,16 +688,15 @@ public function &read_attribute(string $name) * * @return Model|array|null */ - protected function initRelationships(string $name): mixed + protected function initRelationships(string $name): Model|array|null { $table = static::table(); - if ($relationship = $table->get_relationship($name)) { + $relationship = $table->get_relationship($name); + if (null !== $relationship) { $this->__relationships[$name] = $relationship->load($this); - - return $this->__relationships[$name]; } - return null; + return array_key_exists($name, $this->__relationships) ? $this->__relationships[$name] : null; } /** diff --git a/test/ActiveRecordTest.php b/test/ActiveRecordTest.php index b6efe85f..ba1933bd 100644 --- a/test/ActiveRecordTest.php +++ b/test/ActiveRecordTest.php @@ -2,6 +2,7 @@ use ActiveRecord\Exception\ActiveRecordException; use ActiveRecord\Exception\ReadOnlyException; +use ActiveRecord\Exception\RelationshipException; use ActiveRecord\Exception\UndefinedPropertyException; use test\models\Author; use test\models\AwesomePerson; @@ -55,6 +56,13 @@ public function testGetterUndefinedPropertyExceptionIncludesModelName() }); } + public function testUnknownRelationship() + { + $this->expectException(RelationshipException::class); + $author = new Author(); + $author->set_relationship_from_eager_load(null, 'unknown'); + } + public function testMassAssignmentUndefinedPropertyExceptionIncludesModelName() { $this->assert_exception_message_contains('Author->this_better_not_exist', function () { @@ -239,6 +247,9 @@ public function testIsset() $this->assertTrue(isset($book->name)); $this->assertFalse(isset($book->sharks)); + $venue = new Venue(); + $this->assertTrue(isset($venue->customState)); + $author = Author::find(1); $this->assertTrue(isset($author->awesome_person)); } diff --git a/test/models/Venue.php b/test/models/Venue.php index ad394ecd..76ff6932 100644 --- a/test/models/Venue.php +++ b/test/models/Venue.php @@ -40,4 +40,11 @@ public function set_state($value) return $this->assign_attribute('state', $value); } + + public function get_customState() + { + static::$use_custom_get_state_getter = true; + + return $this->get_state(); + } } From 917788498556d896d566d13a1e67f5cc261a28af Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 18 Sep 2023 15:04:27 -0400 Subject: [PATCH 20/27] Cleanup as per code review --- lib/Model.php | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/lib/Model.php b/lib/Model.php index a49784e5..cd5ea2a8 100644 --- a/lib/Model.php +++ b/lib/Model.php @@ -696,7 +696,7 @@ protected function initRelationships(string $name): Model|array|null $this->__relationships[$name] = $relationship->load($this); } - return array_key_exists($name, $this->__relationships) ? $this->__relationships[$name] : null; + return $this->__relationships[$name] ?? null; } /** @@ -1481,37 +1481,6 @@ public function reset_dirty(): void /** * Enables the use of dynamic finders. * - * Dynamic finders are just an easy way to do queries quickly without having to - * specify an options array with conditions in it. - * - * ``` - * SomeModel::find_by_first_name('Tito'); - * SomeModel::find_by_first_name_and_last_name('Tito','the Grief'); - * SomeModel::find_by_first_name_or_last_name('Tito','the Grief'); - * ``` - * - * You can also create the model if the find call returned no results: - * - * ``` - * Person::find_or_create_by_name('Tito'); - * - * # would be the equivalent of - * if (!Person::find_by_name('Tito')) - * Person::create(['Tito']); - * ``` - * - * Some other examples of find_or_create_by: - * - * ``` - * Person::find_or_create_by_name_and_id('Tito',1); - * Person::find_or_create_by_name_and_id(['name' => 'Tito', 'id' => 1]); - * ``` - * - * @param $method Name of method - * @param $args Method args - * - * @throws ActiveRecordException - * * @see Relation->__call() */ public static function __callStatic(string $method, mixed $args): static|null From 41ab61fd2ba98c0255930746f86dfa4b19eec8c4 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 18 Sep 2023 15:39:33 -0400 Subject: [PATCH 21/27] Fixed two bugs and wrote test cases to illuminate them: 1) Author::order('author_id DESC')->find_by_name('Tito'); would clobber order 2) Author::limit(2)->find_by_name('Tito'); would clobber limit --- lib/Relation.php | 29 ++++++++++++++++++++--------- test/ActiveRecordFindByTest.php | 17 ++++++++++++++++- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/lib/Relation.php b/lib/Relation.php index 4ee01119..1dac5cb4 100644 --- a/lib/Relation.php +++ b/lib/Relation.php @@ -114,9 +114,10 @@ public function none(): Relation * specify an options array with conditions in it. * * ``` - * SomeModel::select('author_id')->find_by_first_name('Tito'); - * SomeModel::select('author_id')->find_by_first_name_and_last_name('Tito','the Grief'); - * SomeModel::select('author_id')->find_by_first_name_or_last_name('Tito','the Grief'); + * Person::select('id')->find_by_first_name('Tito'); + * Person::select('id')->order('last_name DESC')->find_by_first_name('Tito'); + * Person::select('id')->find_by_first_name_and_last_name('Tito','the Grief'); + * Person::select('id')->find_by_first_name_or_last_name('Tito','the Grief'); * ``` * * You can also create the model if the find call returned no results: @@ -125,7 +126,7 @@ public function none(): Relation * Person::find_or_create_by_name('Tito'); * * # would be the equivalent of - * if (!Person::select('author_id')->find_by_name('Tito')) + * if (!Person::find_by_name('Tito')) * Person::create(['Tito']); * ``` * @@ -133,17 +134,19 @@ public function none(): Relation * * ``` * Person::select('id')->find_or_create_by_name_and_id('Tito',1); - * Person::select('id')->find_or_create_by_name_and_id(['name' => 'Tito', 'id' => 1]); + * Person::find_or_create_by_name_and_id(['name' => 'Tito', 'id' => 1]); * ``` * * @param $method Name of method * @param $args Method args * - * @throws ActiveRecordException + * @throws ActiveRecordException If the method name does not start with the find_by or find_or_create_by templates * * @see find + * + * @return Model|null The first model that meets the find by criteria, or null if no row meets that criteria */ - public function __call(string $method, mixed $args): mixed + public function __call(string $method, mixed $args): Model|null { $create = false; @@ -164,7 +167,15 @@ public function __call(string $method, mixed $args): mixed $this->options['conditions'] ??= []; $this->options['conditions'][] = WhereClause::from_underscored_string($this->table()->conn, $attributes, $args, $this->alias_attribute); - $ret = $this->firstOrLast(1, true); + $oldLimit = $this->options['limit'] ?? null; + $ret = $this->limit(1)->to_a(); + + if (null === $oldLimit) { + unset($this->options['limit']); + } else { + $this->options['limit'] = $oldLimit; + } + if (0 === count($ret)) { if ($create) { return $this->className::create(SQLBuilder::create_hash_from_underscored_string( @@ -182,7 +193,7 @@ public function __call(string $method, mixed $args): mixed private function extract_dynamic_vars(string $methodName, string $dynamicPart): string { if (str_starts_with($methodName, $dynamicPart)) { - $attributes = substr($methodName, strlen($dynamicPart) +1); + $attributes = substr($methodName, strlen($dynamicPart) + 1); return $attributes; } diff --git a/test/ActiveRecordFindByTest.php b/test/ActiveRecordFindByTest.php index 84083691..d1966534 100644 --- a/test/ActiveRecordFindByTest.php +++ b/test/ActiveRecordFindByTest.php @@ -32,16 +32,31 @@ public function testFindBy() $this->assertEquals('Tito', Author::find_by_name(['Tito', 'George W. Bush'])->name); } - public function testFindBySelect() + public function testFindByChained() { $author = Author::select('author_id')->find_by_name('Tito'); $this->assertInstanceOf(Author::class, $author); $this->assertEquals(1, $author->author_id); + $author = Author::select('author_id')->order('author_id DESC')->find_by_name('Tito'); + $this->assertEquals(5, $author->author_id); + $this->expectException(ActiveRecordException::class); $author->name; } + public function testFindByDoesNotClobberOldLimit() + { + $rel = Author::where(['mixedCaseField' => 'Bill'])->limit(2); + $authors = $rel->to_a(); + $this->assertEquals(2, count($authors)); + + $author = $rel->find_by_mixedCaseField('Bill'); + $this->assertInstanceOf(Author::class, $author); + + $this->assertEquals($authors, $rel->to_a()); + } + public function testFindByNoResults() { $this->assertNull(Author::find_by_name('SHARKS WIT LASERZ')); From 94cb360bfc641a0ec5be197230f2457e7cc8b7f7 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 18 Sep 2023 16:42:49 -0400 Subject: [PATCH 22/27] Fixed a bug: Calling find_by(), first(), last(), and take() should not have side effects on $this->options --- lib/Relation.php | 54 +++++++++++++++++------------- test/ActiveRecordFindByTest.php | 5 +++ test/ActiveRecordFirstLastTest.php | 11 ++++++ test/ActiveRecordTakeTest.php | 12 +++++++ 4 files changed, 59 insertions(+), 23 deletions(-) diff --git a/lib/Relation.php b/lib/Relation.php index 1dac5cb4..8c4f61e3 100644 --- a/lib/Relation.php +++ b/lib/Relation.php @@ -164,17 +164,13 @@ public function __call(string $method, mixed $args): Model|null throw new ActiveRecordException("Call to undefined method: $method"); } - $this->options['conditions'] ??= []; - $this->options['conditions'][] = WhereClause::from_underscored_string($this->table()->conn, $attributes, $args, $this->alias_attribute); + $options = $this->options; - $oldLimit = $this->options['limit'] ?? null; - $ret = $this->limit(1)->to_a(); + $options['conditions'] ??= []; + $options['conditions'][] = WhereClause::from_underscored_string($this->table()->conn, $attributes, $args, $this->alias_attribute); - if (null === $oldLimit) { - unset($this->options['limit']); - } else { - $this->options['limit'] = $oldLimit; - } + $options['limit'] = 1; + $ret = $this->to_a_withOptions($options); if (0 === count($ret)) { if ($create) { @@ -722,9 +718,8 @@ public function find(): Model|array $options = $this->options; $options['conditions'] ??= []; $options['conditions'][] = $this->pk_conditions($args); - $options['mapped_names'] = $this->alias_attribute; - $list = iterator_to_array($this->table()->find($options)); + $list = $this->to_a_withOptions($options); if (is_array($args) && count($list) != count($args)) { throw new RecordNotFound('found ' . count($list) . ', but was looking for ' . count($args)); } @@ -745,8 +740,9 @@ public function find(): Model|array */ public function take(int $limit = null): Model|array|null { - $this->limit($limit ?? 1); - $models = $this->to_a(); + $options = $this->options; + $options = array_merge($this->options, ['limit' => $limit ?? 1]); + $models = $this->to_a_withOptions($options); return isset($limit) ? $models : $models[0] ?? null; } @@ -816,39 +812,51 @@ public function reverse_order(): array */ private function firstOrLast(int $limit = null, bool $isAscending): array { - $this->limit($limit ?? 1); + $options = array_merge($this->options, ['limit' => $limit ?? 1]); $pk = $this->table()->pk; if (!empty($pk)) { - if (array_key_exists('order', $this->options)) { + if (array_key_exists('order', $options)) { $reverseCommand = $isAscending ? 'DESC' : 'ASC'; - if (str_contains($this->options['order'], implode(" {$reverseCommand}, ", $this->table()->pk) . " {$reverseCommand}")) { - $this->options['order'] = SQLBuilder::reverse_order((string) $this->options['order']); + if (str_contains($options['order'], implode(" {$reverseCommand}, ", $this->table()->pk) . " {$reverseCommand}")) { + $options['order'] = SQLBuilder::reverse_order((string) $options['order']); } - } elseif (!array_key_exists('having', $this->options)) { + } elseif (!array_key_exists('having', $options)) { $command = $isAscending ? 'ASC' : 'DESC'; - $this->options['order'] = implode(" {$command}, ", $this->table()->pk) . " {$command}"; + $options['order'] = implode(" {$command}, ", $this->table()->pk) . " {$command}"; } } - return $this->to_a(); + return $this->to_a_withOptions($options); } /** - * Converts relation objects to array. + * Converts relation objects to array with the currently set of options. * * @return array All the rows that matches query. If no rows match, returns [] */ public function to_a(): array + { + return $this->to_a_withOptions($this->options); + } + + /** + * Converts relation objects to array with a given options. + * + * @param RelationOptions $options + * + * @return array All the rows that matches query. If no rows match, returns [] + */ + private function to_a_withOptions(array $options): array { if ($this->isNone) { return []; } - $this->options['mapped_names'] = $this->alias_attribute; + $options['mapped_names'] = $this->alias_attribute; - return iterator_to_array($this->table()->find($this->options)); + return iterator_to_array($this->table()->find($options)); } /** diff --git a/test/ActiveRecordFindByTest.php b/test/ActiveRecordFindByTest.php index d1966534..dbee3acf 100644 --- a/test/ActiveRecordFindByTest.php +++ b/test/ActiveRecordFindByTest.php @@ -55,6 +55,11 @@ public function testFindByDoesNotClobberOldLimit() $this->assertInstanceOf(Author::class, $author); $this->assertEquals($authors, $rel->to_a()); + + $author = $rel->find_by_name('Bill Clinton'); + $this->assertInstanceOf(Author::class, $author); + + $this->assertEquals($authors, $rel->to_a()); } public function testFindByNoResults() diff --git a/test/ActiveRecordFirstLastTest.php b/test/ActiveRecordFirstLastTest.php index b6e8e42f..fce6f899 100644 --- a/test/ActiveRecordFirstLastTest.php +++ b/test/ActiveRecordFirstLastTest.php @@ -105,6 +105,17 @@ public function testLastWithCountOverridesLimit() $this->assertEquals(2, count($authors)); } + public function testDoesNotClobberOrderOrLimit() + { + $rel = Author::limit(2)->order('author_id DESC'); + $this->assertEquals(1, $rel->first()->author_id); + + $authors = $rel->to_a(); + $this->assertEquals(2, count($authors)); + $this->assertEquals('Tito', $authors[0]->name); + $this->assertEquals('Uncle Bob', $authors[1]->name); + } + public function testLastNull() { $query = Author::where(['mixedCaseField' => 'Does not exist'])->last(); diff --git a/test/ActiveRecordTakeTest.php b/test/ActiveRecordTakeTest.php index eb33a9ee..517407ec 100644 --- a/test/ActiveRecordTakeTest.php +++ b/test/ActiveRecordTakeTest.php @@ -65,4 +65,16 @@ public function testWithSelectNonSelectedFieldsShouldNotHaveAttributes() $author = Author::select('name, 123 as bubba')->take(); $author->id; } + + public function testDoesNotClobberOldLimit() + { + $rel = Author::limit(2); + $authors = $rel->to_a(); + $this->assertEquals(2, count($authors)); + + $author = $rel->take(); + $this->assertInstanceOf(Author::class, $author); + + $this->assertEquals($authors, $rel->to_a()); + } } From c979ab4a4548caa4d1099f04e84b2b699ac06339 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 18 Sep 2023 16:46:16 -0400 Subject: [PATCH 23/27] Removed an extraneous line --- lib/Relation.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Relation.php b/lib/Relation.php index 8c4f61e3..90f35dae 100644 --- a/lib/Relation.php +++ b/lib/Relation.php @@ -740,7 +740,6 @@ public function find(): Model|array */ public function take(int $limit = null): Model|array|null { - $options = $this->options; $options = array_merge($this->options, ['limit' => $limit ?? 1]); $models = $this->to_a_withOptions($options); From b77a738577d6eedbcd8f4996f6ba91a7a53f1d87 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 18 Sep 2023 18:46:54 -0400 Subject: [PATCH 24/27] Added tests for Book::not('book_id in (?)', [1]) The existing code works. --- test/ActiveRecordNotTest.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/test/ActiveRecordNotTest.php b/test/ActiveRecordNotTest.php index bfa8ac47..62bda2d7 100644 --- a/test/ActiveRecordNotTest.php +++ b/test/ActiveRecordNotTest.php @@ -25,7 +25,13 @@ public static function conditions(): array ], 'WHERE `name` = ?', 'WHERE !(`name` = ?)' - ] + ], + 'in' => [[ + 'book_id in (?)', [1,2], + ], + 'WHERE book_id in (?,?)', + 'WHERE !(book_id in (?,?))', + ], ]; } @@ -46,4 +52,11 @@ public function testListOfArguments() $book = Book::where('name = ?', 'Another Book')->take(); $this->assertEquals('Another Book', $book->name); } + + public function testIn() + { + $books = Book::not('book_id in (?)', [1])->to_a(); + $this->assertEquals(1, count($books)); + $this->assertEquals('Another Book', $books[0]->name); + } } From 06b40bb6963544bd4c6c6a0194d6f50ec99d2f3d Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 18 Sep 2023 18:48:24 -0400 Subject: [PATCH 25/27] Style fix --- test/ActiveRecordNotTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ActiveRecordNotTest.php b/test/ActiveRecordNotTest.php index 62bda2d7..3bdddcf2 100644 --- a/test/ActiveRecordNotTest.php +++ b/test/ActiveRecordNotTest.php @@ -27,7 +27,7 @@ public static function conditions(): array 'WHERE !(`name` = ?)' ], 'in' => [[ - 'book_id in (?)', [1,2], + 'book_id in (?)', [1, 2], ], 'WHERE book_id in (?,?)', 'WHERE !(book_id in (?,?))', From a8601287acca372d4fc6e9cf16a112cfe08b8ba9 Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 18 Sep 2023 21:55:40 -0400 Subject: [PATCH 26/27] Fixed a bug due to the misunderstanding of what first() and last() do. This now passes: $rel = Author::limit(2)->order('author_id DESC'); $this->assertEquals(Author::last(), $rel->first()); $this->assertEquals(Author::first(), $rel->last()); --- lib/Relation.php | 8 ++++---- test/ActiveRecordFirstLastTest.php | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/Relation.php b/lib/Relation.php index 90f35dae..e47ef97d 100644 --- a/lib/Relation.php +++ b/lib/Relation.php @@ -816,10 +816,10 @@ private function firstOrLast(int $limit = null, bool $isAscending): array $pk = $this->table()->pk; if (!empty($pk)) { if (array_key_exists('order', $options)) { - $reverseCommand = $isAscending ? 'DESC' : 'ASC'; - - if (str_contains($options['order'], implode(" {$reverseCommand}, ", $this->table()->pk) . " {$reverseCommand}")) { - $options['order'] = SQLBuilder::reverse_order((string) $options['order']); + if (!$isAscending) { + if (str_contains($options['order'], implode(" DESC, ", $this->table()->pk) . " DESC")) { + $options['order'] = SQLBuilder::reverse_order((string) $options['order']); + } } } elseif (!array_key_exists('having', $options)) { $command = $isAscending ? 'ASC' : 'DESC'; diff --git a/test/ActiveRecordFirstLastTest.php b/test/ActiveRecordFirstLastTest.php index fce6f899..5a43e94b 100644 --- a/test/ActiveRecordFirstLastTest.php +++ b/test/ActiveRecordFirstLastTest.php @@ -108,7 +108,8 @@ public function testLastWithCountOverridesLimit() public function testDoesNotClobberOrderOrLimit() { $rel = Author::limit(2)->order('author_id DESC'); - $this->assertEquals(1, $rel->first()->author_id); + $this->assertEquals(Author::last(), $rel->first()); + $this->assertEquals(Author::first(), $rel->last()); $authors = $rel->to_a(); $this->assertEquals(2, count($authors)); From ab321f378f0683632ecf829a6d142ebbef151b4e Mon Sep 17 00:00:00 2001 From: Michael Hu Date: Mon, 18 Sep 2023 21:59:33 -0400 Subject: [PATCH 27/27] Small style-fix --- lib/Relation.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Relation.php b/lib/Relation.php index e47ef97d..81290fd4 100644 --- a/lib/Relation.php +++ b/lib/Relation.php @@ -817,7 +817,7 @@ private function firstOrLast(int $limit = null, bool $isAscending): array if (!empty($pk)) { if (array_key_exists('order', $options)) { if (!$isAscending) { - if (str_contains($options['order'], implode(" DESC, ", $this->table()->pk) . " DESC")) { + if (str_contains($options['order'], implode(' DESC, ', $this->table()->pk) . ' DESC')) { $options['order'] = SQLBuilder::reverse_order((string) $options['order']); } }