From 40817c472089b4bcac82f4bef6169186887015a2 Mon Sep 17 00:00:00 2001 From: Vova Soroka Date: Wed, 23 Sep 2015 19:56:52 +0300 Subject: [PATCH 1/6] BAP-9064: Possible to created extended fields with identical names Conflicts: src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php --- .../EnumEntityConfigDumperExtensionTest.php | 81 ++++++++++++++++++- .../EnumEntityConfigDumperExtension.php | 5 +- .../Tools/ExtendConfigDumper.php | 44 ++++++---- .../ExtendEntityGeneratorExtension.php | 42 +++++----- 4 files changed, 133 insertions(+), 39 deletions(-) diff --git a/src/Oro/Bundle/EntityExtendBundle/Tests/Unit/Tools/DumperExtensions/EnumEntityConfigDumperExtensionTest.php b/src/Oro/Bundle/EntityExtendBundle/Tests/Unit/Tools/DumperExtensions/EnumEntityConfigDumperExtensionTest.php index 0663fc9fcab..c64632ccaca 100644 --- a/src/Oro/Bundle/EntityExtendBundle/Tests/Unit/Tools/DumperExtensions/EnumEntityConfigDumperExtensionTest.php +++ b/src/Oro/Bundle/EntityExtendBundle/Tests/Unit/Tools/DumperExtensions/EnumEntityConfigDumperExtensionTest.php @@ -560,8 +560,7 @@ public function testPostUpdateForMultiEnumFields() ] ], 'property' => [ - ExtendHelper::getMultiEnumSnapshotFieldName('field1') => - ExtendHelper::getMultiEnumSnapshotFieldName('field1') + ExtendHelper::getMultiEnumSnapshotFieldName('field1') => [] ] ], $entityConfig1->get('schema') @@ -636,11 +635,85 @@ public function testPostUpdateForMultiEnumFieldsInCustomEntity() ] ], 'property' => [ - ExtendHelper::getMultiEnumSnapshotFieldName('field1') => - ExtendHelper::getMultiEnumSnapshotFieldName('field1') + ExtendHelper::getMultiEnumSnapshotFieldName('field1') => [] ] ], $entityConfig1->get('schema') ); } + + public function testPostUpdateForDeletedMultiEnumField() + { + $entityConfig = new Config(new EntityConfigId('extend', 'Extend\EnumValue1')); + $entityConfig->set('owner', ExtendScope::OWNER_CUSTOM); + $entityConfig->set('is_extend', true); + $entityConfig->set( + 'schema', + [ + 'doctrine' => [ + 'Extend\EnumValue1' => [ + 'fields' => [ + ExtendHelper::getMultiEnumSnapshotFieldName('field2') => [ + 'column' => 'field2' + ] + ] + ] + ] + ] + ); + + $fieldConfig = new Config(new FieldConfigId('extend', 'Extend\EnumValue1', 'field1', 'multiEnum')); + $fieldConfig->set('is_deleted', true); + + $entityConfigs = [$entityConfig]; + $fieldConfigs = [$fieldConfig]; + + $extendConfigProvider = $this->getMockBuilder('Oro\Bundle\EntityConfigBundle\Provider\ConfigProvider') + ->disableOriginalConstructor() + ->getMock(); + $this->configManager->expects($this->once()) + ->method('getProvider') + ->with('extend') + ->will($this->returnValue($extendConfigProvider)); + $extendConfigProvider->expects($this->at(0)) + ->method('getConfigs') + ->with(null, true) + ->will($this->returnValue($entityConfigs)); + $extendConfigProvider->expects($this->at(1)) + ->method('getConfigs') + ->with($entityConfig->getId()->getClassName()) + ->will($this->returnValue($fieldConfigs)); + + $this->configManager->expects($this->once()) + ->method('persist') + ->with($this->identicalTo($entityConfig)); + + $this->extension->postUpdate(); + + $this->assertEquals( + [ + 'doctrine' => [ + 'Extend\EnumValue1' => [ + 'fields' => [ + ExtendHelper::getMultiEnumSnapshotFieldName('field1') => [ + 'column' => $this->nameGenerator->generateMultiEnumSnapshotColumnName('field1'), + 'type' => 'string', + 'nullable' => true, + 'length' => ExtendHelper::MAX_ENUM_SNAPSHOT_LENGTH, + ], + ExtendHelper::getMultiEnumSnapshotFieldName('field2') => [ + 'column' => 'field2' + ] + ] + ] + ], + 'property' => [ + ExtendHelper::getMultiEnumSnapshotFieldName('field1') => [ + 'private' => true + ] + ] + ], + $entityConfig->get('schema') + ); + } } diff --git a/src/Oro/Bundle/EntityExtendBundle/Tools/DumperExtensions/EnumEntityConfigDumperExtension.php b/src/Oro/Bundle/EntityExtendBundle/Tools/DumperExtensions/EnumEntityConfigDumperExtension.php index d1040780d6b..37d0a38d484 100644 --- a/src/Oro/Bundle/EntityExtendBundle/Tools/DumperExtensions/EnumEntityConfigDumperExtension.php +++ b/src/Oro/Bundle/EntityExtendBundle/Tools/DumperExtensions/EnumEntityConfigDumperExtension.php @@ -190,7 +190,10 @@ public function postUpdate() continue; } - $schema['property'][$snapshotFieldName] = $snapshotFieldName; + $schema['property'][$snapshotFieldName] = []; + if ($fieldConfig->is('is_deleted')) { + $schema['property'][$snapshotFieldName]['private'] = true; + } $schema['doctrine'][$mappingClassName]['fields'][$snapshotFieldName] = [ 'column' => $this->nameGenerator->generateMultiEnumSnapshotColumnName($fieldName), diff --git a/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php b/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php index 3074b77a9fb..4518965362b 100644 --- a/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php +++ b/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php @@ -204,22 +204,37 @@ protected function checkFields( array &$properties, array &$doctrine ) { + if ($fieldConfig->is('state', ExtendScope::STATE_DELETE)) { + $fieldConfig->set('is_deleted', true); + } else { + $fieldConfig->set('state', ExtendScope::STATE_ACTIVE); + } if ($fieldConfig->is('is_extend')) { /** @var FieldConfigId $fieldConfigId */ $fieldConfigId = $fieldConfig->getId(); $fieldName = $fieldConfigId->getFieldName(); $fieldType = $fieldConfigId->getFieldType(); + $isDeleted = $fieldConfig->is('is_deleted'); $underlyingFieldType = $this->fieldTypeHelper->getUnderlyingType($fieldType); if (in_array($underlyingFieldType, array_merge(RelationType::$anyToAnyRelations, ['optionSet']))) { - $relationProperties[$fieldName] = $fieldName; + $relationProperties[$fieldName] = []; + if ($isDeleted) { + $relationProperties[$fieldName]['private'] = true; + } if ($underlyingFieldType !== RelationType::MANY_TO_ONE && !$fieldConfig->is('without_default')) { $defaultName = self::DEFAULT_PREFIX . $fieldName; - $defaultProperties[$defaultName] = $defaultName; + $defaultProperties[$defaultName] = []; + if ($isDeleted) { + $defaultProperties[$defaultName]['private'] = true; + } } } else { - $properties[$fieldName] = $fieldName; + $properties[$fieldName] = []; + if ($isDeleted) { + $properties[$fieldName]['private'] = true; + } $doctrine[$entityName]['fields'][$fieldName] = [ 'column' => $fieldName, @@ -231,12 +246,6 @@ protected function checkFields( ]; } } - - if ($fieldConfig->is('state', ExtendScope::STATE_DELETE)) { - $fieldConfig->set('is_deleted', true); - } else { - $fieldConfig->set('state', ExtendScope::STATE_ACTIVE); - } } /** @@ -309,12 +318,17 @@ protected function checkSchema(ConfigInterface $extendConfig, $aliases, array $s if ($relation['field_id']->getFieldType() !== RelationType::MANY_TO_ONE) { $fieldName = $relation['field_id']->getFieldName(); - $addRemoveMethods[$fieldName]['self'] = $fieldName; - if ($relation['target_field_id']) { - $addRemoveMethods[$fieldName]['target'] = - $relation['target_field_id']->getFieldName(); - $addRemoveMethods[$fieldName]['is_target_addremove'] = - $relation['field_id']->getFieldType() === RelationType::MANY_TO_MANY; + $fieldConfig = $extendProvider->getConfig($relation['field_id']->getClassName(), $fieldName); + $isDeleted = $fieldConfig->is('is_deleted'); + + if (!$isDeleted) { + $addRemoveMethods[$fieldName]['self'] = $fieldName; + if ($relation['target_field_id']) { + $addRemoveMethods[$fieldName]['target'] = + $relation['target_field_id']->getFieldName(); + $addRemoveMethods[$fieldName]['is_target_addremove'] = + $relation['field_id']->getFieldType() === RelationType::MANY_TO_MANY; + } } } diff --git a/src/Oro/Bundle/EntityExtendBundle/Tools/GeneratorExtensions/ExtendEntityGeneratorExtension.php b/src/Oro/Bundle/EntityExtendBundle/Tools/GeneratorExtensions/ExtendEntityGeneratorExtension.php index 3c664263e91..ba22df543f6 100644 --- a/src/Oro/Bundle/EntityExtendBundle/Tools/GeneratorExtensions/ExtendEntityGeneratorExtension.php +++ b/src/Oro/Bundle/EntityExtendBundle/Tools/GeneratorExtensions/ExtendEntityGeneratorExtension.php @@ -89,15 +89,15 @@ protected function generateToStringMethod(array $schema, PhpClass $class) { $toString = []; foreach ($schema['property'] as $fieldName => $config) { - if ($schema['doctrine'][$schema['entity']]['fields'][$fieldName]['type'] == 'string') { + $isPrivate = is_array($config) && isset($config['private']) && $config['private']; + if (!$isPrivate && $schema['doctrine'][$schema['entity']]['fields'][$fieldName]['type'] === 'string') { $toString[] = '$this->' . $this->generateGetMethodName($fieldName) . '()'; } } - $toStringBody = 'return (string) $this->getId();'; - if (count($toString) > 0) { - $toStringBody = 'return (string)' . implode(' . ', $toString) . ';'; - } + $toStringBody = empty($toString) + ? 'return (string) $this->getId();' + : 'return (string)' . implode(' . ', $toString) . ';'; $class->setMethod($this->generateClassMethod('__toString', $toStringBody)); } @@ -109,21 +109,25 @@ protected function generateToStringMethod(array $schema, PhpClass $class) protected function generateProperties($propertyType, array $schema, PhpClass $class) { foreach ($schema[$propertyType] as $fieldName => $config) { - $class - ->setProperty(PhpProperty::create($fieldName)->setVisibility('protected')) - ->setMethod( - $this->generateClassMethod( - $this->generateGetMethodName($fieldName), - 'return $this->' . $fieldName . ';' + $class->setProperty(PhpProperty::create($fieldName)->setVisibility('protected')); + + $isPrivate = is_array($config) && isset($config['private']) && $config['private']; + if (!$isPrivate) { + $class + ->setMethod( + $this->generateClassMethod( + $this->generateGetMethodName($fieldName), + 'return $this->' . $fieldName . ';' + ) ) - ) - ->setMethod( - $this->generateClassMethod( - $this->generateSetMethodName($fieldName), - $this->getSetterBody($fieldName, $schema), - ['value'] - ) - ); + ->setMethod( + $this->generateClassMethod( + $this->generateSetMethodName($fieldName), + $this->getSetterBody($fieldName, $schema), + ['value'] + ) + ); + } } } From b73fe1d526416f5add1aae0a29324c7e5480d4c7 Mon Sep 17 00:00:00 2001 From: Vova Soroka Date: Wed, 23 Sep 2015 21:52:09 +0300 Subject: [PATCH 2/6] BAP-9064: Possible to created extended fields with identical names Conflicts: src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php --- .../Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php b/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php index 4518965362b..69faf193f6e 100644 --- a/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php +++ b/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php @@ -317,9 +317,9 @@ protected function checkSchema(ConfigInterface $extendConfig, $aliases, array $s $relation['assign'] = true; if ($relation['field_id']->getFieldType() !== RelationType::MANY_TO_ONE) { $fieldName = $relation['field_id']->getFieldName(); - - $fieldConfig = $extendProvider->getConfig($relation['field_id']->getClassName(), $fieldName); - $isDeleted = $fieldConfig->is('is_deleted'); + $isDeleted = $extendProvider->hasConfig($relation['field_id']->getClassName(), $fieldName) + ? $extendProvider->getConfig($relation['field_id']->getClassName(), $fieldName)->is('is_deleted') + : false; if (!$isDeleted) { $addRemoveMethods[$fieldName]['self'] = $fieldName; From 9d368bc8af8e388e246ca54a8784494620c3defd Mon Sep 17 00:00:00 2001 From: Denis Voronin Date: Thu, 24 Sep 2015 20:05:24 +0300 Subject: [PATCH 3/6] BAP-9052: Impossible to upgrade with deleted fields --- .../EntityExtendBundle/Tools/ExtendConfigDumper.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php b/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php index 69faf193f6e..6d247d9209c 100644 --- a/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php +++ b/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php @@ -288,11 +288,11 @@ protected function checkSchema(ConfigInterface $extendConfig, $aliases, array $s ]; } - $schema = $extendConfig->get('schema'); - $properties = []; - $relationProperties = $schema ? $schema['relation'] : []; - $defaultProperties = []; - $addRemoveMethods = []; + $schema = $extendConfig->get('schema', false, []); + $properties = isset($schema['property']) && null !== $filter ? $schema['property'] : []; + $relationProperties = isset($schema['relation']) && null !== $filter ? $schema['relation'] : []; + $defaultProperties = isset($schema['default']) && null !== $filter ? $schema['default'] : []; + $addRemoveMethods = isset($schema['addremove']) && null !== $filter ? $schema['addremove'] : []; $fieldConfigs = $extendProvider->filter($this->createOriginFilterCallback($skippedOrigins), $className, true); foreach ($fieldConfigs as $fieldConfig) { From 44d167767b9425ff4541b6cf8ecbd800d078452f Mon Sep 17 00:00:00 2001 From: Denis Voronin Date: Fri, 25 Sep 2015 14:48:36 +0300 Subject: [PATCH 4/6] BAP-9052: Impossible to upgrade with deleted fields --- .../EventListener/OptionSetListener.php | 2 +- .../EntityExtendBundle/Tools/ExtendConfigDumper.php | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Oro/Bundle/EntityConfigBundle/EventListener/OptionSetListener.php b/src/Oro/Bundle/EntityConfigBundle/EventListener/OptionSetListener.php index 5ed92edf22a..be64047fa3c 100644 --- a/src/Oro/Bundle/EntityConfigBundle/EventListener/OptionSetListener.php +++ b/src/Oro/Bundle/EntityConfigBundle/EventListener/OptionSetListener.php @@ -48,7 +48,7 @@ public function postPersist(LifecycleEventArgs $event) return; } - foreach ($schema['relation'] as $fieldName) { + foreach ($schema['relation'] as $fieldName => $fieldOptions) { if (!$configProvider->hasConfig($className, $fieldName)) { continue; } diff --git a/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php b/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php index 6d247d9209c..7177fd46547 100644 --- a/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php +++ b/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php @@ -289,10 +289,10 @@ protected function checkSchema(ConfigInterface $extendConfig, $aliases, array $s } $schema = $extendConfig->get('schema', false, []); - $properties = isset($schema['property']) && null !== $filter ? $schema['property'] : []; - $relationProperties = isset($schema['relation']) && null !== $filter ? $schema['relation'] : []; - $defaultProperties = isset($schema['default']) && null !== $filter ? $schema['default'] : []; - $addRemoveMethods = isset($schema['addremove']) && null !== $filter ? $schema['addremove'] : []; + $properties = isset($schema['property']) ? $schema['property'] : []; + $relationProperties = isset($schema['relation']) ? $schema['relation'] : []; + $defaultProperties = isset($schema['default']) ? $schema['default'] : []; + $addRemoveMethods = isset($schema['addremove']) ? $schema['addremove'] : []; $fieldConfigs = $extendProvider->filter($this->createOriginFilterCallback($skippedOrigins), $className, true); foreach ($fieldConfigs as $fieldConfig) { @@ -437,7 +437,7 @@ protected function updateRelationValues($targetClass, FieldConfigId $fieldId) /** @var FieldConfigId $relationFieldId */ $relationFieldId = $relation['field_id']; if ($relationFieldId) { - $schema['relation'][$relationFieldId->getFieldName()] = $relationFieldId->getFieldName(); + $schema['relation'][$relationFieldId->getFieldName()] = []; } } } From 9771a197b6bfa2418f2549226c511eaf12a6d078 Mon Sep 17 00:00:00 2001 From: Denis Voronin Date: Fri, 25 Sep 2015 15:20:59 +0300 Subject: [PATCH 5/6] BAP-9052: Impossible to upgrade with deleted fields - Fix test --- .../Tests/Unit/EventListener/OptionSetListenerTest.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Oro/Bundle/EntityConfigBundle/Tests/Unit/EventListener/OptionSetListenerTest.php b/src/Oro/Bundle/EntityConfigBundle/Tests/Unit/EventListener/OptionSetListenerTest.php index 7fab975b732..96d52d26ec8 100644 --- a/src/Oro/Bundle/EntityConfigBundle/Tests/Unit/EventListener/OptionSetListenerTest.php +++ b/src/Oro/Bundle/EntityConfigBundle/Tests/Unit/EventListener/OptionSetListenerTest.php @@ -113,7 +113,11 @@ public function testPostPersist() $fieldName = 'testFieldName'; $secondFieldName = 'secondTestField'; - $schema = array('relation' => array($fieldName, $secondFieldName, $thirdFieldName)); + $schema = array('relation' => array( + $fieldName => array(), + $secondFieldName => array(), + $thirdFieldName => array() + )); $configProvider = $this->getMockBuilder('Oro\Bundle\EntityConfigBundle\Provider\ConfigProvider') ->disableOriginalConstructor() From 9150f6eabd584266f8889aab7deb8fb351f13548 Mon Sep 17 00:00:00 2001 From: Denis Voronin Date: Fri, 25 Sep 2015 19:26:27 +0300 Subject: [PATCH 6/6] BAP-9052: Impossible to upgrade with deleted fields --- .../Tools/ExtendConfigDumper.php | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php b/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php index 7177fd46547..a879882fb3a 100644 --- a/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php +++ b/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php @@ -253,8 +253,8 @@ protected function checkFields( * @SuppressWarnings(PHPMD.ExcessiveMethodLength) * * @param ConfigInterface $extendConfig - * @param array|null $aliases - * @param array|null $skippedOrigins + * @param array|null $aliases + * @param array|null $skippedOrigins */ protected function checkSchema(ConfigInterface $extendConfig, $aliases, array $skippedOrigins = null) { @@ -288,11 +288,12 @@ protected function checkSchema(ConfigInterface $extendConfig, $aliases, array $s ]; } - $schema = $extendConfig->get('schema', false, []); - $properties = isset($schema['property']) ? $schema['property'] : []; + $schema = $extendConfig->get('schema', false, []); + $properties = isset($schema['property']) && !empty($skippedOrigins) ? $schema['property'] : []; + // Need to check if relations already exists cause we can update them in updateRelationValues. $relationProperties = isset($schema['relation']) ? $schema['relation'] : []; - $defaultProperties = isset($schema['default']) ? $schema['default'] : []; - $addRemoveMethods = isset($schema['addremove']) ? $schema['addremove'] : []; + $defaultProperties = isset($schema['default']) && !empty($skippedOrigins) ? $schema['default'] : []; + $addRemoveMethods = isset($schema['addremove']) && !empty($skippedOrigins) ? $schema['addremove'] : []; $fieldConfigs = $extendProvider->filter($this->createOriginFilterCallback($skippedOrigins), $className, true); foreach ($fieldConfigs as $fieldConfig) { @@ -310,29 +311,33 @@ protected function checkSchema(ConfigInterface $extendConfig, $aliases, array $s $relations = $extendConfig->get('relation', false, []); foreach ($relations as &$relation) { - if (!$relation['field_id']) { + /** @var FieldConfigId $fieldId */ + $fieldId = $relation['field_id']; + if (!$fieldId) { continue; } $relation['assign'] = true; - if ($relation['field_id']->getFieldType() !== RelationType::MANY_TO_ONE) { - $fieldName = $relation['field_id']->getFieldName(); - $isDeleted = $extendProvider->hasConfig($relation['field_id']->getClassName(), $fieldName) - ? $extendProvider->getConfig($relation['field_id']->getClassName(), $fieldName)->is('is_deleted') + if ($fieldId->getFieldType() !== RelationType::MANY_TO_ONE) { + $fieldName = $fieldId->getFieldName(); + $isDeleted = $extendProvider->hasConfig($fieldId->getClassName(), $fieldName) + ? $extendProvider->getConfig($fieldId->getClassName(), $fieldName)->is('is_deleted') : false; if (!$isDeleted) { $addRemoveMethods[$fieldName]['self'] = $fieldName; - if ($relation['target_field_id']) { + /** @var FieldConfigId $targetFieldId */ + $targetFieldId = $relation['target_field_id']; + if ($targetFieldId) { $addRemoveMethods[$fieldName]['target'] = - $relation['target_field_id']->getFieldName(); + $targetFieldId->getFieldName(); $addRemoveMethods[$fieldName]['is_target_addremove'] = - $relation['field_id']->getFieldType() === RelationType::MANY_TO_MANY; + $targetFieldId->getFieldType() === RelationType::MANY_TO_MANY; } } } - $this->updateRelationValues($relation['target_entity'], $relation['field_id']); + $this->updateRelationValues($relation['target_entity'], $fieldId); } $extendConfig->set('relation', $relations);