Complex classes like ImageImport 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 ImageImport, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 22 | class ImageImport |
||
| 23 | { |
||
| 24 | /** @var \Shopware\Components\Model\ModelManager */ |
||
| 25 | protected $manager; |
||
| 26 | |||
| 27 | /** @var Helper */ |
||
| 28 | protected $helper; |
||
| 29 | |||
| 30 | /** |
||
| 31 | * @var ThumbnailManager |
||
| 32 | */ |
||
| 33 | protected $thumbnailManager; |
||
| 34 | |||
| 35 | /** @var \ShopwarePlugins\Connect\Components\Logger */ |
||
| 36 | protected $logger; |
||
| 37 | |||
| 38 | public function __construct( |
||
| 39 | ModelManager $manager, |
||
| 40 | Helper $helper, |
||
| 41 | ThumbnailManager $thumbnailManager, |
||
| 42 | Logger $logger |
||
| 43 | ) { |
||
| 44 | $this->manager = $manager; |
||
| 45 | $this->helper = $helper; |
||
| 46 | $this->thumbnailManager = $thumbnailManager; |
||
| 47 | $this->logger = $logger; |
||
| 48 | } |
||
| 49 | |||
| 50 | /** |
||
| 51 | * Helper to determine, if there is a main image for a given articleId |
||
| 52 | * |
||
| 53 | * @param $articleId |
||
| 54 | * @return bool |
||
| 55 | */ |
||
| 56 | public function hasArticleMainImage($articleId) |
||
| 73 | |||
| 74 | /** |
||
| 75 | * Get ids of details that needs an image import |
||
| 76 | * @param null $limit |
||
| 77 | * @return array Ids of products needing an image import |
||
| 78 | */ |
||
| 79 | public function getDetailIdsNeedingImageImport($limit=null) |
||
| 80 | { |
||
| 81 | $updateFlags = $this->helper->getUpdateFlags(); |
||
| 82 | $updateFlagsByName = array_flip($updateFlags); |
||
| 83 | |||
| 84 | $initialImportFlag = $updateFlagsByName['imageInitialImport']; |
||
| 85 | |||
| 86 | $builder = $this->manager->createQueryBuilder(); |
||
| 87 | $builder->from('Shopware\CustomModels\Connect\Attribute', 'at'); |
||
| 88 | $builder->select('at.articleDetailId'); |
||
| 89 | $builder->andWhere('at.shopId IS NOT NULL') |
||
| 90 | ->andWhere('at.lastUpdateFlag IS NOT NULL') |
||
| 91 | ->andWhere('BIT_AND(at.lastUpdateFlag, :initialImportFlag) > 0') |
||
| 92 | ->setParameter('initialImportFlag', $initialImportFlag); |
||
| 93 | |||
| 94 | if ($limit) { |
||
| 95 | $builder->setMaxResults($limit); |
||
| 96 | } |
||
| 97 | |||
| 98 | $ids = $builder->getQuery()->getArrayResult(); |
||
| 99 | |||
| 100 | return array_map('array_pop', $ids); |
||
| 101 | } |
||
| 102 | |||
| 103 | /** |
||
| 104 | * Batch import images for new products without images |
||
| 105 | * |
||
| 106 | * @param int|null $limit |
||
| 107 | */ |
||
| 108 | public function import($limit = null) |
||
| 109 | { |
||
| 110 | $detailRepository = $this->manager->getRepository('Shopware\Models\Article\Detail'); |
||
| 111 | |||
| 112 | $flags = $this->helper->getUpdateFlags(); |
||
| 113 | $flagsByName = array_flip($flags); |
||
| 114 | |||
| 115 | $ids = $this->getDetailIdsNeedingImageImport($limit); |
||
|
|
|||
| 116 | |||
| 117 | foreach ($ids as $id) { |
||
| 118 | /** @var \Shopware\Models\Article\Detail $detail */ |
||
| 119 | $detail = $detailRepository->find($id); |
||
| 120 | $connectAttribute = $this->helper->getConnectAttributeByModel($detail); |
||
| 121 | |||
| 122 | $this->manager->getConnection()->beginTransaction(); |
||
| 123 | try { |
||
| 124 | $lastUpdate = json_decode($connectAttribute->getLastUpdate(), true); |
||
| 125 | $this->importArticleImagesWhenMainDetail($detail, $lastUpdate); |
||
| 126 | $this->importImagesForDetail($lastUpdate['variantImages'], $detail); |
||
| 127 | $connectAttribute->flipLastUpdateFlag($flagsByName['imageInitialImport']); |
||
| 128 | $this->manager->flush(); |
||
| 129 | $this->manager->getConnection()->commit(); |
||
| 130 | } catch (\PDOException $e) { |
||
| 131 | // possible lock with ImageImport from ToShop channel |
||
| 132 | // is thrown before flipping of last update flag |
||
| 133 | // so we should process the detail again in next cron run |
||
| 134 | $this->manager->getConnection()->rollback(); |
||
| 135 | } |
||
| 136 | } |
||
| 137 | } |
||
| 138 | |||
| 139 | /** |
||
| 140 | * Handles the image import of a product. This will: |
||
| 141 | * - delete all images imported from connect before and not in the current import list |
||
| 142 | * - create new images which have not already been imported |
||
| 143 | * - set the main image, if there is no main image, yet |
||
| 144 | * |
||
| 145 | * Images are identified via the URL of the connect image. So we don't need to md5 the |
||
| 146 | * actual image content every time. |
||
| 147 | * |
||
| 148 | * @param array $images |
||
| 149 | * @param $article \Shopware\Models\Article\Article |
||
| 150 | */ |
||
| 151 | public function importImagesForArticle($images, Article $article) |
||
| 152 | { |
||
| 153 | $localImagesFromConnect = $this->getImportedImages($article); |
||
| 154 | $remoteImagesFromConnect = array_flip($images); |
||
| 155 | |||
| 156 | // Build up arrays of images to delete and images to create |
||
| 157 | $imagesToDelete = array_diff_key($localImagesFromConnect, $remoteImagesFromConnect); |
||
| 158 | $imagesToCreate = array_diff_key($remoteImagesFromConnect, $localImagesFromConnect); |
||
| 159 | |||
| 160 | // Delete old connect images and media objects |
||
| 161 | foreach ($imagesToDelete as $hash => $data) { |
||
| 162 | /** @var \Shopware\Models\Article\Image $image */ |
||
| 163 | $image = $data['image']; |
||
| 164 | // if the image has mapping, it's variant image and shouldn't be deleted |
||
| 165 | if (count($image->getMappings()) > 0) { |
||
| 166 | continue; |
||
| 167 | } |
||
| 168 | $this->manager->remove($data['media']); |
||
| 169 | $this->manager->remove($image); |
||
| 170 | } |
||
| 171 | $this->manager->flush(); |
||
| 172 | |||
| 173 | try { |
||
| 174 | $this->importImages($imagesToCreate, $article); |
||
| 175 | } catch (\Exception $e) { |
||
| 176 | // log exception message if for some reason |
||
| 177 | // image import fails |
||
| 178 | $this->logger->write(true, 'Import images', $e->getMessage()); |
||
| 179 | } |
||
| 180 | |||
| 181 | $this->manager->flush(); |
||
| 182 | } |
||
| 183 | |||
| 184 | /** |
||
| 185 | * Handles the image import of a product. This will: |
||
| 186 | * - delete all images imported from connect before and not in the current import list |
||
| 187 | * - create new images which have not already been imported |
||
| 188 | * - set the main image, if there is no main image, yet |
||
| 189 | * |
||
| 190 | * Images are identified via the URL of the connect image. So we don't need to md5 the |
||
| 191 | * actual image content every time. |
||
| 192 | * |
||
| 193 | * @param array $variantImages |
||
| 194 | * @param $detail \Shopware\Models\Article\Detail |
||
| 195 | */ |
||
| 196 | public function importImagesForDetail(array $variantImages, Detail $detail) |
||
| 197 | { |
||
| 198 | $article = $detail->getArticle(); |
||
| 199 | $articleImages = $article->getImages(); |
||
| 200 | |||
| 201 | $localImagesFromConnect = $this->getImportedImages($detail); |
||
| 202 | $localArticleImagesFromConnect = $this->getImportedImages($article); |
||
| 203 | |||
| 204 | $remoteImagesFromConnect = array_flip($variantImages); |
||
| 205 | |||
| 206 | // Build up arrays of images to delete and images to create |
||
| 207 | $imagesToDelete = array_diff_key($localImagesFromConnect, $remoteImagesFromConnect); |
||
| 208 | $imagesToCreate = array_diff_key($remoteImagesFromConnect, $localImagesFromConnect); |
||
| 209 | |||
| 210 | $mappingRepository = $this->manager->getRepository('Shopware\Models\Article\Image\Mapping'); |
||
| 211 | // Delete old connect images and media objects |
||
| 212 | foreach ($imagesToDelete as $hash => $data) { |
||
| 213 | /** @var \Shopware\Models\Article\Image $image */ |
||
| 214 | $image = $data['image']; |
||
| 215 | /** @var \Shopware\Models\Article\Image $child */ |
||
| 216 | foreach ($image->getChildren() as $child) { |
||
| 217 | if ($detail->getId() == $child->getArticleDetail()->getId()) { |
||
| 218 | $childAttribute = $child->getAttribute(); |
||
| 219 | if (!$childAttribute) { |
||
| 220 | break; |
||
| 221 | } |
||
| 222 | |||
| 223 | $mapping = $mappingRepository->find($childAttribute->getConnectDetailMappingId()); |
||
| 224 | if (!$mapping) { |
||
| 225 | break; |
||
| 226 | } |
||
| 227 | |||
| 228 | $this->manager->remove($mapping); |
||
| 229 | $this->manager->remove($child); |
||
| 230 | break; |
||
| 231 | } |
||
| 232 | } |
||
| 233 | |||
| 234 | if (count($image->getChildren()) == 1) { |
||
| 235 | $this->manager->remove($image); |
||
| 236 | $this->manager->remove($data['media']); |
||
| 237 | } |
||
| 238 | } |
||
| 239 | $this->manager->flush(); |
||
| 240 | |||
| 241 | try { |
||
| 242 | $positions = []; |
||
| 243 | foreach ($article->getImages() as $image) { |
||
| 244 | $positions[] = $image->getPosition(); |
||
| 245 | } |
||
| 246 | $maxPosition = count($positions) > 0 ? max($positions) : 0; |
||
| 247 | |||
| 248 | /** @var \Shopware\Models\Media\Album $album */ |
||
| 249 | $album = $this->manager->find('Shopware\Models\Media\Album', -1); |
||
| 250 | foreach ($imagesToCreate as $imageUrl => $key) { |
||
| 251 | // check if image already exists in article images |
||
| 252 | // 1) if it exists skip import and it's global image |
||
| 253 | if (array_key_exists($imageUrl, $localArticleImagesFromConnect) |
||
| 254 | && empty($localArticleImagesFromConnect[$imageUrl]['image']->getMappings())) { |
||
| 255 | // if image doesn't have mappings |
||
| 256 | // it's global for all details |
||
| 257 | // do nothing, just continue |
||
| 258 | continue; |
||
| 259 | } |
||
| 260 | |||
| 261 | // 2) if it has mapping, add new one for current detail |
||
| 262 | if (array_key_exists($imageUrl, $localArticleImagesFromConnect)) { |
||
| 263 | /** @var \Shopware\Models\Article\Image $articleImage */ |
||
| 264 | $articleImage = $localArticleImagesFromConnect[$imageUrl]['image']; |
||
| 265 | $articleMedia = $localArticleImagesFromConnect[$imageUrl]['media']; |
||
| 266 | |||
| 267 | // add new mapping |
||
| 268 | $mapping = new Image\Mapping(); |
||
| 269 | $mapping->setImage($articleImage); |
||
| 270 | foreach ($detail->getConfiguratorOptions() as $option) { |
||
| 271 | $rule = new Image\Rule(); |
||
| 272 | $rule->setMapping($mapping); |
||
| 273 | $rule->setOption($option); |
||
| 274 | $mapping->getRules()->add($rule); |
||
| 275 | } |
||
| 276 | $this->manager->persist($mapping); |
||
| 277 | // mapping should have id, because it should be stored as child image attribute |
||
| 278 | $this->manager->flush($mapping); |
||
| 279 | $articleImage->getMappings()->add($mapping); |
||
| 280 | |||
| 281 | // add child image |
||
| 282 | $childImage = new Image(); |
||
| 283 | $childImage->setMain(2); |
||
| 284 | $childImage->setPosition($maxPosition + $key + 1); |
||
| 285 | $childImage->setParent($articleImage); |
||
| 286 | $childImage->setArticleDetail($detail); |
||
| 287 | $childImage->setExtension($articleMedia->getExtension()); |
||
| 288 | $childImageAttribute = $childImage->getAttribute() ?: new ImageAttribute(); |
||
| 289 | $childImageAttribute->setArticleImage($childImage); |
||
| 290 | $childImageAttribute->setConnectDetailMappingId($mapping->getId()); |
||
| 291 | |||
| 292 | $detail->getImages()->add($childImage); |
||
| 293 | $articleImage->getChildren()->add($childImage); |
||
| 294 | |||
| 295 | $this->manager->persist($childImage); |
||
| 296 | $this->manager->persist($childImageAttribute); |
||
| 297 | $this->manager->persist($articleImage); |
||
| 298 | |||
| 299 | continue; |
||
| 300 | } |
||
| 301 | |||
| 302 | // 3) if it doesn't exist, import it |
||
| 303 | $importedImages = $this->importImages([$imageUrl => $key], $article, $maxPosition); |
||
| 304 | $image = reset($importedImages); |
||
| 305 | $media = $image->getMedia(); |
||
| 306 | |||
| 307 | // add new mapping |
||
| 308 | $mapping = new Image\Mapping(); |
||
| 309 | $mapping->setImage($image); |
||
| 310 | foreach ($detail->getConfiguratorOptions() as $option) { |
||
| 311 | $rule = new Image\Rule(); |
||
| 312 | $rule->setMapping($mapping); |
||
| 313 | $rule->setOption($option); |
||
| 314 | $rules = $mapping->getRules(); |
||
| 315 | $rules->add($rule); |
||
| 316 | $mapping->setRules($rules); |
||
| 317 | $this->manager->persist($rule); |
||
| 318 | } |
||
| 319 | $this->manager->persist($mapping); |
||
| 320 | // mapping should have id, because it should be stored as child image attribute |
||
| 321 | $this->manager->flush($mapping); |
||
| 322 | |||
| 323 | $mappings = $image->getMappings(); |
||
| 324 | $mappings->add($mapping); |
||
| 325 | $image->setMappings($mappings); |
||
| 326 | |||
| 327 | // add child image |
||
| 328 | $childImage = new Image(); |
||
| 329 | $childImage->setMain(2); |
||
| 330 | $childImage->setPosition($maxPosition + $key + 1); |
||
| 331 | $childImage->setParent($image); |
||
| 332 | $childImage->setArticleDetail($detail); |
||
| 333 | $childImage->setExtension($media->getExtension()); |
||
| 334 | $childImageAttribute = $childImage->getAttribute() ?: new ImageAttribute(); |
||
| 335 | $childImageAttribute->setArticleImage($childImage); |
||
| 336 | $childImageAttribute->setConnectDetailMappingId($mapping->getId()); |
||
| 337 | $detail->getImages()->add($childImage); |
||
| 338 | |||
| 339 | $image->getChildren()->add($childImage); |
||
| 340 | |||
| 341 | $this->manager->persist($childImage); |
||
| 342 | $this->manager->persist($childImageAttribute); |
||
| 343 | $this->manager->persist($image); |
||
| 344 | |||
| 345 | $this->thumbnailManager->createMediaThumbnail( |
||
| 346 | $media, |
||
| 347 | $this->getThumbnailSize($album), |
||
| 348 | true |
||
| 349 | ); |
||
| 350 | } |
||
| 351 | } catch (\Exception $e) { |
||
| 352 | // log exception message if for some reason |
||
| 353 | // image import fails |
||
| 354 | $this->logger->write(true, 'Import images', $e->getMessage()); |
||
| 355 | } |
||
| 356 | |||
| 357 | $article->setImages($articleImages); |
||
| 358 | $this->manager->persist($article); |
||
| 359 | $this->manager->flush(); |
||
| 360 | } |
||
| 361 | |||
| 362 | /** |
||
| 363 | * Helper: Read images for a given detail |
||
| 364 | * |
||
| 365 | * @param int $articleDetailId |
||
| 366 | * @return array |
||
| 367 | */ |
||
| 368 | public function getImagesForDetail($articleDetailId) |
||
| 369 | { |
||
| 370 | $builder = $this->manager->createQueryBuilder(); |
||
| 371 | $builder->select('media.path') |
||
| 372 | ->from('Shopware\Models\Article\Image', 'images') |
||
| 373 | ->join('images.media', 'media') |
||
| 374 | ->where('images.articleDetailId = :articleDetailId') |
||
| 375 | ->andWhere('images.parentId IS NULL') |
||
| 376 | ->setParameter('articleDetailId', $articleDetailId) |
||
| 377 | ->orderBy('images.main', 'ASC') |
||
| 378 | ->addOrderBy('images.position', 'ASC'); |
||
| 379 | |||
| 380 | return array_map( |
||
| 381 | function ($image) { |
||
| 382 | return $image['path']; |
||
| 383 | }, |
||
| 384 | $builder->getQuery()->getArrayResult() |
||
| 385 | ); |
||
| 386 | } |
||
| 387 | |||
| 388 | /** |
||
| 389 | * Download, import and assign images to article |
||
| 390 | * |
||
| 391 | * @param array $imagesToCreate |
||
| 392 | * @param Article|Detail $model |
||
| 393 | * @param null|int $maxPosition |
||
| 394 | * @throws \Doctrine\ORM\ORMException |
||
| 395 | * @throws \Doctrine\ORM\OptimisticLockException |
||
| 396 | * @throws \Doctrine\ORM\TransactionRequiredException |
||
| 397 | * @throws \Exception |
||
| 398 | * @return \Shopware\Models\Article\Image |
||
| 399 | */ |
||
| 400 | private function importImages(array $imagesToCreate, $model, $maxPosition = null) |
||
| 468 | |||
| 469 | /** |
||
| 470 | * Returns a list with already imported images from Connect |
||
| 471 | * by given Article or Detail |
||
| 472 | * |
||
| 473 | * @param Article|Detail $model |
||
| 474 | * @return array |
||
| 475 | */ |
||
| 476 | private function getImportedImages($model) |
||
| 511 | |||
| 512 | /** |
||
| 513 | * @param $imageUrl |
||
| 514 | * @param Supplier $supplier |
||
| 515 | */ |
||
| 516 | public function importImageForSupplier($imageUrl, Supplier $supplier) |
||
| 553 | |||
| 554 | /** |
||
| 555 | * Returns thumbnails size by album |
||
| 556 | * @param $album \Shopware\Models\Media\Album |
||
| 557 | * @return array |
||
| 558 | */ |
||
| 559 | protected function getThumbnailSize($album) |
||
| 586 | |||
| 587 | /** |
||
| 588 | * @param $imageUrl string |
||
| 589 | * @param $articleId int |
||
| 590 | */ |
||
| 591 | public function importMainImage($imageUrl, $articleId) |
||
| 621 | |||
| 622 | /** |
||
| 623 | * @param $imageUrl string |
||
| 624 | * @param $articleId int |
||
| 625 | * @return bool |
||
| 626 | */ |
||
| 627 | public function hasMainImageChanged($imageUrl, $articleId) |
||
| 641 | |||
| 642 | /** |
||
| 643 | * @param Detail $detail |
||
| 644 | * @param array $lastUpdate |
||
| 645 | */ |
||
| 646 | private function importArticleImagesWhenMainDetail($detail, array $lastUpdate) |
||
| 652 | } |
||
| 653 |
This check looks at variables that have been passed in as parameters and are passed out again to other methods.
If the outgoing method call has stricter type requirements than the method itself, an issue is raised.
An additional type check may prevent trouble.