Extbase:
authorSebastian Kurfürst <sebastian@typo3.org>
Sat, 11 Jul 2009 09:59:15 +0000 (09:59 +0000)
committerSebastian Kurfürst <sebastian@typo3.org>
Sat, 11 Jul 2009 09:59:15 +0000 (09:59 +0000)
* Added some review comments for Extbase persistence rewrite

typo3/sysext/extbase/Classes/DomainObject/AbstractEntity.php
typo3/sysext/extbase/Classes/DomainObject/DomainObjectInterface.php
typo3/sysext/extbase/Classes/MVC/Controller/Argument.php
typo3/sysext/extbase/Classes/Persistence/QOM/QueryObjectModelFactory.php
typo3/sysext/extbase/Classes/Persistence/Query.php
typo3/sysext/extbase/Classes/Persistence/Repository.php

index 4716772..addab49 100644 (file)
@@ -39,15 +39,16 @@ abstract class Tx_Extbase_DomainObject_AbstractEntity extends Tx_Extbase_DomainO
 
        /**
         * Register an object's clean state, e.g. after it has been reconstituted
-        * from the database
+        * from the database.
         *
-        * @param string $propertyName The name of the property to be memorized. If omittet all persistable properties are memorized.
+        * @param string $propertyName The name of the property to be memorized. If omitted all persistable properties are memorized.
         * @return void
         * @internal
         */
        public function _memorizeCleanState($propertyName = NULL) {
                // TODO Remove dependency to $dataMapper
                if ($propertyName !== NULL) {
+                       // SK: The if part is missing here! Can it be removed?
                } else {
                        $dataMapper = t3lib_div::makeInstance('Tx_Extbase_Persistence_Mapper_DataMapper'); // singleton
                        $this->_cleanProperties = array();
@@ -62,7 +63,7 @@ abstract class Tx_Extbase_DomainObject_AbstractEntity extends Tx_Extbase_DomainO
 
        /**
         * Register an properties's clean state, e.g. after it has been reconstituted
-        * from the database
+        * from the database.
         *
         * @param string $propertyName The name of the property to be memorized. If omittet all persistable properties are memorized.
         * @return void
@@ -74,6 +75,8 @@ abstract class Tx_Extbase_DomainObject_AbstractEntity extends Tx_Extbase_DomainO
                        $this->_cleanProperties = array();
                }
                if (is_object($propertyValue)) {
+                       // SK: Is "clone" semantically correct here? Discussion needed.
+                       // If you see "getDirtyProperties", there you compare with ===, so cloned objects will return a different value I think (but needs to be verified)
                        $this->_cleanProperties[$propertyName] = clone($propertyValue);
                } else {
                        $this->_cleanProperties[$propertyName] = $propertyValue;
index fd624d7..37fbd3e 100644 (file)
 ***************************************************************/
 
 /**
- * A Domain Object Interface
+ * A Domain Object Interface. All domain objects which should be persisted need to implement the below interface.
+ * Usually you will need to subclass Tx_Extbase_DomainObject_AbstractEntity and Tx_Extbase_DomainObject_AbstractValueObject
+ * instead.
+ * 
+ * @see Tx_Extbase_DomainObject_AbstractEntity
+ * @see Tx_Extbase_DomainObject_AbstractValueObject
  *
  * @package Extbase
  * @subpackage DomainObject
index 6b2acc3..5d70dec 100644 (file)
@@ -110,6 +110,7 @@ class Tx_Extbase_MVC_Controller_Argument {
                if (!is_string($name) || strlen($name) < 1) throw new InvalidArgumentException('$name must be of type string, ' . gettype($name) . ' given.', 1187951688);
                $this->name = $name;
                if (is_array($dataType)) {
+                       // SK setNewValidatorChain does not exist! I think this needs to be changed to setNewValidatorConjunction.
                        $this->setNewValidatorChain($dataType);
                } else {
                        $this->setDataType($dataType);
index ce7de12..d738bd9 100644 (file)
@@ -34,7 +34,7 @@
  * @scope prototype
  */
 class Tx_Extbase_Persistence_QOM_QueryObjectModelFactory implements Tx_Extbase_Persistence_QOM_QueryObjectModelFactoryInterface {
-
+// SK: Needs to be cleaned up (methods might need to be removed, and comments fixed)
        /**
         * @var Tx_Extbase_Persistence_Storage_BackendInterface
         */
index d2b3788..e1d55bf 100644 (file)
@@ -26,7 +26,7 @@
 ***************************************************************/
 
 /**
- * The Query classs used to run queries against the database
+ * The Query class used to run queries against the database
  *
  * @package Extbase
  * @subpackage Persistence
@@ -34,7 +34,7 @@
  * @scope prototype
  */
 class Tx_Extbase_Persistence_Query implements Tx_Extbase_Persistence_QueryInterface {
-
+// SK: Is "limit" and "order" and "offset" evaluated?
        /**
         * @var string
         */
@@ -158,7 +158,7 @@ class Tx_Extbase_Persistence_Query implements Tx_Extbase_Persistence_QueryInterf
        }
 
        /**
-        * Executes the query against TYPO3CR and returns the result
+        * Executes the query against the database and returns the result
         *
         * @return Tx_Extbase_Persistence_QueryResultInterface The query result
         */
@@ -328,7 +328,7 @@ class Tx_Extbase_Persistence_Query implements Tx_Extbase_Persistence_QueryInterf
                        } else {
                                $comparison = $this->QOMFactory->comparison(
                                        $this->QOMFactory->lowerCase(
-                                       $this->QOMFactory->propertyValue($propertyName, $sourceSelectorName)
+                                               $this->QOMFactory->propertyValue($propertyName, $sourceSelectorName)
                                        ),
                                        Tx_Extbase_Persistence_QOM_QueryObjectModelConstantsInterface::JCR_OPERATOR_EQUAL_TO,
                                        $this->QOMFactory->bindVariable($propertyName)
index 6c380b2..837c81a 100644 (file)
@@ -28,6 +28,7 @@
 // TODO Remove if autoloader is active
 require_once(PATH_t3lib . 'interfaces/interface.t3lib_singleton.php');
 require_once(PATH_tslib . 'class.tslib_content.php');
+// SK: Remove these require statements as we have the autoloader
 
 /**
  * The base repository - will usually be extended by a more concrete repository.
@@ -49,13 +50,6 @@ class Tx_Extbase_Persistence_Repository implements Tx_Extbase_Persistence_Reposi
        protected $persistenceManager;
 
        /**
-        * Contains the persistence session of the current extension
-        *
-        * @var Tx_Extbase_Persistence_Session
-        */
-       protected $session;
-
-       /**
         * Constructs a new Repository
         *
         */
@@ -71,6 +65,7 @@ class Tx_Extbase_Persistence_Repository implements Tx_Extbase_Persistence_Reposi
         * @return void
         */
        public function add($object) {
+               // SK: see the following comment, I think this needs to be added again
                // if (!($object instanceof $this->aggregateRootClassName)) throw new Tx_Extbase_Persistence_Exception_InvalidClass('The class "' . get_class($object) . '" is not supported by the repository.');
                $this->persistenceManager->getSession()->registerAddedObject($object);
        }
@@ -82,6 +77,7 @@ class Tx_Extbase_Persistence_Repository implements Tx_Extbase_Persistence_Reposi
         * @return void
         */
        public function remove($object) {
+               // SK: see the following comment, I think this needs to be added again
                // if (!($object instanceof $this->aggregateRootClassName)) throw new Tx_Extbase_Persistence_Exception_InvalidClass('The class "' . get_class($object) . '" is not supported by the repository.');
                $this->persistenceManager->getSession()->registerRemovedObject($object);
        }
@@ -197,4 +193,4 @@ class Tx_Extbase_Persistence_Repository implements Tx_Extbase_Persistence_Reposi
        }
 
 }
-?>
+?>
\ No newline at end of file