Complex classes like SchemaValidator often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use SchemaValidator, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 37 | class SchemaValidator |
||
| 38 | { |
||
| 39 | /** |
||
| 40 | * @var EntityManagerInterface |
||
| 41 | */ |
||
| 42 | private $em; |
||
| 43 | |||
| 44 | /** |
||
| 45 | * @param EntityManagerInterface $em |
||
| 46 | */ |
||
| 47 | 49 | public function __construct(EntityManagerInterface $em) |
|
| 51 | |||
| 52 | /** |
||
| 53 | * Checks the internal consistency of all mapping files. |
||
| 54 | * |
||
| 55 | * There are several checks that can't be done at runtime or are too expensive, which can be verified |
||
| 56 | * with this command. For example: |
||
| 57 | * |
||
| 58 | * 1. Check if a relation with "mappedBy" is actually connected to that specified field. |
||
| 59 | * 2. Check if "mappedBy" and "inversedBy" are consistent to each other. |
||
| 60 | * 3. Check if "referencedColumnName" attributes are really pointing to primary key columns. |
||
| 61 | * |
||
| 62 | * @return array |
||
| 63 | */ |
||
| 64 | 6 | public function validateMapping() |
|
| 78 | |||
| 79 | /** |
||
| 80 | * Validates a single class of the current. |
||
| 81 | * |
||
| 82 | * @param ClassMetadataInfo $class |
||
| 83 | * |
||
| 84 | * @return array |
||
| 85 | */ |
||
| 86 | 49 | public function validateClass(ClassMetadataInfo $class) |
|
| 87 | { |
||
| 88 | 49 | $ce = []; |
|
| 89 | 49 | $cmf = $this->em->getMetadataFactory(); |
|
| 90 | |||
| 91 | 49 | foreach ($class->fieldMappings as $fieldName => $mapping) { |
|
| 92 | 48 | if (!Type::hasType($mapping['type'])) { |
|
| 93 | 48 | $ce[] = "The field '" . $class->name . "#" . $fieldName."' uses a non-existent type '" . $mapping['type'] . "'."; |
|
| 94 | } |
||
| 95 | } |
||
| 96 | |||
| 97 | 49 | foreach ($class->associationMappings as $fieldName => $assoc) { |
|
| 98 | 46 | if (!class_exists($assoc['targetEntity']) || $cmf->isTransient($assoc['targetEntity'])) { |
|
| 99 | $ce[] = "The target entity '" . $assoc['targetEntity'] . "' specified on " . $class->name . '#' . $fieldName . ' is unknown or not an entity.'; |
||
| 100 | |||
| 101 | return $ce; |
||
| 102 | } |
||
| 103 | |||
| 104 | 46 | if ($assoc['mappedBy'] && $assoc['inversedBy']) { |
|
| 105 | $ce[] = "The association " . $class . "#" . $fieldName . " cannot be defined as both inverse and owning."; |
||
| 106 | } |
||
| 107 | |||
| 108 | 46 | $targetMetadata = $cmf->getMetadataFor($assoc['targetEntity']); |
|
| 109 | |||
| 110 | 46 | if (isset($assoc['id']) && $targetMetadata->containsForeignIdentifier) { |
|
|
|
|||
| 111 | 1 | $ce[] = "Cannot map association '" . $class->name. "#". $fieldName ." as identifier, because " . |
|
| 112 | 1 | "the target entity '". $targetMetadata->name . "' also maps an association as identifier."; |
|
| 113 | } |
||
| 114 | |||
| 115 | 46 | if ($assoc['mappedBy']) { |
|
| 116 | 40 | if ($targetMetadata->hasField($assoc['mappedBy'])) { |
|
| 117 | $ce[] = "The association " . $class->name . "#" . $fieldName . " refers to the owning side ". |
||
| 118 | "field " . $assoc['targetEntity'] . "#" . $assoc['mappedBy'] . " which is not defined as association, but as field."; |
||
| 119 | } |
||
| 120 | 40 | if (!$targetMetadata->hasAssociation($assoc['mappedBy'])) { |
|
| 121 | $ce[] = "The association " . $class->name . "#" . $fieldName . " refers to the owning side ". |
||
| 122 | "field " . $assoc['targetEntity'] . "#" . $assoc['mappedBy'] . " which does not exist."; |
||
| 123 | 40 | } elseif ($targetMetadata->associationMappings[$assoc['mappedBy']]['inversedBy'] == null) { |
|
| 124 | 1 | $ce[] = "The field " . $class->name . "#" . $fieldName . " is on the inverse side of a ". |
|
| 125 | 1 | "bi-directional relationship, but the specified mappedBy association on the target-entity ". |
|
| 126 | 1 | $assoc['targetEntity'] . "#" . $assoc['mappedBy'] . " does not contain the required ". |
|
| 127 | 1 | "'inversedBy=\"" . $fieldName . "\"' attribute."; |
|
| 128 | 39 | } elseif ($targetMetadata->associationMappings[$assoc['mappedBy']]['inversedBy'] != $fieldName) { |
|
| 129 | $ce[] = "The mappings " . $class->name . "#" . $fieldName . " and " . |
||
| 130 | $assoc['targetEntity'] . "#" . $assoc['mappedBy'] . " are ". |
||
| 131 | "inconsistent with each other."; |
||
| 132 | } |
||
| 133 | } |
||
| 134 | |||
| 135 | 46 | if ($assoc['inversedBy']) { |
|
| 136 | 37 | if ($targetMetadata->hasField($assoc['inversedBy'])) { |
|
| 137 | $ce[] = "The association " . $class->name . "#" . $fieldName . " refers to the inverse side ". |
||
| 138 | "field " . $assoc['targetEntity'] . "#" . $assoc['inversedBy'] . " which is not defined as association."; |
||
| 139 | } |
||
| 140 | |||
| 141 | 37 | if (!$targetMetadata->hasAssociation($assoc['inversedBy'])) { |
|
| 142 | $ce[] = "The association " . $class->name . "#" . $fieldName . " refers to the inverse side ". |
||
| 143 | "field " . $assoc['targetEntity'] . "#" . $assoc['inversedBy'] . " which does not exist."; |
||
| 144 | 37 | } elseif ($targetMetadata->associationMappings[$assoc['inversedBy']]['mappedBy'] == null) { |
|
| 145 | $ce[] = "The field " . $class->name . "#" . $fieldName . " is on the owning side of a ". |
||
| 146 | "bi-directional relationship, but the specified mappedBy association on the target-entity ". |
||
| 147 | $assoc['targetEntity'] . "#" . $assoc['mappedBy'] . " does not contain the required ". |
||
| 148 | "'inversedBy' attribute."; |
||
| 149 | 37 | } elseif ($targetMetadata->associationMappings[$assoc['inversedBy']]['mappedBy'] != $fieldName) { |
|
| 150 | $ce[] = "The mappings " . $class->name . "#" . $fieldName . " and " . |
||
| 151 | $assoc['targetEntity'] . "#" . $assoc['inversedBy'] . " are ". |
||
| 152 | "inconsistent with each other."; |
||
| 153 | } |
||
| 154 | |||
| 155 | // Verify inverse side/owning side match each other |
||
| 156 | 37 | if (array_key_exists($assoc['inversedBy'], $targetMetadata->associationMappings)) { |
|
| 157 | 37 | $targetAssoc = $targetMetadata->associationMappings[$assoc['inversedBy']]; |
|
| 158 | 37 | if ($assoc['type'] == ClassMetadataInfo::ONE_TO_ONE && $targetAssoc['type'] !== ClassMetadataInfo::ONE_TO_ONE) { |
|
| 159 | $ce[] = "If association " . $class->name . "#" . $fieldName . " is one-to-one, then the inversed " . |
||
| 160 | "side " . $targetMetadata->name . "#" . $assoc['inversedBy'] . " has to be one-to-one as well."; |
||
| 161 | 37 | } elseif ($assoc['type'] == ClassMetadataInfo::MANY_TO_ONE && $targetAssoc['type'] !== ClassMetadataInfo::ONE_TO_MANY) { |
|
| 162 | $ce[] = "If association " . $class->name . "#" . $fieldName . " is many-to-one, then the inversed " . |
||
| 163 | "side " . $targetMetadata->name . "#" . $assoc['inversedBy'] . " has to be one-to-many."; |
||
| 164 | 37 | } elseif ($assoc['type'] == ClassMetadataInfo::MANY_TO_MANY && $targetAssoc['type'] !== ClassMetadataInfo::MANY_TO_MANY) { |
|
| 165 | $ce[] = "If association " . $class->name . "#" . $fieldName . " is many-to-many, then the inversed " . |
||
| 166 | "side " . $targetMetadata->name . "#" . $assoc['inversedBy'] . " has to be many-to-many as well."; |
||
| 167 | } |
||
| 168 | } |
||
| 169 | } |
||
| 170 | |||
| 171 | 46 | if ($assoc['isOwningSide']) { |
|
| 172 | 42 | if ($assoc['type'] == ClassMetadataInfo::MANY_TO_MANY) { |
|
| 173 | 22 | $identifierColumns = $class->getIdentifierColumnNames(); |
|
| 174 | 22 | foreach ($assoc['joinTable']['joinColumns'] as $joinColumn) { |
|
| 175 | 22 | if (!in_array($joinColumn['referencedColumnName'], $identifierColumns)) { |
|
| 176 | $ce[] = "The referenced column name '" . $joinColumn['referencedColumnName'] . "' " . |
||
| 177 | "has to be a primary key column on the target entity class '".$class->name."'."; |
||
| 178 | 22 | break; |
|
| 179 | } |
||
| 180 | } |
||
| 181 | |||
| 182 | 22 | $identifierColumns = $targetMetadata->getIdentifierColumnNames(); |
|
| 183 | 22 | foreach ($assoc['joinTable']['inverseJoinColumns'] as $inverseJoinColumn) { |
|
| 184 | 22 | if (!in_array($inverseJoinColumn['referencedColumnName'], $identifierColumns)) { |
|
| 185 | $ce[] = "The referenced column name '" . $joinColumn['referencedColumnName'] . "' " . |
||
| 186 | "has to be a primary key column on the target entity class '".$targetMetadata->name."'."; |
||
| 187 | 22 | break; |
|
| 188 | } |
||
| 189 | } |
||
| 190 | |||
| 191 | 22 | if (count($targetMetadata->getIdentifierColumnNames()) != count($assoc['joinTable']['inverseJoinColumns'])) { |
|
| 192 | 1 | $ce[] = "The inverse join columns of the many-to-many table '" . $assoc['joinTable']['name'] . "' " . |
|
| 193 | 1 | "have to contain to ALL identifier columns of the target entity '". $targetMetadata->name . "', " . |
|
| 194 | 1 | "however '" . implode(", ", array_diff($targetMetadata->getIdentifierColumnNames(), array_values($assoc['relationToTargetKeyColumns']))) . |
|
| 195 | 1 | "' are missing."; |
|
| 196 | } |
||
| 197 | |||
| 198 | 22 | if (count($class->getIdentifierColumnNames()) != count($assoc['joinTable']['joinColumns'])) { |
|
| 199 | 1 | $ce[] = "The join columns of the many-to-many table '" . $assoc['joinTable']['name'] . "' " . |
|
| 200 | 1 | "have to contain to ALL identifier columns of the source entity '". $class->name . "', " . |
|
| 201 | 1 | "however '" . implode(", ", array_diff($class->getIdentifierColumnNames(), array_values($assoc['relationToSourceKeyColumns']))) . |
|
| 202 | 22 | "' are missing."; |
|
| 203 | } |
||
| 204 | |||
| 205 | 38 | } elseif ($assoc['type'] & ClassMetadataInfo::TO_ONE) { |
|
| 206 | 38 | $identifierColumns = $targetMetadata->getIdentifierColumnNames(); |
|
| 207 | 38 | foreach ($assoc['joinColumns'] as $joinColumn) { |
|
| 208 | 38 | if (!in_array($joinColumn['referencedColumnName'], $identifierColumns)) { |
|
| 209 | 2 | $ce[] = "The referenced column name '" . $joinColumn['referencedColumnName'] . "' " . |
|
| 210 | 38 | "has to be a primary key column on the target entity class '".$targetMetadata->name."'."; |
|
| 211 | } |
||
| 212 | } |
||
| 213 | |||
| 214 | 38 | if (count($identifierColumns) != count($assoc['joinColumns'])) { |
|
| 215 | 1 | $ids = []; |
|
| 216 | |||
| 217 | 1 | foreach ($assoc['joinColumns'] as $joinColumn) { |
|
| 218 | 1 | $ids[] = $joinColumn['name']; |
|
| 219 | } |
||
| 220 | |||
| 221 | 1 | $ce[] = "The join columns of the association '" . $assoc['fieldName'] . "' " . |
|
| 222 | 1 | "have to match to ALL identifier columns of the target entity '". $targetMetadata->name . "', " . |
|
| 223 | 1 | "however '" . implode(", ", array_diff($targetMetadata->getIdentifierColumnNames(), $ids)) . |
|
| 224 | 1 | "' are missing."; |
|
| 225 | } |
||
| 226 | } |
||
| 227 | } |
||
| 228 | |||
| 229 | 46 | if (isset($assoc['orderBy']) && $assoc['orderBy'] !== null) { |
|
| 230 | 11 | foreach ($assoc['orderBy'] as $orderField => $orientation) { |
|
| 231 | 11 | if (!$targetMetadata->hasField($orderField) && !$targetMetadata->hasAssociation($orderField)) { |
|
| 232 | 1 | $ce[] = "The association " . $class->name."#".$fieldName." is ordered by a foreign field " . |
|
| 233 | 1 | $orderField . " that is not a field on the target entity " . $targetMetadata->name . "."; |
|
| 234 | 1 | continue; |
|
| 235 | } |
||
| 236 | 11 | if ($targetMetadata->isCollectionValuedAssociation($orderField)) { |
|
| 237 | 1 | $ce[] = "The association " . $class->name."#".$fieldName." is ordered by a field " . |
|
| 238 | 1 | $orderField . " on " . $targetMetadata->name . " that is a collection-valued association."; |
|
| 239 | 1 | continue; |
|
| 240 | } |
||
| 241 | 11 | if ($targetMetadata->isAssociationInverseSide($orderField)) { |
|
| 242 | 1 | $ce[] = "The association " . $class->name."#".$fieldName." is ordered by a field " . |
|
| 243 | 1 | $orderField . " on " . $targetMetadata->name . " that is the inverse side of an association."; |
|
| 244 | 46 | continue; |
|
| 245 | } |
||
| 246 | } |
||
| 247 | } |
||
| 248 | } |
||
| 249 | |||
| 250 | 49 | foreach ($class->subClasses as $subClass) { |
|
| 251 | 13 | if (!in_array($class->name, class_parents($subClass))) { |
|
| 252 | $ce[] = "According to the discriminator map class '" . $subClass . "' has to be a child ". |
||
| 253 | 13 | "of '" . $class->name . "' but these entities are not related through inheritance."; |
|
| 254 | } |
||
| 255 | } |
||
| 256 | |||
| 257 | 49 | return $ce; |
|
| 258 | } |
||
| 259 | |||
| 260 | /** |
||
| 261 | * Checks if the Database Schema is in sync with the current metadata state. |
||
| 262 | * |
||
| 263 | * @return bool |
||
| 264 | */ |
||
| 265 | public function schemaInSyncWithMetadata() |
||
| 273 | } |
||
| 274 |
If you access a property on an interface, you most likely code against a concrete implementation of the interface.
Available Fixes
Adding an additional type check:
Changing the type hint: