Merge branch 'KIME-4583' into 'master'

Refactoring and argument identifier



See merge request !7

Classes/WE/SpreadsheetImport/Command/SpreadsheetImportCommandController.php

@@ -54,7 +54,7 @@ class SpreadsheetImportCommandController extends CommandController {
 			$this->quit();
 		}
 		/** @var SpreadsheetImport $spreadsheetImport */
-		$spreadsheetImport = $this->spreadsheetImportRepository->findPreviousOneInQueue();
+		$spreadsheetImport = $this->spreadsheetImportRepository->findNextInQueue();
 		if ($spreadsheetImport instanceof SpreadsheetImport) {
 			// mark importing status as "Progressing" before continuing the importing
 			$spreadsheetImport->setImportingStatus(SpreadsheetImport::IMPORTING_STATUS_IN_PROGRESS);

Classes/WE/SpreadsheetImport/Domain/Model/SpreadsheetImport.php

@@ -19,10 +19,11 @@ use Doctrine\ORM\Mapping as ORM;
  */
 class SpreadsheetImport {
 
-	const IMPORTING_STATUS_IN_QUEUE = 0;
-	const IMPORTING_STATUS_IN_PROGRESS = 1;
-	const IMPORTING_STATUS_COMPLETED = 2;
-	const IMPORTING_STATUS_FAILED = 3;
+	const IMPORTING_STATUS_DRAFT = 0;
+	const IMPORTING_STATUS_IN_QUEUE = 1;
+	const IMPORTING_STATUS_IN_PROGRESS = 2;
+	const IMPORTING_STATUS_COMPLETED = 3;
+	const IMPORTING_STATUS_FAILED = 4;
 
 	/**
 	 * @var string
@@ -72,7 +73,7 @@ class SpreadsheetImport {
 	 * @var int
 	 * @ORM\Column(options={"default": 0})
 	 */
-	protected $importingStatus = self::IMPORTING_STATUS_IN_QUEUE;
+	protected $importingStatus = self::IMPORTING_STATUS_DRAFT;
 
 	/**
 	 * @var int

Classes/WE/SpreadsheetImport/Domain/Repository/SpreadsheetImportRepository.php

@@ -29,10 +29,10 @@ class SpreadsheetImportRepository extends Repository {
 	/**
 	 * @return SpreadsheetImport
 	 */
-	public function findPreviousOneInQueue() {
+	public function findNextInQueue() {
 		$query = $this->createQuery();
 		$constraint = $query->logicalAnd(
-			$query->equals('importingStatus', SpreadsheetImport::IMPORTING_STATUS_IN_QUEUE),
+			$query->lessThanOrEqual('importingStatus', SpreadsheetImport::IMPORTING_STATUS_IN_QUEUE),
 			$query->lessThanOrEqual('scheduleDate', new \DateTime())
 		);
 		return $query->matching($constraint)
@@ -51,4 +51,18 @@ class SpreadsheetImportRepository extends Repository {
 		return $query->matching($constraint)->execute();
 	}
 
+	/**
+	 * @param string $context
+	 * @param array $arguments
+	 *
+	 * @return \TYPO3\Flow\Persistence\QueryResultInterface
+	 */
+	public function findByContextAndArguments($context, $arguments = array()) {
+		$query = $this->createQuery();
+		$constraint = $query->logicalAnd(
+			$query->equals('context', $context),
+			$query->equals('arguments', serialize($arguments))
+		);
+		return $query->matching($constraint)->execute();
+	}
 }

Classes/WE/SpreadsheetImport/FrontendMappingUtility.php → Classes/WE/SpreadsheetImport/FrontendMappingService.php

@@ -11,42 +11,76 @@ namespace WE\SpreadsheetImport;
  * The TYPO3 project - inspiring people to share!                         *
  *                                                                        */
 
+use TYPO3\Flow\Annotations as Flow;
 use TYPO3\Flow\Mvc\ActionRequest;
 use WE\SpreadsheetImport\Annotations\Mapping;
 
 /**
- * Utility class of basic FE mapping functionality for simple usage on separate implementations.
+ * Service class of basic FE mapping functionality for simple usage on separate implementations.
+ *
+ * @Flow\Scope("singleton")
  */
-class FrontendMappingUtility {
+class FrontendMappingService {
 
 	/**
-	 * @param \WE\SpreadsheetImport\SpreadsheetImportService $spreadsheetImportService
+	 * @Flow\Inject
+	 * @var \WE\SpreadsheetImport\SpreadsheetImportService
+	 */
+	protected $spreadsheetImportService;
+
+	/**
+	 * @Flow\InjectConfiguration
+	 * @var array
+	 */
+	protected $settings;
+
+	/**
+	 * @param string $context
 	 * @param \TYPO3\Flow\Mvc\ActionRequest $request
 	 *
 	 * @return array
 	 */
-	public static function getSpreadsheetImportMappingByRequest(SpreadsheetImportService $spreadsheetImportService, ActionRequest $request) {
+	public function getContextArgumentsForRequest($context, ActionRequest $request) {
+		$arguments = array();
+		$contextArguments = $this->settings[$context]['arguments'];
+		if (is_array($contextArguments)) {
+			foreach ($contextArguments as $contextArgument) {
+				$name = $contextArgument['name'];
+				$default = isset($contextArgument['default']) ? $contextArgument['default'] : NULL;
+				$arguments[$name] = $request->hasArgument($name) ? $request->getArgument($name) : $default;
+			}
+		}
+		return $arguments;
+	}
+
+	/**
+	 * @param array $mappingProperties
+	 * @param array $columns
+	 *
+	 * @return array
+	 */
+	public function getSpreadsheetImportMapping($mappingProperties, $columns = array()) {
 		$mappings = array();
-		$domainMappingProperties = $spreadsheetImportService->getMappingProperties();
-		foreach ($domainMappingProperties as $property => $mapping) {
-			$column = $request->getArgument($property);
-			$mappings[$property] = $column;
+		foreach ($mappingProperties as $property => $mapping) {
+			$column = isset($columns[$property]) ? $columns[$property] : '';
+			$columnMapping = array('column' => $column, 'mapping' => $mapping);
+			$mappings[$property] = $columnMapping;
 		}
 		return $mappings;
 	}
 
 	/**
-	 * @param \WE\SpreadsheetImport\SpreadsheetImportService $spreadsheetImportService
+	 * @param array $mapping
 	 * @param int $record
 	 *
 	 * @return array
 	 */
-	public static function getMappingPreview(SpreadsheetImportService $spreadsheetImportService, $record) {
-		$domainMappingProperties = $spreadsheetImportService->getMappingProperties();
-		$previewObject = $spreadsheetImportService->getObjectByRow($record);
+	public function getMappingPreview($mapping, $record) {
+		$previewObject = $this->spreadsheetImportService->getObjectByRow($record);
 		$preview = array();
-		/** @var Mapping $mapping */
-		foreach ($domainMappingProperties as $property => $mapping) {
+		foreach ($mapping as $property => $columnMapping) {
+			/** @var Mapping $mapping */
+			$mapping = $columnMapping['mapping'];
 			$getter = empty($mapping->getter) ? 'get' . ucfirst($property) : $mapping->getter;
 			$preview[$property] = $previewObject->$getter();
 		}

Classes/WE/SpreadsheetImport/SpreadsheetImportService.php

@@ -12,6 +12,7 @@ namespace WE\SpreadsheetImport;
  *                                                                        */
 
 use TYPO3\Flow\Annotations as Flow;
+use TYPO3\Flow\Persistence\QueryInterface;
 use TYPO3\Flow\Persistence\RepositoryInterface;
 use WE\SpreadsheetImport\Annotations\Mapping;
 use WE\SpreadsheetImport\Domain\Model\SpreadsheetImport;
@@ -20,6 +21,7 @@ use WE\SpreadsheetImport\Domain\Model\SpreadsheetImport;
  * @Flow\Scope("singleton")
  */
 class SpreadsheetImportService {
+
 	/**
 	 * @var SpreadsheetImport
 	 */
@@ -30,16 +32,6 @@ class SpreadsheetImportService {
 	 */
 	protected $domain;
 
-	/**
-	 * @var array
-	 */
-	protected $mappingProperties;
-
-	/**
-	 * @var array
-	 */
-	protected $columnPropertyMapping;
-
 	/**
 	 * @Flow\InjectConfiguration
 	 * @var array
@@ -76,6 +68,13 @@ class SpreadsheetImportService {
 	 */
 	protected $validatorResolver;
 
+	/**
+	 * Inverse SpreadsheetImport mapping array
+	 *
+	 * @var array
+	 */
+	private $inverseSpreadsheetImportMapping;
+
 	/**
 	 * @param \WE\SpreadsheetImport\Domain\Model\SpreadsheetImport $spreadsheetImport
 	 *
@@ -84,64 +83,20 @@ class SpreadsheetImportService {
 	public function init(SpreadsheetImport $spreadsheetImport) {
 		$this->spreadsheetImport = $spreadsheetImport;
 		$this->domain = $this->settings[$spreadsheetImport->getContext()]['domain'];
-		$this->initDomainMappingProperties();
-		$this->initColumnPropertyMapping();
 
 		return $this;
 	}
 
 	/**
-	 * Initializes the properties declared by annotations.
+	 * @return array
 	 */
-	private function initDomainMappingProperties() {
-		$this->mappingProperties = array();
+	public function getAnnotationMappingProperties() {
+		$mappingPropertyAnnotations = array();
 		$properties = $this->reflectionService->getPropertyNamesByAnnotation($this->domain, Mapping::class);
 		foreach ($properties as $property) {
-			$this->mappingProperties[$property] = $this->reflectionService->getPropertyAnnotation($this->domain, $property, Mapping::class);
+			$mappingPropertyAnnotations[$property] = $this->reflectionService->getPropertyAnnotation($this->domain, $property, Mapping::class);
 		}
-	}
-
-	/**
-	 * Flip mapping and return it as a 2-dim array in case the same column is assigned to multiple properties
-	 */
-	private function initColumnPropertyMapping() {
-		$this->columnPropertyMapping = array();
-		foreach ($this->spreadsheetImport->getMapping() as $property => $column) {
-			$this->columnPropertyMapping[$column][] = $property;
-		}
-	}
-
-	/**
-	 * Adds additional mapping properties to the domain mapping properties retrieved by annotations. This increases
-	 * flexibility for dynamic property mapping.
-	 *
-	 * This was implemented for the single use case to support the Flow package Radmiraal.CouchDB
-	 *
-	 * Note: Those additional property configurations are not persisted and need to be added after each initialization
-	 * of the service. The persisted mappings in the SpreadsheetImport object only contain the property without any
-	 * configuration. Therefore, the import works but only for the default setters and without identifiers. To support
-	 * all, the additional mapping properties need to be persisted together with the mappings.
-	 *
-	 * @param array $additionalMappingProperties
-	 */
-	public function addAdditionalMappingProperties(array $additionalMappingProperties) {
-		$this->mappingProperties = array_merge($this->mappingProperties, $additionalMappingProperties);
-	}
-
-	/**
-	 * @return array
-	 */
-	public function getMappingProperties() {
-		return $this->mappingProperties;
-	}
-
-	/**
-	 * @param string $context
-	 *
-	 * @return array
-	 */
-	public function getArgumentsByContext($context) {
-		return $this->settings[$context]['arguments'];
+		return $mappingPropertyAnnotations;
 	}
 
 	/**
@@ -274,6 +229,7 @@ class SpreadsheetImportService {
 	 * @return array
 	 */
 	private function getDomainMappingIdentifierProperties() {
+		// TODO: Don't use the annotation properties but the SpreadsheetImport mapping since we store the Mapping object there as well
 		$domainMappingProperties = array();
 		$properties = $this->reflectionService->getPropertyNamesByAnnotation($this->domain, Mapping::class);
 		foreach ($properties as $property) {
@@ -298,13 +254,14 @@ class SpreadsheetImportService {
 		$spreadsheetImportMapping = $this->spreadsheetImport->getMapping();
 		/** @var Mapping $mapping */
 		foreach ($identifierProperties as $property => $mapping) {
-			$column = $spreadsheetImportMapping[$property];
+			$column = $spreadsheetImportMapping[$property]['column'];
 			/** @var \PHPExcel_Worksheet_RowCellIterator $cellIterator */
 			$cellIterator = $row->getCellIterator($column, $column);
 			$value = $cellIterator->current()->getValue();
 			$propertyName = $mapping->queryPropertyName ?: $property;
 			$constraints[] = $query->equals($propertyName, $value);
 		}
+		$this->mergeQueryConstraintsWithArgumentIdentifiers($query, $constraints);
 		if (!empty($constraints)) {
 			return $query->matching($query->logicalAnd($constraints))->execute()->getFirst();
 		} else {
@@ -312,6 +269,26 @@ class SpreadsheetImportService {
 		}
 	}
 
+	/**
+	 * @param \TYPO3\Flow\Persistence\QueryInterface $query
+	 * @param array $constraints
+	 */
+	private function mergeQueryConstraintsWithArgumentIdentifiers(QueryInterface $query, &$constraints) {
+		$contextArguments = $this->settings[$this->spreadsheetImport->getContext()]['arguments'];
+		if (is_array($contextArguments)) {
+			foreach ($contextArguments as $contextArgument) {
+				if (isset($contextArgument['identifier']) && $contextArgument['identifier'] == TRUE) {
+					$name = $contextArgument['name'];
+					$arguments = $this->spreadsheetImport->getArguments();
+					if (array_key_exists($name, $arguments)) {
+						$value = $arguments[$name];
+						$constraints[] = $query->equals($name, $value);
+					}
+				}
+			}
+		}
+	}
+
 	/**
 	 * @param array $identifiers
 	 *
@@ -328,20 +305,17 @@ class SpreadsheetImportService {
 	 * @param \PHPExcel_Worksheet_Row $row
 	 */
 	private function setObjectPropertiesByRow($object, $row) {
-		$domainMappingProperties = $this->mappingProperties;
+		$inverseSpreadsheetImportMapping = $this->getInverseSpreadsheetImportMapping();
 		/** @var \PHPExcel_Cell $cell */
 		foreach ($row->getCellIterator() as $cell) {
 			$column = $cell->getColumn();
-			if (array_key_exists($column, $this->columnPropertyMapping)) {
-				$properties = $this->columnPropertyMapping[$column];
-				foreach ($properties as $property) {
-					if (array_key_exists($property, $domainMappingProperties)) {
-						/** @var Mapping $mapping */
-						$mapping = $domainMappingProperties[$property];
-						$setter = empty($mapping->setter) ? 'set' . ucfirst($property) : $mapping->setter;
-					} else {
-						$setter = 'set' . ucfirst($property);
-					}
+			if (array_key_exists($column, $inverseSpreadsheetImportMapping)) {
+				$properties = $inverseSpreadsheetImportMapping[$column];
+				foreach ($properties as $propertyMapping) {
+					$property = $propertyMapping['property'];
+					/** @var Mapping $mapping */
+					$mapping = $propertyMapping['mapping'];
+					$setter = empty($mapping->setter) ? 'set' . ucfirst($property) : $mapping->setter;
 					$object->$setter($cell->getValue());
 				}
 			}
@@ -349,11 +323,28 @@ class SpreadsheetImportService {
 		$this->setObjectArgumentProperties($object);
 	}
 
+	/**
+	 * Return an inverse SpreadsheetImport mapping array. It flips the property and column attribute and returns it as a
+	 * 3-dim array instead of a 2-dim array. The reason for that is the case when the same column is assigned to multiple
+	 * properties.
+	 */
+	private function getInverseSpreadsheetImportMapping() {
+		if (empty($this->inverseSpreadsheetImportMapping)) {
+			$this->inverseSpreadsheetImportMapping = array();
+			foreach ($this->spreadsheetImport->getMapping() as $property => $columnMapping) {
+				$column = $columnMapping['column'];
+				$propertyMapping = array('property' => $property, 'mapping' => $columnMapping['mapping']);
+				$this->inverseSpreadsheetImportMapping[$column][] = $propertyMapping;
+			}
+		}
+		return $this->inverseSpreadsheetImportMapping;
+	}
+
 	/**
 	 * @param $object
 	 */
 	private function setObjectArgumentProperties($object) {
-		$contextArguments = $this->getArgumentsByContext($this->spreadsheetImport->getContext());
+		$contextArguments = $this->settings[$this->spreadsheetImport->getContext()]['arguments'];
 		if (is_array($contextArguments)) {
 			$arguments = $this->spreadsheetImport->getArguments();
 			foreach ($contextArguments as $contextArgument) {

Tests/Functional/SpreadsheetImportServiceTest.php

@@ -62,7 +62,9 @@ class SpreadsheetImportServiceTest extends FunctionalTestCase {
 		$spreadsheetImport->setContext('testing');
 		$resource = $this->resourceManager->importResource(__DIR__ . '/Fixtures/Resources/sample.xlsx');
 		$spreadsheetImport->setFile($resource);
-		$spreadsheetImport->setMapping(array('id' => 'C', 'name' => 'A'));
+		$idMapping = array('column' => 'C', 'mapping' => new Mapping());
+		$nameMapping = array('column' => 'A', 'mapping' => new Mapping());
+		$spreadsheetImport->setMapping(array('id' => $idMapping, 'name' => $nameMapping));
 		$this->spreadsheetImportService->init($spreadsheetImport);
 	}
 
@@ -70,7 +72,7 @@ class SpreadsheetImportServiceTest extends FunctionalTestCase {
 	 * @test
 	 */
 	public function getMappingPropertiesReturnsPropertiesWithMappingAnnotation() {
-		$properties = $this->spreadsheetImportService->getMappingProperties();
+		$properties = $this->spreadsheetImportService->getAnnotationMappingProperties();
 		$this->assertArrayHasKey('id', $properties);
 		$this->assertArrayHasKey('name', $properties);
 		/** @var Mapping $id */