From bfaa289c582e6bb99fe6d1c4955855a73b33241d Mon Sep 17 00:00:00 2001 From: AlexeyDsov Date: Thu, 24 May 2012 22:18:58 +0400 Subject: [PATCH 1/3] moved StorableDAO::unite "properties compare logic" to AbstractProtoClass and (Inner|Light)MetaProperty --- main/Base/AbstractProtoClass.class.php | 17 +- main/Base/InnerMetaProperty.class.php | 11 +- main/Base/LightMetaProperty.class.php | 32 ++- main/DAOs/StorableDAO.class.php | 38 +-- .../AbstractProtoClassFillQueryTest.class.php | 227 ++++++++++++++++++ 5 files changed, 289 insertions(+), 36 deletions(-) create mode 100644 test/core/AbstractProtoClassFillQueryTest.class.php diff --git a/main/Base/AbstractProtoClass.class.php b/main/Base/AbstractProtoClass.class.php index 733f730c06..6b229e1a13 100644 --- a/main/Base/AbstractProtoClass.class.php +++ b/main/Base/AbstractProtoClass.class.php @@ -211,11 +211,24 @@ public function makeForm($prefix = null) * @return InsertOrUpdateQuery **/ public function fillQuery( - InsertOrUpdateQuery $query, Prototyped $object + InsertOrUpdateQuery $query, + Prototyped $object, + Prototyped $old = null ) { + if ($old) { + if ($object instanceof Identifiable) { + Assert::isNotNull($object->getId()); + + Assert::isTypelessEqual( + $object->getId(), $old->getId(), + 'cannot merge different objects' + ); + } + } + foreach ($this->getPropertyList() as $property) { - $property->fillQuery($query, $object); + $property->fillQuery($query, $object, $old); } return $query; diff --git a/main/Base/InnerMetaProperty.class.php b/main/Base/InnerMetaProperty.class.php index 0bac817b40..0c8a776b86 100644 --- a/main/Base/InnerMetaProperty.class.php +++ b/main/Base/InnerMetaProperty.class.php @@ -52,13 +52,20 @@ public function fillForm(Form $form, $prefix = null) public function fillQuery( InsertOrUpdateQuery $query, - Prototyped $object + Prototyped $object, + Prototyped $old = null ) { + $inner = $object->{$this->getGetter()}(); + $oldInner = $old ? $old->{$this->getGetter()}() : null; + return $this->getProto()->fillQuery( $query, - $object->{$this->getGetter()}() + $inner, + //when old and objects have one value object + // we'll update all valueObject fields: + $oldInner !== $inner ? $oldInner : null ); } diff --git a/main/Base/LightMetaProperty.class.php b/main/Base/LightMetaProperty.class.php index 78bf621ef3..7865f79c2e 100644 --- a/main/Base/LightMetaProperty.class.php +++ b/main/Base/LightMetaProperty.class.php @@ -346,7 +346,8 @@ public function fillForm(Form $form, $prefix = null) **/ public function fillQuery( InsertOrUpdateQuery $query, - Prototyped $object + Prototyped $object, + Prototyped $old = null ) { if ( @@ -370,6 +371,35 @@ public function fillQuery( } $value = $object->{$getter}(); + if ($old) { + $oldValue = $old->{$getter}(); + if ($oldValue === null && $value === $oldValue) { + return $query; + } elseif ( + $this->relationId + && $this->strategyId == FetchStrategy::LAZY + && ($value === $oldValue) + ) { + return $query; + } elseif ( + $value instanceof Identifiable + && $oldValue instanceof Identifiable + && $value->getId() === $oldValue->getId() + ) { + return $query; + } elseif ( + $value instanceof Stringable + && $oldValue instanceof Stringable + && $value->toString() == $oldValue->toString() + ) { + return $query; + } elseif ( + !$this->relationId + && ($value === $oldValue) + ) { + return $query; + } + } if ($this->type == 'binary') { $query->set($this->columnName, new DBBinary($value)); diff --git a/main/DAOs/StorableDAO.class.php b/main/DAOs/StorableDAO.class.php index 6d24b923c5..2c93cef240 100644 --- a/main/DAOs/StorableDAO.class.php +++ b/main/DAOs/StorableDAO.class.php @@ -77,40 +77,16 @@ public function unite( Identifiable $object, Identifiable $old ) { - Assert::isNotNull($object->getId()); - - Assert::isTypelessEqual( - $object->getId(), $old->getId(), - 'cannot merge different objects' - ); - - $query = OSQL::update($this->getTable()); - - foreach ($this->getProtoClass()->getPropertyList() as $property) { - $getter = $property->getGetter(); - - if ($property->getClassName() === null) { - $changed = ($old->$getter() !== $object->$getter()); - } else { - /** - * way to skip pointless update and hack for recursive - * comparsion. - **/ - $changed = - ($old->$getter() !== $object->$getter()) - || ($old->$getter() != $object->$getter()); - } - - if ($changed) - $property->fillQuery($query, $object); - } + $query = $this->getProtoClass() + ->fillQuery(OSQL::update($this->getTable()), $object, $old); if (!$query->getFieldsCount()) return $object; - $this->targetizeUpdateQuery($query, $object); - - return $this->doInject($query, $object); + return $this->doInject( + $this->targetizeUpdateQuery($query, $object), + $object + ); } /** @@ -124,4 +100,4 @@ private function targetizeUpdateQuery( return $query->where(Expression::eqId($this->getIdName(), $object)); } } -?> \ No newline at end of file +?> diff --git a/test/core/AbstractProtoClassFillQueryTest.class.php b/test/core/AbstractProtoClassFillQueryTest.class.php new file mode 100644 index 0000000000..d6348c4db9 --- /dev/null +++ b/test/core/AbstractProtoClassFillQueryTest.class.php @@ -0,0 +1,227 @@ +spawnCity(); + + $insertCity = $city->proto()->fillQuery(OSQL::insert(), $city); + $this->assertEquals( + 'INSERT INTO (id, name, capital, large) VALUES (20, Saint-Peterburg, TRUE, TRUE)', + $insertCity->toDialectString(ImaginaryDialect::me()) + ); + } + + public function testFullUpdateQueryByUser() + { + $city = $this->spawnCity(); + $user = $this->spawnUser(array('city' => $city)); + $updateUser = $user->proto()->fillQuery(OSQL::update(), $user); + $this->assertEquals( + 'UPDATE SET id = 77, nickname = NULL, password = NULL, ' + .'very_custom_field_name = 2011-12-31 00:00:00, ' + .'registered = 2011-12-30 00:00:00, strange_time = 01:23:45, ' + .'city_id = 20, first_optional_id = NULL, second_optional_id = NULL, ' + .'url = https://www.github.com, ' + .'properties = "a"=>"apple","b"=>"bananas",, ip = 127.0.0.1', + $updateUser->toDialectString(ImaginaryDialect::me()) + ); + } + + public function testFullUpdateQueryByUserWithContactExt() + { + $contactValue = $this->spawnContactValueExt(); + $user = $this->spawnUserWithContactExt(array('contactExt' => $contactValue)); + + $updateUser = $user->proto()->fillQuery(OSQL::update(), $user); + $this->assertEquals( + 'UPDATE SET id = 77, name = Aleksey, surname = Alekseev, email = foo@bar.com, ' + .'icq = 12345678, phone = 89012345678, city_id = NULL, ' + .'web = https://www.github.com/, skype = github', + $updateUser->toDialectString(ImaginaryDialect::me()) + ); + } + + public function testUpdateQueryByCityOneBoolean() + { + $cityOld = $this->spawnCity(array('capital' => false)); + $city = $this->spawnCity(array('capital' => true)); //1918 + + $updateCity = $city->proto()->fillQuery(OSQL::update(), $city, $cityOld); + $this->assertEquals( + 'UPDATE SET capital = TRUE', + $updateCity->toDialectString(ImaginaryDialect::me()) + ); + } + + public function testUpdateQueryByCityOneString() + { + $cityOld = $this->spawnCity(array('name' => 'Leningrad')); + $city = $this->spawnCity(array('name' => 'Saint-Peterburg')); + + $updateCity = $city->proto()->fillQuery(OSQL::update(), $city, $cityOld); + $this->assertEquals( + 'UPDATE SET name = Saint-Peterburg', + $updateCity->toDialectString(ImaginaryDialect::me()) + ); + } + + public function testUpdateQueryByUserTimesAndUrl() + { + $oldUser = $this->spawnUser(array( + 'strangeTime' => Time::create('12:12:12'), + 'url' => HttpUrl::create()->parse('http://code.google.com/'), + )); + $user = $this->spawnUser(array('lastLogin' => Timestamp::create('2012-01-01'))); + + $updateUser = $user->proto()->fillQuery(OSQL::update(), $user, $oldUser); + $this->assertEquals( + 'UPDATE SET very_custom_field_name = 2012-01-01 00:00:00, ' + .'strange_time = 01:23:45, url = https://www.github.com', + $updateUser->toDialectString(ImaginaryDialect::me()) + ); + } + + public function testUpdateQueryByUserCitiesAndHstore() + { + $moscow = $this->spawnCity(array('id' => 1, 'name' => 'Moscow')); + $piter = $this->spawnCity(array('id' => 2, 'name' => 'Saint-Peterburg')); + $omsk = $this->spawnCity(array('id' => 3, 'name' => 'Omsk')); + + $userParams = array( + 'city' => $moscow, + 'firstOptional' => $piter, + 'secondOptional' => null, + 'properties' => Hstore::make(array()), + ); + $oldUser = $this->spawnUser($userParams); + $userParams = array( + 'city' => $piter, + 'firstOptional' => null, + 'secondOptional' => $omsk, + 'properties' => Hstore::make(array('param' => 'value')), + ); + $user = $this->spawnUser($userParams); + + $updateUser = $user->proto()->fillQuery(OSQL::update(), $user, $oldUser); + $this->assertEquals( + 'UPDATE SET city_id = 2, first_optional_id = NULL, ' + .'second_optional_id = 3, properties = "param"=>"value",', + $updateUser->toDialectString(ImaginaryDialect::me()) + ); + } + + public function testUpdateQueryByUserWithValueObject() + { + $moscow = $this->spawnCity(); + $oldContactExt = $this->spawnContactValueExt(array('email' => 'bar@foo.com')); + $oldUser = $this->spawnUserWithContactExt(array('contactExt' => $oldContactExt)); + $contactExt = $this->spawnContactValueExt(array('city' => $moscow)); + $user = $this->spawnUserWithContactExt(array('contactExt' => $contactExt)); + + $updateUser = $user->proto()->fillQuery(OSQL::update(), $user, $oldUser); + $this->assertEquals( + 'UPDATE SET email = foo@bar.com, city_id = 20', + $updateUser->toDialectString(ImaginaryDialect::me()) + ); + } + + public function testUpdateQueryByUserWithSameValueObject() + { + //if value object same for both main objects - we'll update all fields from value object + $contactExt = $this->spawnContactValueExt(); + $oldUser = $this->spawnUserWithContactExt(array('contactExt' => $contactExt)); + $user = $this->spawnUserWithContactExt(array('contactExt' => $contactExt)); + + $updateUser = $user->proto()->fillQuery(OSQL::update(), $user, $oldUser); + $this->assertEquals( + 'UPDATE SET email = foo@bar.com, icq = 12345678, ' + .'phone = 89012345678, city_id = NULL, ' + .'web = https://www.github.com/, skype = github', + $updateUser->toDialectString(ImaginaryDialect::me()) + ); + } + + /** + * @return TestCity + */ + private function spawnCity($options = array()) + { + $options += array( + 'capital' => true, + 'large' => true, + 'name' => 'Saint-Peterburg', + 'id' => 20, + ); + + return $this->spawnObject(TestCity::create(), $options); + } + + /** + * @return TestUser + */ + private function spawnUser($options = array()) + { + $options += array( + 'id' => '77', + 'credentials' => Credentials::create(), + 'lastLogin' => Timestamp::create('2011-12-31'), + 'registered' => Timestamp::create('2011-12-30'), + 'strangeTime' => Time::create('01:23:45'), + 'city' => null, + 'firstOptional' => null, + 'secondOptional' => null, + 'url' => HttpUrl::create()->parse('https://www.github.com'), + 'properties' => Hstore::make(array('a' => 'apple', 'b' => 'bananas')), + 'ip' => IpAddress::create('127.0.0.1'), + ); + + return $this->spawnObject(TestUser::create(), $options); + + } + + /** + * @return TestContactValueExtended + */ + private function spawnContactValueExt($options = array()) + { + $options += array( + 'web' => 'https://www.github.com/', + 'skype' => 'github', + 'email' => 'foo@bar.com', + 'icq' => 12345678, + 'phone' => '89012345678', + 'city' => null, + ); + + return $this->spawnObject(TestContactValueExtended::create(), $options); + } + + /** + * @return TestUserWithContactExtended + */ + private function spawnUserWithContactExt($options = array()) + { + $options += array( + 'id' => '77', + 'name' => 'Aleksey', + 'surname' => 'Alekseev', + 'contactExt' => null, + ); + + return $this->spawnObject(TestUserWithContactExtended::create(), $options); + } + + private function spawnObject(Prototyped $object, array $options) + { + foreach ($object->proto()->getPropertyList() as $propName => $property) { + /* @var $property LightMetaProperty */ + if (isset($options[$propName])) { + $setter = $property->getSetter(); + $object->{$setter}($options[$propName]); + } + } + return $object; + } + } +?> \ No newline at end of file From 538ff663b39498db9a66de3e7fdc2999c507d32a Mon Sep 17 00:00:00 2001 From: AlexeyDsov Date: Sat, 26 May 2012 13:50:39 +0400 Subject: [PATCH 2/3] LightMetaProperty::fillQuery added compare object using serialization instead toString --- main/Base/LightMetaProperty.class.php | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/main/Base/LightMetaProperty.class.php b/main/Base/LightMetaProperty.class.php index 7865f79c2e..e7f5b06998 100644 --- a/main/Base/LightMetaProperty.class.php +++ b/main/Base/LightMetaProperty.class.php @@ -387,16 +387,7 @@ public function fillQuery( && $value->getId() === $oldValue->getId() ) { return $query; - } elseif ( - $value instanceof Stringable - && $oldValue instanceof Stringable - && $value->toString() == $oldValue->toString() - ) { - return $query; - } elseif ( - !$this->relationId - && ($value === $oldValue) - ) { + } elseif (serialize($value) == serialize($oldValue)) { return $query; } } From b6e7a56190fd0c5154d16ec8877ca738c9be2d39 Mon Sep 17 00:00:00 2001 From: AlexeyDsov Date: Wed, 27 Jun 2012 00:47:14 +0400 Subject: [PATCH 3/3] update doc/ChangeLog --- doc/ChangeLog | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/doc/ChangeLog b/doc/ChangeLog index ef66516391..622da55aed 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,3 +1,14 @@ +2012-06-27 Alexey S. Denisov + + * main/Base/AbstractProtoClass.class.php + main/Base/InnerMetaProperty.class.php + main/Base/LightMetaProperty.class.php + main/DAOs/StorableDAO.class.php + test/core/AbstractProtoClassFillQueryTest.class.php: + + moved and updated StorableDAO::unite "properties compare logic" to AbstractProtoClass + and (Inner|Light)MetaProperty and covered with tests + 2012-06-04 Evgeny M. Stepanov * main/Base/Hstore.class.php: fix @@ -12,7 +23,7 @@ core/Cache/PeclMemcached.class.php core/Cache/{Memcached.class.php → SocketMemcached.class.php} -Code clean & add SocketMemcached instead of Memcached + Code clean & add SocketMemcached instead of Memcached 2012-05-23 Georgiy T. Kutsurua