From 855c4e61cb11e75182b7a5ab94375c3e8c08770f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steffen=20Ro=C3=9Fkamp?= Date: Wed, 10 Jan 2018 10:55:41 +0100 Subject: [PATCH 1/3] Allow object nullification in RowAddedEvent --- Doctrine/Importer.php | 18 +++++++++++++----- Event/RowAddedEvent.php | 16 ++++++++++++---- Import/Importer.php | 18 +++++++++++++----- 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/Doctrine/Importer.php b/Doctrine/Importer.php index 6f52014..bf19d5b 100644 --- a/Doctrine/Importer.php +++ b/Doctrine/Importer.php @@ -12,6 +12,7 @@ use Avro\CsvBundle\Util\Reader; use Doctrine\Common\Persistence\ObjectManager; use Doctrine\ORM\Mapping\ClassMetadataInfo; +use Doctrine\ORM\Mapping\MappingException; use Symfony\Component\EventDispatcher\EventDispatcherInterface; /** @@ -37,6 +38,10 @@ class Importer * @var Reader */ protected $reader; + /** + * @var EventDispatcherInterface + */ + protected $dispatcher; /** * @var int */ @@ -81,8 +86,6 @@ public function __construct(Reader $reader, EventDispatcherInterface $dispatcher * @param string $class The class name of the entity * @param string $delimiter The csv's delimiter * @param string $headerFormat The header case format - * - * @return bool true if successful */ public function init($file, $class, $delimiter = ',', $headerFormat = 'title') { @@ -121,6 +124,8 @@ public function getRow() * @param array $fields The fields to persist * * @return true if successful + * + * @throws MappingException */ public function import($fields) { @@ -180,6 +185,8 @@ private function convertToFormFieldName($input) * @param array $row An array of data * @param array $fields An array of the fields to import * @param bool $andFlush Flush the ObjectManager + * + * @throws MappingException */ private function addRow($row, $fields, $andFlush = true) { @@ -239,9 +246,10 @@ private function addRow($row, $fields, $andFlush = true) } } - $this->dispatcher->dispatch('avro_csv.row_added', new RowAddedEvent($entity, $row, $fields)); - - // Allow the RowAddedEvent Listener to nullify invalid objects + // Allow RowAddedEvent listeners to nullify objects (i.e. when invalid) + $event = new RowAddedEvent($entity, $row, $fields); + $this->dispatcher->dispatch('avro_csv.row_added', $event); + $entity = $event->getObject(); if (null !== $entity) { $this->objectManager->persist($entity); } diff --git a/Event/RowAddedEvent.php b/Event/RowAddedEvent.php index 918e488..67fe8e8 100644 --- a/Event/RowAddedEvent.php +++ b/Event/RowAddedEvent.php @@ -21,9 +21,9 @@ class RowAddedEvent extends Event protected $fields; /** - * @param \stdClass $object The new object being persisted - * @param array $row The row being imported - * @param array $fields The mapped fields + * @param \stdClass|null $object The new object being persisted + * @param array $row The row being imported + * @param array $fields The mapped fields */ public function __construct($object, array $row, array $fields) { @@ -32,10 +32,18 @@ public function __construct($object, array $row, array $fields) $this->fields = $fields; } + /** + * @param \stdClass|null $object + */ + public function setObject($object) + { + $this->object = $object; + } + /** * Get the doctrine object. * - * @return \stdClass + * @return \stdClass|null */ public function getObject() { diff --git a/Import/Importer.php b/Import/Importer.php index 533fd85..bc70d04 100644 --- a/Import/Importer.php +++ b/Import/Importer.php @@ -12,6 +12,7 @@ use Avro\CsvBundle\Util\Reader; use Doctrine\Common\Persistence\ObjectManager; use Doctrine\ORM\Mapping\ClassMetadataInfo; +use Doctrine\ORM\Mapping\MappingException; use Symfony\Component\EventDispatcher\EventDispatcherInterface; /** @@ -37,6 +38,10 @@ class Importer * @var Reader */ protected $reader; + /** + * @var EventDispatcherInterface + */ + protected $dispatcher; /** * @var int */ @@ -81,8 +86,6 @@ public function __construct(Reader $reader, EventDispatcherInterface $dispatcher * @param string $class The class name of the entity * @param string $delimiter The csv's delimiter * @param string $headerFormat The header case format - * - * @return bool true if successful */ public function init($file, $class, $delimiter = ',', $headerFormat = 'title') { @@ -102,6 +105,8 @@ public function init($file, $class, $delimiter = ',', $headerFormat = 'title') * @param array $fields The fields to persist * * @return true if successful + * + * @throws MappingException */ public function import($fields) { @@ -161,6 +166,8 @@ private function convertToFormFieldName($input) * @param array $row An array of data * @param array $fields An array of the fields to import * @param bool $andFlush Flush the ObjectManager + * + * @throws MappingException */ private function addRow($row, $fields, $andFlush = true) { @@ -220,9 +227,10 @@ private function addRow($row, $fields, $andFlush = true) } } - $this->dispatcher->dispatch('avro_csv.row_added', new RowAddedEvent($entity, $row, $fields)); - - // Allow the RowAddedEvent Listener to nullify invalid objects + // Allow RowAddedEvent listeners to nullify objects (i.e. when invalid) + $event = new RowAddedEvent($entity, $row, $fields); + $this->dispatcher->dispatch('avro_csv.row_added', $event); + $entity = $event->getObject(); if (null !== $entity) { $this->objectManager->persist($entity); } From e5687b408a21220b03e8c48501480cba9304f8aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steffen=20Ro=C3=9Fkamp?= Date: Wed, 10 Jan 2018 12:23:22 +0100 Subject: [PATCH 2/3] Add import error handling, add events class --- .php_cs | 9 +- AvroCsvEvents.php | 49 +++++ Controller/ExportController.php | 6 +- Controller/ImportController.php | 19 +- Doctrine/Importer.php | 240 +-------------------- Event/ExportEvent.php | 2 +- Event/RowAddedEvent.php | 2 +- Event/RowErrorEvent.php | 51 +++++ Export/Doctrine/ORM/Exporter.php | 4 +- Export/Exporter.php | 4 +- Export/ExporterInterface.php | 26 ++- Form/Type/ImportFormType.php | 30 +-- Import/Importer.php | 69 +++--- Tests/Doctrine/ImporterTest.php | 13 +- Tests/Export/Doctrine/ORM/ExporterTest.php | 10 +- Tests/Import/ImporterTest.php | 4 +- Tests/TestEntity.php | 4 +- Tests/Util/FieldRetrieverTest.php | 12 +- Tests/Util/ReaderTest.php | 30 +-- Util/FieldRetriever.php | 2 +- Util/Reader.php | 6 +- 21 files changed, 236 insertions(+), 356 deletions(-) create mode 100644 AvroCsvEvents.php create mode 100644 Event/RowErrorEvent.php diff --git a/.php_cs b/.php_cs index 60138b1..ce5db15 100644 --- a/.php_cs +++ b/.php_cs @@ -1,11 +1,4 @@ setRules( - [ - '@Symfony' => true, - 'ordered_imports' => true, - ] - ) - ->setUsingCache(true) + ->setRules(['@Symfony' => true, 'array_syntax' => ['syntax' => 'short'], 'ordered_imports' => true]) ->setCacheFile(__DIR__.'/.php_cs.cache'); diff --git a/AvroCsvEvents.php b/AvroCsvEvents.php new file mode 100644 index 0000000..5c0d472 --- /dev/null +++ b/AvroCsvEvents.php @@ -0,0 +1,49 @@ + + */ +final class AvroCsvEvents +{ + /** + * The EXPORT event occurs after the exporter has been initialized. + * + * This event allows you to alter the exporter or its query builder. + * + * @Event("Avro\CsvBundle\Event\ExportEvent") + */ + const EXPORT = 'avro_csv.exporter_export'; + /** + * The EXPORTED event occurs after the exporter has generated the content. + * + * This event allows you to further process the exported content. + * + * @Event("Avro\CsvBundle\Event\ExportedEvent") + */ + const EXPORTED = 'avro_csv.exporter_exported'; + /** + * The ROW_ADDED event occurs after the importer has generated the entity. + * + * This event allows you to alter or nullify the entity. + * + * @Event("Avro\CsvBundle\Event\RowAddedEvent") + */ + const ROW_ADDED = 'avro_csv.row_added'; + /** + * The ROW_ERROR event occurs when the importer could not import an row. + * + * This event allows you to further process the erroneous row. + * + * @Event("Avro\CsvBundle\Event\RowErrorEvent") + */ + const ROW_ERROR = 'avro_csv.row_error'; +} diff --git a/Controller/ExportController.php b/Controller/ExportController.php index c1fceda..37d8b26 100644 --- a/Controller/ExportController.php +++ b/Controller/ExportController.php @@ -7,6 +7,7 @@ namespace Avro\CsvBundle\Controller; +use Avro\CsvBundle\AvroCsvEvents; use Avro\CsvBundle\Event\ExportedEvent; use Avro\CsvBundle\Event\ExportEvent; use Symfony\Component\DependencyInjection\ContainerAwareInterface; @@ -21,6 +22,7 @@ class ExportController implements ContainerAwareInterface { use ContainerAwareTrait; + /** * Export a db table. * @@ -36,11 +38,11 @@ public function exportAction($alias) $exporter->init($class); $dispatcher = $this->container->get('event_dispatcher'); - $dispatcher->dispatch('avro_csv.exporter_export', new ExportEvent($exporter)); + $dispatcher->dispatch(AvroCsvEvents::EXPORT, new ExportEvent($exporter)); $content = $exporter->getContent(); - $dispatcher->dispatch('avro_csv.exporter_exported', new ExportedEvent($content)); + $dispatcher->dispatch(AvroCsvEvents::EXPORTED, new ExportedEvent($content)); $response = new Response($content); $response->headers->set('Content-Type', 'application/csv'); diff --git a/Controller/ImportController.php b/Controller/ImportController.php index e358faa..7234fff 100644 --- a/Controller/ImportController.php +++ b/Controller/ImportController.php @@ -22,6 +22,7 @@ class ImportController implements ContainerAwareInterface { use ContainerAwareTrait; + /** * Upload a csv. * @@ -33,12 +34,12 @@ public function uploadAction($alias) { $fieldChoices = $this->container->get('avro_csv.field_retriever')->getFields($this->container->getParameter(sprintf('avro_csv.objects.%s.class', $alias)), 'title', true); - $form = $this->container->get('form.factory')->create(ImportFormType::class, null, array('field_choices' => $fieldChoices)); + $form = $this->container->get('form.factory')->create(ImportFormType::class, null, ['field_choices' => $fieldChoices]); - return $this->container->get('templating')->renderResponse('@AvroCsv/Import/upload.html.twig', array( + return $this->container->get('templating')->renderResponse('@AvroCsv/Import/upload.html.twig', [ 'form' => $form->createView(), 'alias' => $alias, - )); + ]); } /** @@ -53,7 +54,7 @@ public function mappingAction(Request $request, $alias) { $fieldChoices = $this->container->get('avro_csv.field_retriever')->getFields($this->container->getParameter(sprintf('avro_csv.objects.%s.class', $alias)), 'title', true); - $form = $this->container->get('form.factory')->create(ImportFormType::class, null, array('field_choices' => $fieldChoices)); + $form = $this->container->get('form.factory')->create(ImportFormType::class, null, ['field_choices' => $fieldChoices]); if ('POST' == $request->getMethod()) { $form->handleRequest($request); @@ -73,19 +74,19 @@ public function mappingAction(Request $request, $alias) $headers = $this->container->get('avro_csv.importer')->toFormFieldName($fileHeaders); // Recreate form and create proper fields child for each header - $form = $this->container->get('form.factory')->create(ImportFormType::class, null, array('field_choices' => $fieldChoices)); + $form = $this->container->get('form.factory')->create(ImportFormType::class, null, ['field_choices' => $fieldChoices]); $form->get('fields')->setData(array_fill_keys((array) $headers, null)); $form->handleRequest($request); $rows = $reader->getRows($this->container->getParameter('avro_csv.sample_count')); - return $this->container->get('templating')->renderResponse('@AvroCsv/Import/mapping.html.twig', array( + return $this->container->get('templating')->renderResponse('@AvroCsv/Import/mapping.html.twig', [ 'form' => $form->createView(), 'alias' => $alias, 'headers' => array_combine((array) $headers, (array) $fileHeaders), 'headersJson' => json_encode($this->container->get('avro_case.converter')->toTitleCase($fileHeaders), JSON_FORCE_OBJECT), 'rows' => $rows, - )); + ]); } } else { return new RedirectResponse($this->container->get('router')->generate($this->container->getParameter(sprintf('avro_csv.objects.%s.redirect_route', $alias)))); @@ -104,7 +105,7 @@ public function processAction(Request $request, $alias) { $fieldChoices = $this->container->get('avro_csv.field_retriever')->getFields($this->container->getParameter(sprintf('avro_csv.objects.%s.class', $alias)), 'title', true); - $form = $this->container->get('form.factory')->create(ImportFormType::class, null, array('field_choices' => $fieldChoices)); + $form = $this->container->get('form.factory')->create(ImportFormType::class, null, ['field_choices' => $fieldChoices]); if ('POST' == $request->getMethod()) { $form->handleRequest($request); @@ -124,7 +125,7 @@ public function processAction(Request $request, $alias) $importer->import($form['fields']->getData()); - $this->container->get('session')->getFlashBag()->set('success', $importer->getImportCount().' items imported.'); + $this->container->get('session')->getFlashBag()->set('success', $importer->getImportCount().' items imported. '.$importer->getImportErrors().' errors.'); } else { $this->container->get('session')->getFlashBag()->set('error', 'Import failed. Please try again.'); } diff --git a/Doctrine/Importer.php b/Doctrine/Importer.php index bf19d5b..70c60fe 100644 --- a/Doctrine/Importer.php +++ b/Doctrine/Importer.php @@ -7,98 +7,15 @@ namespace Avro\CsvBundle\Doctrine; -use Avro\CaseBundle\Util\CaseConverter; -use Avro\CsvBundle\Event\RowAddedEvent; -use Avro\CsvBundle\Util\Reader; -use Doctrine\Common\Persistence\ObjectManager; -use Doctrine\ORM\Mapping\ClassMetadataInfo; -use Doctrine\ORM\Mapping\MappingException; -use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Avro\CsvBundle\Import\Importer as BaseImporter; /** * Import csv to doctrine entity/document. * * @author Joris de Wit */ -class Importer +class Importer extends BaseImporter { - /** - * @var string[] - */ - protected $headers; - /** - * @var string[] - */ - protected $fields; - /** - * @var ClassMetadataInfo - */ - protected $metadata; - /** - * @var Reader - */ - protected $reader; - /** - * @var EventDispatcherInterface - */ - protected $dispatcher; - /** - * @var int - */ - protected $batchSize = 20; - /** - * @var int - */ - protected $importCount = 0; - /** - * @var CaseConverter - */ - protected $caseConverter; - /** - * @var ObjectManager - */ - protected $objectManager; - /** - * @var string - */ - protected $class; - - /** - * @param Reader $reader The csv reader - * @param EventDispatcherInterface $dispatcher The event dispatcher - * @param CaseConverter $caseConverter The case Converter - * @param ObjectManager $objectManager The Doctrine Object Manager - * @param int $batchSize The batch size before flushing & clearing the om - */ - public function __construct(Reader $reader, EventDispatcherInterface $dispatcher, CaseConverter $caseConverter, ObjectManager $objectManager, $batchSize) - { - $this->reader = $reader; - $this->dispatcher = $dispatcher; - $this->caseConverter = $caseConverter; - $this->objectManager = $objectManager; - $this->batchSize = $batchSize; - } - - /** - * Import a file. - * - * @param string $file The csv file - * @param string $class The class name of the entity - * @param string $delimiter The csv's delimiter - * @param string $headerFormat The header case format - */ - public function init($file, $class, $delimiter = ',', $headerFormat = 'title') - { - $this->reader->open($file, $delimiter); - $this->class = $class; - $this->metadata = $this->objectManager->getClassMetadata($class); - if ('form' === $headerFormat) { - $this->headers = $this->toFormFieldName($this->reader->getHeaders()); - } else { - $this->headers = $this->caseConverter->convert($this->reader->getHeaders(), $headerFormat); - } - } - /** * Get the csv's header row. * @@ -108,6 +25,7 @@ public function getHeaders() { return $this->headers; } + /** * Get the csv's next row. * @@ -117,156 +35,4 @@ public function getRow() { return $this->reader->getRow(); } - - /** - * Import the csv and persist to database. - * - * @param array $fields The fields to persist - * - * @return true if successful - * - * @throws MappingException - */ - public function import($fields) - { - $fields = array_unique($this->caseConverter->toPascalCase($fields)); - - while ($row = $this->reader->getRow()) { - if (0 !== $this->importCount && ($this->importCount % $this->batchSize) === 0) { - $this->addRow($row, $fields, true); - } else { - $this->addRow($row, $fields, false); - } - ++$this->importCount; - } - - // one last flush to make sure no persisted objects get left behind - $this->objectManager->flush(); - - return true; - } - - /** - * Converts a string to a format suitable as form name. - * - * @param string|array $input - * - * @return string|array - */ - public function toFormFieldName($input) - { - if (is_array($input)) { - $result = array(); - foreach ($input as $val) { - $result[] = $this->convertToFormFieldName($val); - } - } else { - $result = $this->convertToFormFieldName($input); - } - - return $result; - } - - /** - * Generate a hash string suitable as form field name. - * - * @param string $input - * - * @return string - */ - private function convertToFormFieldName($input) - { - return sha1($input); - } - - /** - * Add Csv row to db. - * - * @param array $row An array of data - * @param array $fields An array of the fields to import - * @param bool $andFlush Flush the ObjectManager - * - * @throws MappingException - */ - private function addRow($row, $fields, $andFlush = true) - { - // Create new entity - $entity = new $this->class(); - - if (in_array('Id', $fields)) { - $key = array_search('Id', $fields); - if ($this->metadata->hasField('legacyId')) { - $entity->setLegacyId($row[$key]); - } - unset($fields[$key]); - } - - // loop through fields and set to row value - foreach ($fields as $k => $v) { - if ($this->metadata->hasField(lcfirst($v))) { - $entity->{'set'.$fields[$k]}($row[$k]); - } elseif ($this->metadata->hasAssociation(lcfirst($v))) { - $association = $this->metadata->getAssociationMapping(lcfirst($v)); - switch ($association['type']) { - case '1': // oneToOne - //Todo: - break; - case '2': // manyToOne - continue; - // still needs work - $joinColumnId = $association['joinColumns'][0]['name']; - $legacyId = $row[array_search($this->caseConverter->toCamelCase($joinColumnId), $this->headers)]; - if ($legacyId) { - try { - $criteria = array('legacyId' => $legacyId); - if ($this->useOwner) { - $criteria['owner'] = $this->owner->getId(); - } - - $associationClass = new \ReflectionClass($association['targetEntity']); - if ($associationClass->hasProperty('legacyId')) { - $relation = $this->objectManager->getRepository($association['targetEntity'])->findOneBy($criteria); - if ($relation) { - $entity->{'set'.ucfirst($association['fieldName'])}($relation); - } - } - } catch (\Exception $e) { - // legacyId does not exist - // fail silently - } - } - break; - case '4': // oneToMany - //TODO: - break; - case '8': // manyToMany - //TODO: - break; - } - } - } - - // Allow RowAddedEvent listeners to nullify objects (i.e. when invalid) - $event = new RowAddedEvent($entity, $row, $fields); - $this->dispatcher->dispatch('avro_csv.row_added', $event); - $entity = $event->getObject(); - if (null !== $entity) { - $this->objectManager->persist($entity); - } - - if ($andFlush) { - $this->objectManager->flush(); - $this->objectManager->clear($this->class); - } - } - - /** - * Get import count. - * - * @return int - */ - public function getImportCount() - { - return $this->importCount; - } } diff --git a/Event/ExportEvent.php b/Event/ExportEvent.php index dd12f86..cb10fd1 100644 --- a/Event/ExportEvent.php +++ b/Event/ExportEvent.php @@ -8,7 +8,7 @@ namespace Avro\CsvBundle\Event; use Avro\CsvBundle\Export\ExporterInterface; -use Doctrine\DBAL\Query\QueryBuilder; +use Doctrine\ORM\QueryBuilder; use Symfony\Component\EventDispatcher\Event; /** diff --git a/Event/RowAddedEvent.php b/Event/RowAddedEvent.php index 67fe8e8..9aea5d1 100644 --- a/Event/RowAddedEvent.php +++ b/Event/RowAddedEvent.php @@ -22,7 +22,7 @@ class RowAddedEvent extends Event /** * @param \stdClass|null $object The new object being persisted - * @param array $row The row being imported + * @param array $row The row being imported * @param array $fields The mapped fields */ public function __construct($object, array $row, array $fields) diff --git a/Event/RowErrorEvent.php b/Event/RowErrorEvent.php new file mode 100644 index 0000000..b59274c --- /dev/null +++ b/Event/RowErrorEvent.php @@ -0,0 +1,51 @@ + + */ +class RowErrorEvent extends Event +{ + protected $row; + protected $fields; + + /** + * @param array $row The row being imported + * @param array $fields The mapped fields + */ + public function __construct(array $row, array $fields) + { + $this->row = $row; + $this->fields = $fields; + } + + /** + * Get field row. + * + * @return array + */ + public function getRow() + { + return $this->row; + } + + /** + * Get mapped fields. + * + * @return array + */ + public function getFields() + { + return $this->fields; + } +} diff --git a/Export/Doctrine/ORM/Exporter.php b/Export/Doctrine/ORM/Exporter.php index 88594be..0129919 100644 --- a/Export/Doctrine/ORM/Exporter.php +++ b/Export/Doctrine/ORM/Exporter.php @@ -12,7 +12,7 @@ use Doctrine\ORM\EntityManager; /** - * Import csv to doctrine entity/document + * Import csv to doctrine entity/document. * * @author Joris de Wit */ @@ -29,7 +29,7 @@ public function __construct(EntityManager $entityManager) } /** - * Initialize the exporter by setting the queryBuilder + * Initialize the exporter by setting the queryBuilder. * * @param string $class The fully qualified class name */ diff --git a/Export/Exporter.php b/Export/Exporter.php index e702c6b..e56f38b 100644 --- a/Export/Exporter.php +++ b/Export/Exporter.php @@ -33,7 +33,7 @@ public function getContent() $content = null; foreach ($iterableResults as $row) { $row = reset($row); - if ($content == null) { + if (null == $content) { $content = $this->arrayToCsv(array_keys($row)); } $content .= $this->arrayToCsv(array_values($row)); @@ -53,7 +53,7 @@ public function getContent() */ public function arrayToCsv(array $fields, $delimiter = ',', $enclosure = '"') { - $output = array(); + $output = []; foreach ($fields as $field) { $output[] = $enclosure.str_replace($enclosure, $enclosure.$enclosure, $this->stringify($field)).$enclosure; } diff --git a/Export/ExporterInterface.php b/Export/ExporterInterface.php index 5e648c0..48f7acd 100644 --- a/Export/ExporterInterface.php +++ b/Export/ExporterInterface.php @@ -10,35 +10,35 @@ use Doctrine\ORM\QueryBuilder; /** - * Exporter interface + * Exporter interface. * * @author Joris de Wit */ interface ExporterInterface { /** - * Initialize the exporter by setting the queryBuilder + * Initialize the exporter by setting the queryBuilder. * * @param string $class The fully qualified class name */ public function init($class); /** - * Export all of an objects data to csv format + * Export all of an objects data to csv format. * - * @return $content + * @return string */ public function getContent(); /** - * Converts an array into a CSV string. - * - * @param array $fields The php array to convert - * @param string $delimiter The CSV delimiter - * @param string $enclosure The CSV enclosure - * - * @return string CSV formatted string - */ + * Converts an array into a CSV string. + * + * @param array $fields The php array to convert + * @param string $delimiter The CSV delimiter + * @param string $enclosure The CSV enclosure + * + * @return string CSV formatted string + */ public function arrayToCsv(array $fields, $delimiter = ',', $enclosure = '"'); /** @@ -46,5 +46,3 @@ public function arrayToCsv(array $fields, $delimiter = ',', $enclosure = '"'); */ public function getQueryBuilder(); } - - diff --git a/Form/Type/ImportFormType.php b/Form/Type/ImportFormType.php index d588698..cb5d769 100644 --- a/Form/Type/ImportFormType.php +++ b/Form/Type/ImportFormType.php @@ -31,44 +31,44 @@ public function buildForm(FormBuilderInterface $builder, array $options) ->add( 'delimiter', ChoiceType::class, - array( - 'choices' => array( + [ + 'choices' => [ 'comma' => ',', 'semicolon' => ';', 'pipe' => '|', 'colon' => ':', - ), + ], 'choices_as_values' => true, 'label' => 'Delimiter', - ) + ] ) ->add( 'file', FileType::class, - array( + [ 'label' => 'File', 'required' => true, - ) + ] ) ->add( 'filename', HiddenType::class, - array( + [ 'required' => false, - ) + ] ) ->add( 'fields', CollectionType::class, - array( + [ 'allow_add' => true, 'entry_type' => ChoiceType::class, - 'entry_options' => array( + 'entry_options' => [ 'choices' => $options['field_choices'], - ), + ], 'label' => 'Fields', 'required' => false, - ) + ] ); $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) { @@ -90,9 +90,9 @@ public function buildForm(FormBuilderInterface $builder, array $options) */ public function configureOptions(OptionsResolver $resolver) { - $resolver->setDefaults(array( - 'field_choices' => array(), - )); + $resolver->setDefaults([ + 'field_choices' => [], + ]); } /** diff --git a/Import/Importer.php b/Import/Importer.php index bc70d04..1b3bc7b 100644 --- a/Import/Importer.php +++ b/Import/Importer.php @@ -8,7 +8,9 @@ namespace Avro\CsvBundle\Import; use Avro\CaseBundle\Util\CaseConverter; +use Avro\CsvBundle\AvroCsvEvents; use Avro\CsvBundle\Event\RowAddedEvent; +use Avro\CsvBundle\Event\RowErrorEvent; use Avro\CsvBundle\Util\Reader; use Doctrine\Common\Persistence\ObjectManager; use Doctrine\ORM\Mapping\ClassMetadataInfo; @@ -50,6 +52,10 @@ class Importer * @var int */ protected $importCount = 0; + /** + * @var int + */ + protected $importErrors = 0; /** * @var CaseConverter */ @@ -111,16 +117,18 @@ public function init($file, $class, $delimiter = ',', $headerFormat = 'title') public function import($fields) { $fields = array_unique($this->caseConverter->toPascalCase($fields)); - while ($row = $this->reader->getRow()) { - if (0 !== $this->importCount && ($this->importCount % $this->batchSize) === 0) { - $this->addRow($row, $fields, true); + if (0 !== $this->importCount && 0 === ($this->importCount % $this->batchSize)) { + $result = $this->addRow($row, $fields, true); + } else { + $result = $this->addRow($row, $fields, false); + } + if ($result) { + ++$this->importCount; } else { - $this->addRow($row, $fields, false); + ++$this->importErrors; } - ++$this->importCount; } - // one last flush to make sure no persisted objects get left behind $this->objectManager->flush(); @@ -137,7 +145,7 @@ public function import($fields) public function toFormFieldName($input) { if (is_array($input)) { - $result = array(); + $result = []; foreach ($input as $val) { $result[] = $this->convertToFormFieldName($val); } @@ -160,6 +168,26 @@ private function convertToFormFieldName($input) return sha1($input); } + /** + * Get import count. + * + * @return int + */ + public function getImportCount() + { + return $this->importCount; + } + + /** + * Get import errors. + * + * @return int + */ + public function getImportErrors() + { + return $this->importErrors; + } + /** * Add Csv row to db. * @@ -167,13 +195,14 @@ private function convertToFormFieldName($input) * @param array $fields An array of the fields to import * @param bool $andFlush Flush the ObjectManager * + * @return bool + * * @throws MappingException */ private function addRow($row, $fields, $andFlush = true) { // Create new entity $entity = new $this->class(); - if (in_array('Id', $fields)) { $key = array_search('Id', $fields); if ($this->metadata->hasField('legacyId')) { @@ -181,7 +210,6 @@ private function addRow($row, $fields, $andFlush = true) } unset($fields[$key]); } - // loop through fields and set to row value foreach ($fields as $k => $v) { if ($this->metadata->hasField(lcfirst($v))) { @@ -199,7 +227,7 @@ private function addRow($row, $fields, $andFlush = true) $legacyId = $row[array_search($this->caseConverter->toCamelCase($joinColumnId), $this->headers)]; if ($legacyId) { try { - $criteria = array('legacyId' => $legacyId); + $criteria = ['legacyId' => $legacyId]; if ($this->useOwner) { $criteria['owner'] = $this->owner->getId(); } @@ -226,27 +254,20 @@ private function addRow($row, $fields, $andFlush = true) } } } - // Allow RowAddedEvent listeners to nullify objects (i.e. when invalid) $event = new RowAddedEvent($entity, $row, $fields); - $this->dispatcher->dispatch('avro_csv.row_added', $event); + $this->dispatcher->dispatch(AvroCsvEvents::ROW_ADDED, $event); $entity = $event->getObject(); - if (null !== $entity) { - $this->objectManager->persist($entity); - } + if (null === $entity) { + $this->dispatcher->dispatch(AvroCsvEvents::ROW_ERROR, new RowErrorEvent($row, $fields)); + return false; + } + $this->objectManager->persist($entity); if ($andFlush) { $this->objectManager->flush(); } - } - /** - * Get import count. - * - * @return int - */ - public function getImportCount() - { - return $this->importCount; + return true; } } diff --git a/Tests/Doctrine/ImporterTest.php b/Tests/Doctrine/ImporterTest.php index 34c9ab2..fbbe649 100644 --- a/Tests/Doctrine/ImporterTest.php +++ b/Tests/Doctrine/ImporterTest.php @@ -5,7 +5,6 @@ use Avro\CaseBundle\Util\CaseConverter; use Avro\CsvBundle\Doctrine\Importer; use Avro\CsvBundle\Util\Reader; - use Doctrine\Common\Annotations\AnnotationReader; class ImporterTest extends \PHPUnit_Framework_TestCase @@ -15,7 +14,7 @@ class ImporterTest extends \PHPUnit_Framework_TestCase protected $importer; /** - * Setup test class + * Setup test class. */ public function setUp() { @@ -38,23 +37,21 @@ public function setUp() $class = 'Avro\CsvBundle\Tests\TestEntity'; $this->importer = new Importer($reader, $dispatcher, $caseConverter, $objectManager, 5); - $this->importer->init(__DIR__ . '/../import.csv', $class, ',', 'title'); + $this->importer->init(__DIR__.'/../import.csv', $class, ',', 'title'); } /** - * Test getHeaders + * Test getHeaders. */ public function testGetHeaders() { $this->assertEquals( - array( + [ 0 => 'Header 1', 1 => 'Header 2', 2 => 'Header 3', - ), + ], $this->importer->getHeaders() ); } - - } diff --git a/Tests/Export/Doctrine/ORM/ExporterTest.php b/Tests/Export/Doctrine/ORM/ExporterTest.php index 01a9562..6738969 100644 --- a/Tests/Export/Doctrine/ORM/ExporterTest.php +++ b/Tests/Export/Doctrine/ORM/ExporterTest.php @@ -22,11 +22,11 @@ class ExporterTest extends \PHPUnit_Framework_TestCase public function setUp() { - $query = $this->getMock('Doctrine\ORM\AbstractQuery', array('iterate', 'HYDRATE_ARRAY', 'getSQL', '_doExecute'), array(), '', false); + $query = $this->getMock('Doctrine\ORM\AbstractQuery', ['iterate', 'HYDRATE_ARRAY', 'getSQL', '_doExecute'], [], '', false); $query->expects($this->any()) ->method('iterate') - ->will($this->returnValue(array(0 => array(0 => array('row 1' => 'val\'1', 'row 2' => 'val,2', 'row 3' => 'val"3'))))); - $queryBuilder = $this->getMock('Doctrine\ORM\QueryBuilder', array('select', 'from', 'getQuery'), array(), '', false); + ->will($this->returnValue([0 => [0 => ['row 1' => 'val\'1', 'row 2' => 'val,2', 'row 3' => 'val"3']]])); + $queryBuilder = $this->getMock('Doctrine\ORM\QueryBuilder', ['select', 'from', 'getQuery'], [], '', false); $queryBuilder->expects($this->any()) ->method('select') ->will($this->returnValue($queryBuilder)); @@ -39,7 +39,7 @@ public function setUp() $queryBuilder->expects($this->any()) ->method('getQuery') ->will($this->returnValue($query)); - $entityManager = $this->getMock('Doctrine\ORM\EntityManager', array('createQueryBuilder'), array(), '', false); + $entityManager = $this->getMock('Doctrine\ORM\EntityManager', ['createQueryBuilder'], [], '', false); $entityManager->expects($this->any()) ->method('createQueryBuilder') ->will($this->returnValue($queryBuilder)); @@ -63,7 +63,7 @@ public function testArrayToCsv() { $this->assertEquals( '"val\'1","val,2","val""3"'."\n", - $this->exporter->arrayToCsv(array('val\'1', 'val,2', 'val"3')) + $this->exporter->arrayToCsv(['val\'1', 'val,2', 'val"3']) ); } diff --git a/Tests/Import/ImporterTest.php b/Tests/Import/ImporterTest.php index ff402e5..e6b5f78 100644 --- a/Tests/Import/ImporterTest.php +++ b/Tests/Import/ImporterTest.php @@ -30,11 +30,11 @@ class ImporterTest extends \PHPUnit_Framework_TestCase */ public function setUp() { - $fields = array('id', 'field1', 'field2'); + $fields = ['id', 'field1', 'field2']; $this->fields = $fields; $caseConverter = new CaseConverter(); $reader = new Reader(); - $metadata = $this->getMockForAbstractClass('Doctrine\Common\Persistence\Mapping\ClassMetadata', array('hasField')); + $metadata = $this->getMockForAbstractClass('Doctrine\Common\Persistence\Mapping\ClassMetadata', ['hasField']); $metadata->expects($this->any()) ->method('hasField') ->will($this->returnCallback(function ($value) use ($fields) { diff --git a/Tests/TestEntity.php b/Tests/TestEntity.php index 3517a16..7a92c6f 100644 --- a/Tests/TestEntity.php +++ b/Tests/TestEntity.php @@ -14,6 +14,7 @@ public function getId() { return $this->id; } + public function setId($id) { $this->id = $id; @@ -23,6 +24,7 @@ public function getField1() { return $this->field1; } + public function setField1($field1) { $this->field1 = $field1; @@ -32,9 +34,9 @@ public function getField2() { return $this->field2; } + public function setField2($field2) { $this->field2 = $field2; } - } diff --git a/Tests/Util/FieldRetrieverTest.php b/Tests/Util/FieldRetrieverTest.php index 14e4d56..30bf966 100644 --- a/Tests/Util/FieldRetrieverTest.php +++ b/Tests/Util/FieldRetrieverTest.php @@ -34,12 +34,12 @@ public function testGetFields() { $this->assertEquals( $this->fieldRetriever->getFields($this->class), - array( + [ '0' => '', '1' => 'Id', '2' => 'Field1', '3' => 'Field2', - ) + ] ); } @@ -47,12 +47,12 @@ public function testGetFieldsAsCamelCase() { $this->assertEquals( $this->fieldRetriever->getFields($this->class, 'camel'), - array( + [ '0' => '', '1' => 'id', '2' => 'field1', '3' => 'field2', - ) + ] ); } @@ -60,12 +60,12 @@ public function testGetFieldsAndCopyKeys() { $this->assertEquals( $this->fieldRetriever->getFields($this->class, 'camel', true), - array( + [ '' => '', 'id' => 'id', 'field1' => 'field1', 'field2' => 'field2', - ) + ] ); } } diff --git a/Tests/Util/ReaderTest.php b/Tests/Util/ReaderTest.php index a02964a..86d4582 100644 --- a/Tests/Util/ReaderTest.php +++ b/Tests/Util/ReaderTest.php @@ -19,17 +19,17 @@ class ReaderTest extends \PHPUnit_Framework_TestCase public function setUp() { $this->reader = new Reader(); - $this->reader->open(__DIR__ . '/../import.csv'); + $this->reader->open(__DIR__.'/../import.csv'); } public function testGetHeaders() { $this->assertEquals( - array( + [ 0 => 'Header 1', 1 => 'Header 2', 2 => 'Header 3', - ), + ], $this->reader->getHeaders() ); } @@ -37,27 +37,27 @@ public function testGetHeaders() public function testGetRow() { $this->assertEquals( - array( + [ 0 => 'row1column1', 1 => 'row1column2', 2 => 'row1column3', - ), + ], $this->reader->getRow() ); $this->assertEquals( - array( + [ 0 => 'row2column1', 1 => 'row2column2', 2 => 'row2column3', - ), + ], $this->reader->getRow() ); $this->assertEquals( - array( + [ 0 => 'row3column1', 1 => 'row3column2', 2 => 'row3column3', - ), + ], $this->reader->getRow() ); } @@ -65,18 +65,18 @@ public function testGetRow() public function testGetRows() { $this->assertEquals( - array( - 0 => array( + [ + 0 => [ 0 => 'row1column1', 1 => 'row1column2', 2 => 'row1column3', - ), - 1 => array( + ], + 1 => [ 0 => 'row2column1', 1 => 'row2column2', 2 => 'row2column3', - ), - ), + ], + ], $this->reader->getRows(2) ); } diff --git a/Util/FieldRetriever.php b/Util/FieldRetriever.php index 91cd342..6716752 100644 --- a/Util/FieldRetriever.php +++ b/Util/FieldRetriever.php @@ -46,7 +46,7 @@ public function getFields($class, $format = 'title', $copyToKey = false) $reflectionClass = new \ReflectionClass($class); $properties = $reflectionClass->getProperties(); - $fields = array(); + $fields = []; foreach ($properties as $property) { $addField = true; foreach ($this->annotationReader->getPropertyAnnotations($property) as $annotation) { diff --git a/Util/Reader.php b/Util/Reader.php index 774cf68..dff5825 100644 --- a/Util/Reader.php +++ b/Util/Reader.php @@ -48,7 +48,7 @@ public function open($file, $delimiter = ',', $mode = 'r+', $enclosure = '"', $h */ public function getRow() { - if (($row = fgetcsv($this->handle, 1000, $this->delimiter, $this->enclosure)) !== false) { + if (false !== ($row = fgetcsv($this->handle, 1000, $this->delimiter, $this->enclosure))) { ++$this->line; return $row; @@ -66,7 +66,7 @@ public function getRow() */ public function getRows($count) { - $rows = array(); + $rows = []; for ($i = 0; $i < $count; ++$i) { $row = $this->getRow(); if ($row) { @@ -84,7 +84,7 @@ public function getRows($count) */ public function getAll() { - $data = array(); + $data = []; while ($row = $this->getRow()) { $data[] = $row; } From d5966e43e1f9d4243097297e73c4b14cc05e7c6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steffen=20Ro=C3=9Fkamp?= Date: Tue, 19 Feb 2019 13:45:22 +0100 Subject: [PATCH 3/3] Symfony 4, PHP 7.2+ and PHPUnit 5 compatibility --- .travis.yml | 8 ++---- Form/Type/ImportFormType.php | 1 - Import/Importer.php | 2 +- Tests/Doctrine/ImporterTest.php | 24 ++++++++++-------- Tests/Export/Doctrine/ORM/ExporterTest.php | 29 ++++++++++++++-------- Tests/Import/ImporterTest.php | 18 ++++++++------ composer.json | 10 ++++---- 7 files changed, 51 insertions(+), 41 deletions(-) diff --git a/.travis.yml b/.travis.yml index 10ebded..2550719 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,9 @@ language: php php: - - 5.5 - - 5.6 - - 7.0 - 7.1 + - 7.2 + - 7.3 before_install: - | @@ -15,6 +14,3 @@ before_install: install: - composer install --no-interaction --prefer-dist - -notifications: - email: joris.w.dewit@gmail.com diff --git a/Form/Type/ImportFormType.php b/Form/Type/ImportFormType.php index cb5d769..7c28025 100644 --- a/Form/Type/ImportFormType.php +++ b/Form/Type/ImportFormType.php @@ -38,7 +38,6 @@ public function buildForm(FormBuilderInterface $builder, array $options) 'pipe' => '|', 'colon' => ':', ], - 'choices_as_values' => true, 'label' => 'Delimiter', ] ) diff --git a/Import/Importer.php b/Import/Importer.php index 1b3bc7b..2ab6987 100644 --- a/Import/Importer.php +++ b/Import/Importer.php @@ -221,7 +221,7 @@ private function addRow($row, $fields, $andFlush = true) //Todo: break; case '2': // manyToOne - continue; + break; // still needs work $joinColumnId = $association['joinColumns'][0]['name']; $legacyId = $row[array_search($this->caseConverter->toCamelCase($joinColumnId), $this->headers)]; diff --git a/Tests/Doctrine/ImporterTest.php b/Tests/Doctrine/ImporterTest.php index fbbe649..2ee408c 100644 --- a/Tests/Doctrine/ImporterTest.php +++ b/Tests/Doctrine/ImporterTest.php @@ -5,7 +5,6 @@ use Avro\CaseBundle\Util\CaseConverter; use Avro\CsvBundle\Doctrine\Importer; use Avro\CsvBundle\Util\Reader; -use Doctrine\Common\Annotations\AnnotationReader; class ImporterTest extends \PHPUnit_Framework_TestCase { @@ -20,19 +19,22 @@ public function setUp() { $caseConverter = new CaseConverter(); $reader = new Reader(); - $metadata = $this->getMock('Doctrine\Common\Persistence\Mapping\ClassMetadata'); - $metadata->expects($this->any()) + $metadata = $this->createMock('Doctrine\Common\Persistence\Mapping\ClassMetadata'); + $metadata + ->expects($this->any()) ->method('hasField') - ->will($this->returnValue(true)); - $metadataFactory = $this->getMock('Doctrine\Common\Persistence\Mapping\ClassMetadataFactory'); - $metadataFactory->expects($this->any()) + ->willReturn(true); + $metadataFactory = $this->createMock('Doctrine\Common\Persistence\Mapping\ClassMetadataFactory'); + $metadataFactory + ->expects($this->any()) ->method('getMetadataFor') - ->will($this->returnValue($metadata)); - $objectManager = $this->getMock('Doctrine\Common\Persistence\ObjectManager'); - $objectManager->expects($this->any()) + ->willReturn($metadata); + $objectManager = $this->createMock('Doctrine\Common\Persistence\ObjectManager'); + $objectManager + ->expects($this->any()) ->method('getMetadataFactory') - ->will($this->returnValue($metadataFactory)); - $dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); + ->willReturn($metadataFactory); + $dispatcher = $this->createMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); $class = 'Avro\CsvBundle\Tests\TestEntity'; diff --git a/Tests/Export/Doctrine/ORM/ExporterTest.php b/Tests/Export/Doctrine/ORM/ExporterTest.php index 6738969..fe2f93b 100644 --- a/Tests/Export/Doctrine/ORM/ExporterTest.php +++ b/Tests/Export/Doctrine/ORM/ExporterTest.php @@ -22,27 +22,36 @@ class ExporterTest extends \PHPUnit_Framework_TestCase public function setUp() { - $query = $this->getMock('Doctrine\ORM\AbstractQuery', ['iterate', 'HYDRATE_ARRAY', 'getSQL', '_doExecute'], [], '', false); + $query = $this->getMockBuilder('Doctrine\ORM\AbstractQuery') + ->disableOriginalConstructor() + ->setMethods(['iterate', 'HYDRATE_ARRAY', 'getSQL', '_doExecute']) + ->getMock(); $query->expects($this->any()) ->method('iterate') - ->will($this->returnValue([0 => [0 => ['row 1' => 'val\'1', 'row 2' => 'val,2', 'row 3' => 'val"3']]])); - $queryBuilder = $this->getMock('Doctrine\ORM\QueryBuilder', ['select', 'from', 'getQuery'], [], '', false); + ->willReturn([0 => [0 => ['row 1' => 'val\'1', 'row 2' => 'val,2', 'row 3' => 'val"3']]]); + $queryBuilder = $this->getMockBuilder('Doctrine\ORM\QueryBuilder') + ->disableOriginalConstructor() + ->setMethods(['select', 'from', 'getQuery']) + ->getMock(); $queryBuilder->expects($this->any()) ->method('select') - ->will($this->returnValue($queryBuilder)); + ->willReturn($queryBuilder); $queryBuilder->expects($this->any()) ->method('from') - ->will($this->returnValue($queryBuilder)); + ->willReturn($queryBuilder); $queryBuilder->expects($this->any()) ->method('from') - ->will($this->returnValue($queryBuilder)); + ->willReturn($queryBuilder); $queryBuilder->expects($this->any()) ->method('getQuery') - ->will($this->returnValue($query)); - $entityManager = $this->getMock('Doctrine\ORM\EntityManager', ['createQueryBuilder'], [], '', false); + ->willReturn($query); + $entityManager = $this->getMockBuilder('Doctrine\ORM\EntityManager') + ->disableOriginalConstructor() + ->setMethods(['createQueryBuilder']) + ->getMock(); $entityManager->expects($this->any()) ->method('createQueryBuilder') - ->will($this->returnValue($queryBuilder)); + ->willReturn($queryBuilder); $this->exporter = new Exporter($entityManager); } @@ -62,7 +71,7 @@ public function testInit() public function testArrayToCsv() { $this->assertEquals( - '"val\'1","val,2","val""3"'."\n", + '"val\'1","val,2","val""3"' . "\n", $this->exporter->arrayToCsv(['val\'1', 'val,2', 'val"3']) ); } diff --git a/Tests/Import/ImporterTest.php b/Tests/Import/ImporterTest.php index e6b5f78..d38afdb 100644 --- a/Tests/Import/ImporterTest.php +++ b/Tests/Import/ImporterTest.php @@ -37,17 +37,21 @@ public function setUp() $metadata = $this->getMockForAbstractClass('Doctrine\Common\Persistence\Mapping\ClassMetadata', ['hasField']); $metadata->expects($this->any()) ->method('hasField') - ->will($this->returnCallback(function ($value) use ($fields) { - return in_array($value, $fields); - })); - $objectManager = $this->getMock('Doctrine\Common\Persistence\ObjectManager'); + ->will( + $this->returnCallback( + function ($value) use ($fields) { + return in_array($value, $fields); + } + ) + ); + $objectManager = $this->createMock('Doctrine\Common\Persistence\ObjectManager'); $objectManager->expects($this->any()) ->method('getClassMetadata') - ->will($this->returnValue($metadata)); - $dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); + ->willReturn($metadata); + $dispatcher = $this->createMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); $dispatcher->expects($this->any()) ->method('dispatch') - ->will($this->returnValue('true')); + ->willReturn('true'); $this->importer = new Importer($reader, $dispatcher, $caseConverter, $objectManager, 5); diff --git a/composer.json b/composer.json index c78c98f..8c65d42 100644 --- a/composer.json +++ b/composer.json @@ -18,17 +18,17 @@ } }, "require": { - "php": "^5.5.9|>=7.0.8", + "php": "^7.1.3", "avro/case-bundle": "*" }, "require-dev": { "twig/twig": "*", "doctrine/doctrine-bundle": "*", "doctrine/orm": "*", - "phpunit/phpunit": "~4.0|~5.0", - "symfony/form": "~2.8|~3.0|~4.0", - "symfony/config": "~2.8|~3.0|~4.0", - "symfony/dependency-injection": "~2.8|~3.0|~4.0" + "phpunit/phpunit": "^5.7", + "symfony/form": "~4.0", + "symfony/config": "~4.0", + "symfony/dependency-injection": "~4.0" }, "autoload": { "psr-4": { "Avro\\CsvBundle\\": "" }