From 6f3ad8f373c0033496c0e3bfb11425c2989fa16d Mon Sep 17 00:00:00 2001 From: "Marc J. Schmidt" Date: Thu, 20 Nov 2014 10:59:40 +0100 Subject: [PATCH] Fixed AbstractCommand to work with newest Propel2 version. Added option `index_property` back to ModelType. Basically copied newest changes from propelv1 symfony bridge to ModelChoiceList to have more fixes included. Fixed also some typehints. --- Command/AbstractCommand.php | 25 +- Command/FixturesLoadCommand.php | 11 +- Form/ChoiceList/ModelChoiceList.php | 359 +++++++++++------- .../CollectionToArrayTransformer.php | 2 +- Form/Type/ModelType.php | 4 +- .../ParamConverter/PropelParamConverter.php | 6 +- 6 files changed, 247 insertions(+), 160 deletions(-) diff --git a/Command/AbstractCommand.php b/Command/AbstractCommand.php index f1803f5..0b90282 100644 --- a/Command/AbstractCommand.php +++ b/Command/AbstractCommand.php @@ -31,7 +31,7 @@ abstract class AbstractCommand extends ContainerAwareCommand protected $cacheDir = null; /** - * @var Symfony\Component\HttpKernel\Bundle\BundleInterface + * @var \Symfony\Component\HttpKernel\Bundle\BundleInterface */ protected $bundle = null; @@ -180,10 +180,19 @@ abstract class AbstractCommand extends ContainerAwareCommand array_unshift($parameters, $this->getName()); // merge the default parameters - $parameters = array_merge(array( - '--input-dir' => $this->cacheDir, - '--verbose' => $input->getOption('verbose'), - ), $parameters); + $extraParameters = [ + '--verbose' => $input->getOption('verbose') + ]; + + if ($command->getDefinition()->hasOption('schema-dir')) { + $extraParameters['--schema-dir'] = $this->cacheDir; + } + + if ($command->getDefinition()->hasOption('config-dir')) { + $extraParameters['--config-dir'] = $this->cacheDir; + } + + $parameters = array_merge($extraParameters, $parameters); if ($input->hasOption('platform')) { $parameters['--platform'] = $input->getOption('platform') ?: $this->getPlatform(); @@ -343,13 +352,11 @@ abstract class AbstractCommand extends ContainerAwareCommand /** * Reads the platform class from the configuration * - * @return The platform class name. + * @return string The platform class name. */ protected function getPlatform() { $config = $this->getContainer()->getParameter('propel.configuration'); - $className = $config['generator']['platformClass']; - - return ($pos = strrpos($className, '\\')) === false ? $className : substr($className, $pos + 1); + return $config['generator']['platformClass']; } } diff --git a/Command/FixturesLoadCommand.php b/Command/FixturesLoadCommand.php index 46ea63d..4830602 100644 --- a/Command/FixturesLoadCommand.php +++ b/Command/FixturesLoadCommand.php @@ -178,16 +178,7 @@ EOT return; } - try { - $nb = $loader->load($datas, $connectionName); - } catch (\Exception $e) { - $this->writeSection($output, array( - '[Propel] Exception', - '', - $e->getMessage()), 'fg=white;bg=red'); - - return false; - } + $nb = $loader->load($datas, $connectionName); $output->writeln(sprintf('%s %s fixtures file%s loaded.', $nb, strtoupper($type), $nb > 1 ? 's' : '')); diff --git a/Form/ChoiceList/ModelChoiceList.php b/Form/ChoiceList/ModelChoiceList.php index 5f84b63..9af9627 100644 --- a/Form/ChoiceList/ModelChoiceList.php +++ b/Form/ChoiceList/ModelChoiceList.php @@ -1,5 +1,4 @@ * @author Toni Uebernickel @@ -53,63 +52,87 @@ class ModelChoiceList extends ObjectChoiceList /** * Whether the model objects have already been loaded. * - * @var Boolean + * @var bool */ protected $loaded = false; /** - * Whether to use the identifier for index generation + * Whether to use the identifier for index generation. * - * @var Boolean + * @var bool */ private $identifierAsIndex = false; /** * Constructor. * - * @see Symfony\Bridge\Propel1\Form\Type\ModelType How to use the preferred choices. + * @see \Symfony\Bridge\Propel1\Form\Type\ModelType How to use the preferred choices. * * @param string $class The FQCN of the model class to be loaded. - * @param string $labelPath A property path pointing to the property used for the choice labels. + * @param string $labelPath A property path pointing to the property used for the choice + * labels. * @param array $choices An optional array to use, rather than fetching the models. * @param ModelCriteria $queryObject The query to use retrieving model data from database. - * @param string $groupPath A property path pointing to the property used to group the choices. + * @param string $groupPath A property path pointing to the property used to group the + * choices. * @param array|ModelCriteria $preferred The preferred items of this choice. * Either an array if $choices is given, * or a ModelCriteria to be merged with the $queryObject. * @param PropertyAccessorInterface $propertyAccessor The reflection graph for reading property paths. + * @param string $useAsIdentifier a custom unique column (eg slug) to use instead of primary + * key. + * + * @throws MissingOptionsException In case the class parameter is empty. + * @throws InvalidOptionsException In case the query class is not found. */ - public function __construct($class, $labelPath = null, $choices = null, $queryObject = null, $groupPath = null, $preferred = array(), PropertyAccessorInterface $propertyAccessor = null) - { - $this->class = $class; - - $queryClass = $this->class.'Query'; - $query = new $queryClass(); - - $this->identifier = $query->getTableMap()->getPrimaryKeys(); - $this->query = $queryObject ?: $query; - $this->loaded = is_array($choices) || $choices instanceof \Traversable; - + public function __construct( + $class, + $labelPath = null, + $choices = null, + $queryObject = null, + $groupPath = null, + $preferred = array(), + PropertyAccessorInterface $propertyAccessor = null, + $useAsIdentifier = null + ) { + $this->class = $class; + $queryClass = $this->class . 'Query'; + if (!class_exists($queryClass)) { + if (empty($this->class)) { + throw new MissingOptionsException('The "class" parameter is empty, you should provide the model class'); + } + throw new InvalidOptionsException( + sprintf( + 'The query class "%s" is not found, you should provide the FQCN of the model class', + $queryClass + ) + ); + } + $query = new $queryClass(); + $this->query = $queryObject ?: $query; + if ($useAsIdentifier) { + $this->identifier = array($this->query->getTableMap()->getColumn($useAsIdentifier)); + } else { + $this->identifier = $this->query->getTableMap()->getPrimaryKeys(); + } + $this->loaded = is_array($choices) || $choices instanceof \Traversable; if ($preferred instanceof ModelCriteria) { $this->preferredQuery = $preferred->mergeWith($this->query); } - if (!$this->loaded) { // Make sure the constraints of the parent constructor are // fulfilled $choices = array(); $preferred = array(); } - - if (1 === count($this->identifier) && $this->isInteger(current($this->identifier))) { + if (1 === count($this->identifier) && $this->isScalar(current($this->identifier))) { $this->identifierAsIndex = true; } - parent::__construct($choices, $labelPath, $preferred, $groupPath, null, $propertyAccessor); } /** - * Returns the class name + * Returns the class name of the model. * * @return string */ @@ -119,143 +142,171 @@ class ModelChoiceList extends ObjectChoiceList } /** - * Returns the list of model objects - * - * @return array - * - * @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface + * {@inheritdoc} */ public function getChoices() { - if (!$this->loaded) { - $this->load(); - } + $this->load(); return parent::getChoices(); } /** - * Returns the values for the model objects - * - * @return array - * - * @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface + * {@inheritdoc} */ public function getValues() { - if (!$this->loaded) { - $this->load(); - } + $this->load(); return parent::getValues(); } /** - * Returns the choice views of the preferred choices as nested array with - * the choice groups as top-level keys. - * - * @return array - * - * @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface + * {@inheritdoc} */ public function getPreferredViews() { - if (!$this->loaded) { - $this->load(); - } + $this->load(); return parent::getPreferredViews(); } /** - * Returns the choice views of the choices that are not preferred as nested - * array with the choice groups as top-level keys. - * - * @return array - * - * @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface + * {@inheritdoc} */ public function getRemainingViews() { - if (!$this->loaded) { - $this->load(); - } + $this->load(); return parent::getRemainingViews(); } /** - * Returns the model objects corresponding to the given values. - * - * @param array $values - * - * @return array - * - * @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface + * {@inheritdoc} */ public function getChoicesForValues(array $values) { + if (empty($values)) { + return array(); + } + + /** + * This performance optimization reflects a common scenario: + * * A simple select of a model entry. + * * The choice option "expanded" is set to false. + * * The current request is the submission of the selected value. + * + * @see \Symfony\Component\Form\Extension\Core\DataTransformer\ChoicesToValuesTransformer::reverseTransform + * @see \Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToValueTransformer::reverseTransform + */ if (!$this->loaded) { if (1 === count($this->identifier)) { - $filterBy = 'filterBy'.current($this->identifier)->getPhpName(); + $filterBy = 'filterBy' . current($this->identifier)->getPhpName(); + // The initial query is cloned, so all additional filters are applied correctly. + $query = clone $this->query; + $result = (array)$query + ->$filterBy( + $values + ) + ->find(); + // Preserve the keys as provided by the values. + $models = array(); + foreach ($values as $index => $value) { + foreach ($result as $eachModel) { + if ($value === $this->createValue($eachModel)) { + // Make sure to convert to the right format + $models[$index] = $this->fixChoice($eachModel); + // If all values have been assigned, skip further loops. + unset($values[$index]); + if (0 === count($values)) { + break 2; + } + } + } + } - return $this->query->create() - ->$filterBy($values) - ->find() - ->getData(); + return $models; } - - $this->load(); } + $this->load(); return parent::getChoicesForValues($values); } /** - * Returns the values corresponding to the given model objects. - * - * @param array $models - * - * @return array - * - * @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface + * {@inheritdoc} */ public function getValuesForChoices(array $models) { + if (empty($models)) { + return array(); + } if (!$this->loaded) { - // Optimize performance for single-field identifiers. We already - // know that the IDs are used as values - - // Attention: This optimization does not check choices for existence + /** + * This performance optimization assumes the validation of the respective values will be done by other means. + * + * It correlates with the performance optimization in {@link ModelChoiceList::getChoicesForValues()} + * as it won't load the actual entries from the database. + * + * @see \Symfony\Component\Form\Extension\Core\DataTransformer\ChoicesToValuesTransformer::transform + * @see \Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToValueTransformer::transform + */ if (1 === count($this->identifier)) { $values = array(); - foreach ($models as $model) { + foreach ($models as $index => $model) { if ($model instanceof $this->class) { // Make sure to convert to the right format - $values[] = $this->fixValue(current($this->getIdentifierValues($model))); + $values[$index] = $this->fixValue(current($this->getIdentifierValues($model))); } } return $values; } - - $this->load(); } - return parent::getValuesForChoices($models); + $this->load(); + $values = array(); + $availableValues = $this->getValues(); + + /* + * Overwriting default implementation. + * + * The two objects may represent the same entry in the database, + * but if they originated from different queries, there are not the same object within the code. + * + * This happens when using m:n relations with either sides model as data_class of the form. + * The choicelist will retrieve the list of available related models with a different query, resulting in different objects. + */ + $choices = $this->fixChoices($models); + foreach ($choices as $i => $givenChoice) { + if (null === $givenChoice) { + continue; + } + foreach ($this->getChoices() as $j => $choice) { + if ($this->isEqual($choice, $givenChoice)) { + $values[$i] = $availableValues[$j]; + // If all choices have been assigned, skip further loops. + unset($choices[$i]); + if (0 === count($choices)) { + break 2; + } + } + } + } + + return $values; } /** - * Returns the indices corresponding to the given models. + * {@inheritdoc} * - * @param array $models - * - * @return array - * - * @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface + * @deprecated Deprecated since version 2.4, to be removed in 3.0. */ public function getIndicesForChoices(array $models) { + if (empty($models)) { + return array(); + } $indices = array(); if (!$this->loaded) { @@ -287,12 +338,15 @@ class ModelChoiceList extends ObjectChoiceList * The choicelist will retrieve the list of available related models with a different query, resulting in different objects. */ $choices = $this->fixChoices($models); - foreach ($this->getChoices() as $i => $choice) { - foreach ($choices as $j => $givenChoice) { - if (null !== $givenChoice && $this->getIdentifierValues($choice) === $this->getIdentifierValues($givenChoice)) { - $indices[] = $i; - unset($choices[$j]); - + foreach ($choices as $i => $givenChoice) { + if (null === $givenChoice) { + continue; + } + foreach ($this->getChoices() as $j => $choice) { + if ($this->isEqual($choice, $givenChoice)) { + $indices[$i] = $j; + // If all choices have been assigned, skip further loops. + unset($choices[$i]); if (0 === count($choices)) { break 2; } @@ -304,27 +358,16 @@ class ModelChoiceList extends ObjectChoiceList } /** - * Returns the models corresponding to the given values. + * {@inheritdoc} * - * @param array $values - * - * @return array - * - * @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface + * @deprecated Deprecated since version 2.4, to be removed in 3.0. */ public function getIndicesForValues(array $values) { - if (!$this->loaded) { - // Optimize performance for single-field identifiers. We already - // know that the IDs are used as indices and values - - // Attention: This optimization does not check values for existence - if ($this->identifierAsIndex) { - return $this->fixIndices($values); - } - - $this->load(); + if (empty($values)) { + return array(); } + $this->load(); return parent::getIndicesForValues($values); } @@ -338,7 +381,7 @@ class ModelChoiceList extends ObjectChoiceList * * @param mixed $model The choice to create an index for * - * @return integer|string A unique index containing only ASCII letters, + * @return int|string A unique index containing only ASCII letters, * digits and underscores. */ protected function createIndex($model) @@ -359,41 +402,49 @@ class ModelChoiceList extends ObjectChoiceList * * @param mixed $model The choice to create a value for * - * @return integer|string A unique value without character limitations. + * @return int|string A unique value without character limitations. */ protected function createValue($model) { - if (1 === count($this->identifier)) { - return (string) current($this->getIdentifierValues($model)); + if ($this->identifierAsIndex) { + return (string)current($this->getIdentifierValues($model)); } return parent::createValue($model); } /** - * Loads the list with model objects. + * Loads the complete choice list entries, once. + * + * If data has been loaded the choice list is initialized with the retrieved data. */ private function load() { - $models = $this->query->find()->getData(); + if ($this->loaded) { + return; + } + $models = (array)$this->query->find(); $preferred = array(); if ($this->preferredQuery instanceof ModelCriteria) { - $preferred = $this->preferredQuery->find()->getData(); + $preferred = (array)$this->preferredQuery->find(); } try { // The second parameter $labels is ignored by ObjectChoiceList parent::initialize($models, array(), $preferred); - } catch (StringCastException $e) { - throw new StringCastException(str_replace('argument $labelPath', 'option "property"', $e->getMessage()), null, $e); + $this->loaded = true; + } catch(StringCastException $e) { + throw new StringCastException( + str_replace('argument $labelPath', 'option "property"', $e->getMessage()), + null, + $e + ); } - - $this->loaded = true; } /** - * Returns the values of the identifier fields of an model + * Returns the values of the identifier fields of a model. * * Propel must know about this model, that is, the model must already * be persisted or added to the idmodel map before. Otherwise an @@ -405,10 +456,17 @@ class ModelChoiceList extends ObjectChoiceList */ private function getIdentifierValues($model) { - if (null === $model) { + if (!$model instanceof $this->class) { return array(); } + if (1 === count($this->identifier) && current($this->identifier) instanceof ColumnMap) { + $phpName = current($this->identifier)->getPhpName(); + if (method_exists($model, 'get' . $phpName)) { + return array($model->{'get' . $phpName}()); + } + } + if ($model instanceof ActiveRecordInterface) { return array($model->getPrimaryKey()); } @@ -417,14 +475,41 @@ class ModelChoiceList extends ObjectChoiceList } /** - * Whether this column in an integer + * Whether this column contains scalar values (to be used as indices). * * @param ColumnMap $column * - * @return Boolean + * @return bool */ - private function isInteger(ColumnMap $column) + private function isScalar(ColumnMap $column) { - return $column->getPdoType() === \PDO::PARAM_INT; + return in_array( + $column->getPdoType(), + array( + \PDO::PARAM_BOOL, + \PDO::PARAM_INT, + \PDO::PARAM_STR, + ) + ); } -} + + /** + * Check the given choices for equality. + * + * @param mixed $choice + * @param mixed $givenChoice + * + * @return bool + */ + private function isEqual($choice, $givenChoice) + { + if ($choice === $givenChoice) { + return true; + } + if ($this->getIdentifierValues($choice) === $this->getIdentifierValues($givenChoice)) { + return true; + } + + return false; + } +} \ No newline at end of file diff --git a/Form/DataTransformer/CollectionToArrayTransformer.php b/Form/DataTransformer/CollectionToArrayTransformer.php index dfe0e63..57351f9 100644 --- a/Form/DataTransformer/CollectionToArrayTransformer.php +++ b/Form/DataTransformer/CollectionToArrayTransformer.php @@ -30,7 +30,7 @@ class CollectionToArrayTransformer implements DataTransformerInterface } if (!$collection instanceof ObjectCollection) { - throw new TransformationFailedException('Expected a \ObjectCollection.'); + throw new TransformationFailedException('Expected a \Propel\Runtime\Collection\ObjectCollection.'); } return $collection->getData(); diff --git a/Form/Type/ModelType.php b/Form/Type/ModelType.php index 1fcdfac..2b1f2e0 100644 --- a/Form/Type/ModelType.php +++ b/Form/Type/ModelType.php @@ -79,7 +79,8 @@ class ModelType extends AbstractType $options['query'], $options['group_by'], $options['preferred_choices'], - $propertyAccessor + $propertyAccessor, + $options['index_property'] ); }; @@ -94,6 +95,7 @@ class ModelType extends AbstractType 'choice_list' => $choiceList, 'group_by' => null, 'by_reference' => false, + 'index_property' => null, )); } diff --git a/Request/ParamConverter/PropelParamConverter.php b/Request/ParamConverter/PropelParamConverter.php index 07595cc..1b4eb42 100644 --- a/Request/ParamConverter/PropelParamConverter.php +++ b/Request/ParamConverter/PropelParamConverter.php @@ -4,6 +4,7 @@ namespace Propel\PropelBundle\Request\ParamConverter; use Propel\PropelBundle\Util\PropelInflector; use Propel\Runtime\ActiveQuery\Criteria; +use Propel\Runtime\ActiveQuery\ModelCriteria; use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter; use Sensio\Bundle\FrameworkExtraBundle\Request\ParamConverter\ParamConverterInterface; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -160,6 +161,7 @@ class PropelParamConverter implements ParamConverterInterface if (!class_exists($classname)) { return false; } + // Propel Class? $class = new \ReflectionClass($configuration->getClass()); if ($class->implementsInterface('\Propel\Runtime\ActiveRecord\ActiveRecordInterface')) { @@ -209,7 +211,7 @@ class PropelParamConverter implements ParamConverterInterface try { $query->{'filterBy' . PropelInflector::camelize($column)}($value); $hasCriteria = true; - } catch (\PropelException $e) { } + } catch (\Exception $e) { } } } @@ -229,7 +231,7 @@ class PropelParamConverter implements ParamConverterInterface * * @param string $classQuery * - * @return \ModelCriteria + * @return ModelCriteria * * @throws \Exception */