Home Modules Customer Magento_Customer Anti-Patterns
Anti-Patterns

Magento_Customer Anti-Patterns

Magento_Customer Anti-Patterns

Magento 2.4.7+ Magento_Customer

Magento_Customer Anti-Patterns

Overview

This document catalogs common anti-patterns, mistakes, and bad practices when working with the Magento_Customer module. Understanding these anti-patterns helps developers write upgrade-safe, performant, and secure customer-related code.

Each anti-pattern includes an explanation of why it's problematic, real-world consequences, and the correct approach with code examples.

Target Versions: Magento 2.4.7+ / Adobe Commerce 2.4.7+ with PHP 8.2+


Table of Contents

  1. Direct Model Usage Instead of Service Contracts
  2. Session Misuse in Service Contracts
  3. Password Handling Mistakes
  4. Bypassing Customer Validation
  5. Improper Exception Handling
  6. Collection Performance Anti-Patterns
  7. Session Data Pollution
  8. Ignoring Customer Registry
  9. Insecure PII Logging
  10. Email Enumeration Vulnerabilities
  11. Hardcoded Customer Group IDs
  12. Improper Address Validation

Direct Model Usage Instead of Service Contracts

Anti-Pattern

Using Magento\Customer\Model\Customer and Magento\Customer\Model\CustomerFactory directly instead of service contracts.

Why It's Wrong

  • Breaks API Stability: Direct model access bypasses service contract guarantees
  • Skips Validation: Service contracts enforce business rules and validation
  • Missing Events: Direct model save doesn't dispatch all necessary events
  • Plugin Bypassing: Plugins on repositories won't execute
  • Upgrade Risk: Model implementations may change; service contracts won't

Bad Example

<?php
declare(strict_types=1);

namespace Vendor\Module\Model;

use Magento\Customer\Model\CustomerFactory;
use Magento\Customer\Model\ResourceModel\Customer as CustomerResource;

class CustomerService
{
    public function __construct(
        private readonly CustomerFactory $customerFactory,
        private readonly CustomerResource $customerResource
    ) {}

    /**
     * ANTI-PATTERN: Direct model usage
     */
    public function createCustomer(array $data): void
    {
        $customer = $this->customerFactory->create();
        $customer->setEmail($data['email']);
        $customer->setFirstname($data['firstname']);
        $customer->setLastname($data['lastname']);
        $customer->setWebsiteId($data['website_id']);

        // PROBLEM: Bypasses service contract validation and events
        $this->customerResource->save($customer);
    }

    /**
     * ANTI-PATTERN: Direct model load
     */
    public function getCustomer(int $customerId)
    {
        $customer = $this->customerFactory->create();
        // PROBLEM: Bypasses CustomerRegistry cache
        $this->customerResource->load($customer, $customerId);
        return $customer;
    }
}

Correct Approach

<?php
declare(strict_types=1);

namespace Vendor\Module\Service;

use Magento\Customer\Api\CustomerRepositoryInterface;
use Magento\Customer\Api\Data\CustomerInterfaceFactory;
use Magento\Framework\Encryption\EncryptorInterface;
use Magento\Framework\Exception\LocalizedException;

class CustomerService
{
    public function __construct(
        private readonly CustomerRepositoryInterface $customerRepository,
        private readonly CustomerInterfaceFactory $customerFactory,
        private readonly EncryptorInterface $encryptor
    ) {}

    /**
     * CORRECT: Use service contract
     */
    public function createCustomer(array $data, string $password): \Magento\Customer\Api\Data\CustomerInterface
    {
        $customer = $this->customerFactory->create();
        $customer->setEmail($data['email'])
            ->setFirstname($data['firstname'])
            ->setLastname($data['lastname'])
            ->setWebsiteId((int)$data['website_id']);

        // Triggers validation, events, and plugins
        return $this->customerRepository->save(
            $customer,
            $this->encryptor->getHash($password, true)
        );
    }

    /**
     * CORRECT: Use repository
     */
    public function getCustomer(int $customerId): \Magento\Customer\Api\Data\CustomerInterface
    {
        // Uses CustomerRegistry cache, triggers plugins
        return $this->customerRepository->getById($customerId);
    }
}

Consequences of Anti-Pattern

  • Skipped validation allows invalid data in database
  • Missing events prevent integrations from working
  • Performance degradation from bypassing registry cache
  • Upgrade failures when model internals change

Session Misuse in Service Contracts

Anti-Pattern

Injecting and using Magento\Customer\Model\Session in service contracts, repositories, or API classes.

Why It's Wrong

  • Breaks API Contracts: REST/GraphQL APIs don't have sessions
  • Area Coupling: Service contracts must work in all areas (cron, API, CLI)
  • Testability: Session dependency makes unit testing difficult
  • State Management: Creates hidden state dependencies

Bad Example

<?php
declare(strict_types=1);

namespace Vendor\Module\Model;

use Magento\Customer\Model\Session;
use Magento\Customer\Api\CustomerRepositoryInterface;

class OrderService
{
    public function __construct(
        private readonly Session $customerSession,
        private readonly CustomerRepositoryInterface $customerRepository
    ) {}

    /**
     * ANTI-PATTERN: Session in service contract
     */
    public function createOrder(array $orderData): void
    {
        // PROBLEM: Breaks in API/cron contexts
        if (!$this->customerSession->isLoggedIn()) {
            throw new \Exception('Customer not logged in');
        }

        $customerId = $this->customerSession->getCustomerId();
        $customer = $this->customerRepository->getById($customerId);

        // Create order...
    }
}

Correct Approach

<?php
declare(strict_types=1);

namespace Vendor\Module\Service;

use Magento\Customer\Api\CustomerRepositoryInterface;
use Magento\Framework\Exception\NoSuchEntityException;

class OrderService
{
    public function __construct(
        private readonly CustomerRepositoryInterface $customerRepository
    ) {}

    /**
     * CORRECT: Accept customer ID as parameter
     */
    public function createOrder(int $customerId, array $orderData): void
    {
        try {
            $customer = $this->customerRepository->getById($customerId);
        } catch (NoSuchEntityException) {
            throw new \InvalidArgumentException('Customer not found');
        }

        // Create order...
    }
}

Controller Layer (where session is appropriate):

<?php
declare(strict_types=1);

namespace Vendor\Module\Controller\Order;

use Magento\Customer\Model\Session;
use Magento\Framework\App\Action\HttpPostActionInterface;
use Magento\Framework\Controller\ResultFactory;
use Vendor\Module\Service\OrderService;

class Create implements HttpPostActionInterface
{
    public function __construct(
        private readonly Session $customerSession,
        private readonly OrderService $orderService,
        private readonly ResultFactory $resultFactory
    ) {}

    public function execute()
    {
        // Session usage appropriate in controller
        if (!$this->customerSession->isLoggedIn()) {
            return $this->resultFactory->create(ResultFactory::TYPE_REDIRECT)
                ->setPath('customer/account/login');
        }

        $customerId = (int)$this->customerSession->getCustomerId();
        $this->orderService->createOrder($customerId, $orderData);

        // Return response...
    }
}

Password Handling Mistakes

Anti-Pattern

Storing, logging, or transmitting plaintext passwords; using weak hashing; storing passwords in custom attributes.

Why It's Wrong

  • Security Breach: Plaintext passwords expose customer accounts
  • Compliance Violation: Violates PCI DSS, GDPR, and other regulations
  • Credential Stuffing: Attackers can use exposed passwords on other sites
  • Audit Failures: Security audits will flag password mishandling

Bad Examples

<?php
// ANTI-PATTERN 1: Logging plaintext password
$logger->info('Customer registered', [
    'email' => $email,
    'password' => $password  // NEVER DO THIS
]);

// ANTI-PATTERN 2: Storing plaintext password
$customer->setData('temp_password', $password);  // NEVER DO THIS

// ANTI-PATTERN 3: Using weak hashing
$passwordHash = md5($password);  // NEVER DO THIS

// ANTI-PATTERN 4: Custom password storage
$customer->setCustomAttribute('backup_password', $password);  // NEVER DO THIS

// ANTI-PATTERN 5: Passing unhashed password to model
$customer->setPassword($password);  // Wrong - needs hash
$customer->save();

Correct Approach

<?php
declare(strict_types=1);

namespace Vendor\Module\Service;

use Magento\Customer\Api\CustomerRepositoryInterface;
use Magento\Customer\Api\Data\CustomerInterface;
use Magento\Framework\Encryption\EncryptorInterface;

class CustomerPasswordService
{
    public function __construct(
        private readonly CustomerRepositoryInterface $customerRepository,
        private readonly EncryptorInterface $encryptor
    ) {}

    /**
     * CORRECT: Hash password before storage
     */
    public function createCustomerWithPassword(
        CustomerInterface $customer,
        string $password
    ): CustomerInterface {
        // Hash using Argon2id (PHP 8.2+) or SHA-256 with salt
        $passwordHash = $this->encryptor->getHash($password, true);

        // Repository handles secure storage
        return $this->customerRepository->save($customer, $passwordHash);
    }

    /**
     * CORRECT: Never log passwords
     */
    public function logCustomerCreation(CustomerInterface $customer): void
    {
        $this->logger->info('Customer created', [
            'customer_id' => $customer->getId(),
            'email' => $customer->getEmail()
            // Password never logged
        ]);
    }
}

Password Change (Correct):

<?php
declare(strict_types=1);

use Magento\Customer\Api\AccountManagementInterface;

class PasswordChangeService
{
    public function __construct(
        private readonly AccountManagementInterface $accountManagement
    ) {}

    public function changePassword(
        string $email,
        string $currentPassword,
        string $newPassword
    ): bool {
        // Service contract handles hashing internally
        return $this->accountManagement->changePassword(
            $email,
            $currentPassword,
            $newPassword
        );
    }
}

Bypassing Customer Validation

Anti-Pattern

Skipping validation by directly saving models or using resource models.

Bad Example

<?php
// ANTI-PATTERN: Skip email validation
$customer->setEmail('invalid-email');
$customerResource->save($customer);  // Saves invalid email

// ANTI-PATTERN: Skip password requirements
$customer->setPasswordHash('weak');  // No strength validation
$customerResource->save($customer);

Correct Approach

<?php
declare(strict_types=1);

use Magento\Customer\Api\AccountManagementInterface;
use Magento\Customer\Api\Data\CustomerInterface;
use Magento\Framework\Exception\InputException;

class CustomerValidator
{
    public function __construct(
        private readonly AccountManagementInterface $accountManagement
    ) {}

    /**
     * CORRECT: Use service contract validation
     */
    public function validateAndCreate(
        CustomerInterface $customer,
        string $password
    ): CustomerInterface {
        // Validates email format, uniqueness, password strength
        $validationResults = $this->accountManagement->validate($customer);

        if (!$validationResults->isValid()) {
            $messages = [];
            foreach ($validationResults->getMessages() as $message) {
                $messages[] = $message->getMessage();
            }
            throw new InputException(__('Validation failed: %1', implode(', ', $messages)));
        }

        return $this->accountManagement->createAccount($customer, $password);
    }
}

Improper Exception Handling

Anti-Pattern

Catching all exceptions, swallowing exceptions, or not handling specific customer exceptions.

Bad Example

<?php
// ANTI-PATTERN 1: Catch all exceptions
try {
    $customer = $customerRepository->get($email);
} catch (\Exception $e) {
    // PROBLEM: Catches too broadly
    return null;
}

// ANTI-PATTERN 2: Swallow exceptions
try {
    $customerRepository->save($customer);
} catch (\Exception $e) {
    // PROBLEM: Silent failure
}

// ANTI-PATTERN 3: Generic error messages
try {
    $accountManagement->authenticate($email, $password);
} catch (\Exception $e) {
    throw new \Exception('Login failed');  // PROBLEM: Loses specific error
}

Correct Approach

<?php
declare(strict_types=1);

use Magento\Customer\Api\CustomerRepositoryInterface;
use Magento\Customer\Api\AccountManagementInterface;
use Magento\Framework\Exception\NoSuchEntityException;
use Magento\Framework\Exception\InvalidEmailOrPasswordException;
use Magento\Framework\Exception\State\UserLockedException;
use Psr\Log\LoggerInterface;

class CustomerAuthService
{
    public function __construct(
        private readonly CustomerRepositoryInterface $customerRepository,
        private readonly AccountManagementInterface $accountManagement,
        private readonly LoggerInterface $logger
    ) {}

    /**
     * CORRECT: Handle specific exceptions
     */
    public function getCustomerByEmail(string $email, int $websiteId): ?\Magento\Customer\Api\Data\CustomerInterface
    {
        try {
            return $this->customerRepository->get($email, $websiteId);
        } catch (NoSuchEntityException) {
            // Expected exception for non-existent customer
            return null;
        } catch (\Exception $e) {
            // Unexpected exceptions still logged
            $this->logger->error('Error loading customer', [
                'email' => $email,
                'error' => $e->getMessage()
            ]);
            throw $e;
        }
    }

    /**
     * CORRECT: Differentiate authentication failures
     */
    public function authenticate(string $email, string $password): ?\Magento\Customer\Api\Data\CustomerInterface
    {
        try {
            return $this->accountManagement->authenticate($email, $password);
        } catch (InvalidEmailOrPasswordException) {
            $this->logger->info('Failed login attempt', ['email' => $email]);
            return null;
        } catch (UserLockedException $e) {
            $this->logger->warning('Account locked', ['email' => $email]);
            throw new \Magento\Framework\Exception\LocalizedException(
                __('Account temporarily locked. Please try again later.')
            );
        }
    }
}

Collection Performance Anti-Patterns

Anti-Pattern

Loading entire customer collections, not using pagination, loading unnecessary attributes, post-collection filtering.

Bad Examples

<?php
// ANTI-PATTERN 1: Load all customers
$collection = $customerCollectionFactory->create();
$allCustomers = $collection->getItems();  // PROBLEM: Can load millions of records

// ANTI-PATTERN 2: Load unnecessary attributes
$collection->addAttributeToSelect('*');  // PROBLEM: Loads all EAV attributes

// ANTI-PATTERN 3: Post-collection filtering
$collection = $customerCollectionFactory->create();
$collection->load();
foreach ($collection as $customer) {
    if ($customer->getEmail() === $searchEmail) {  // PROBLEM: Filtering in PHP, not SQL
        return $customer;
    }
}

// ANTI-PATTERN 4: No pagination
$collection = $customerCollectionFactory->create();
$collection->addFieldToFilter('group_id', 2);
// PROBLEM: No setPageSize() - loads all matching records

Correct Approach

<?php
declare(strict_types=1);

use Magento\Customer\Api\CustomerRepositoryInterface;
use Magento\Framework\Api\SearchCriteriaBuilder;
use Magento\Framework\Api\SortOrderBuilder;
use Magento\Customer\Model\ResourceModel\Customer\CollectionFactory;

class CustomerSearchService
{
    public function __construct(
        private readonly CustomerRepositoryInterface $customerRepository,
        private readonly SearchCriteriaBuilder $searchCriteriaBuilder,
        private readonly SortOrderBuilder $sortOrderBuilder,
        private readonly CollectionFactory $customerCollectionFactory
    ) {}

    /**
     * CORRECT: Use repository with pagination
     */
    public function findCustomersByGroup(int $groupId, int $pageSize = 20, int $page = 1): array
    {
        $sortOrder = $this->sortOrderBuilder
            ->setField('created_at')
            ->setDirection('DESC')
            ->create();

        $searchCriteria = $this->searchCriteriaBuilder
            ->addFilter('group_id', $groupId)
            ->setPageSize($pageSize)
            ->setCurrentPage($page)
            ->addSortOrder($sortOrder)
            ->create();

        $searchResults = $this->customerRepository->getList($searchCriteria);

        return [
            'items' => $searchResults->getItems(),
            'total_count' => $searchResults->getTotalCount()
        ];
    }

    /**
     * CORRECT: Select only needed attributes
     */
    public function getCustomerNamesAndEmails(): array
    {
        $collection = $this->customerCollectionFactory->create();
        $collection->addAttributeToSelect(['firstname', 'lastname', 'email'])
            ->setPageSize(100)
            ->setCurPage(1);

        $results = [];
        foreach ($collection as $customer) {
            $results[] = [
                'name' => $customer->getName(),
                'email' => $customer->getEmail()
            ];
        }

        return $results;
    }

    /**
     * CORRECT: Use SQL filtering
     */
    public function findByEmail(string $email): ?\Magento\Customer\Model\Customer
    {
        $collection = $this->customerCollectionFactory->create();
        $collection->addFieldToFilter('email', $email)
            ->setPageSize(1);

        return $collection->getFirstItem()->getId() ? $collection->getFirstItem() : null;
    }
}

Session Data Pollution

Anti-Pattern

Storing large objects, sensitive data, or temporary data in customer session.

Bad Examples

<?php
// ANTI-PATTERN 1: Store large objects
$customerSession->setData('product_catalog', $largeArray);  // PROBLEM: Session bloat

// ANTI-PATTERN 2: Store sensitive data
$customerSession->setData('credit_card_number', $ccNumber);  // PROBLEM: Security risk

// ANTI-PATTERN 3: Use as cache
$customerSession->setData('api_response_cache', $apiData);  // PROBLEM: Wrong tool

// ANTI-PATTERN 4: Never clean up
$customerSession->setData('temp_checkout_data', $data);
// PROBLEM: No cleanup after use

Correct Approach

<?php
declare(strict_types=1);

use Magento\Customer\Model\Session;
use Magento\Framework\Session\SessionManagerInterface;

class SessionManagementService
{
    public function __construct(
        private readonly Session $customerSession,
        private readonly SessionManagerInterface $sessionManager
    ) {}

    /**
     * CORRECT: Store only essential data
     */
    public function storeCheckoutStep(string $step): void
    {
        // Store minimal data
        $this->customerSession->setData('checkout_step', $step);
    }

    /**
     * CORRECT: Clean up after use
     */
    public function clearCheckoutData(): void
    {
        $this->customerSession->unsData('checkout_step');
        $this->customerSession->unsData('shipping_method');
        $this->customerSession->unsData('payment_method');
    }

    /**
     * CORRECT: Never store sensitive data in session
     * Use secure payment vault instead
     */
    public function storePaymentMethod(string $methodCode): void
    {
        // Only store method code, not card details
        $this->customerSession->setData('payment_method_code', $methodCode);
    }
}

Ignoring Customer Registry

Anti-Pattern

Loading the same customer multiple times in a request instead of using CustomerRegistry.

Bad Example

<?php
// ANTI-PATTERN: Multiple loads
$customer1 = $customerFactory->create()->load($customerId);  // DB query
$customer2 = $customerFactory->create()->load($customerId);  // Redundant DB query
$customer3 = $customerFactory->create()->load($customerId);  // Another redundant query

Correct Approach

<?php
declare(strict_types=1);

use Magento\Customer\Model\CustomerRegistry;
use Magento\Customer\Api\CustomerRepositoryInterface;

class CustomerDataService
{
    public function __construct(
        private readonly CustomerRegistry $customerRegistry,
        private readonly CustomerRepositoryInterface $customerRepository
    ) {}

    /**
     * CORRECT: Registry caches customer
     */
    public function processCustomer(int $customerId): void
    {
        // First call: loads from DB and caches
        $customer1 = $this->customerRegistry->retrieve($customerId);

        // Subsequent calls: returns cached instance
        $customer2 = $this->customerRegistry->retrieve($customerId);
        $customer3 = $this->customerRegistry->retrieve($customerId);

        // All three are same instance, only one DB query
    }

    /**
     * CORRECT: Repository uses registry internally
     */
    public function getCustomerData(int $customerId): array
    {
        // Repository uses registry cache
        $customer = $this->customerRepository->getById($customerId);
        return [
            'name' => $customer->getFirstname() . ' ' . $customer->getLastname(),
            'email' => $customer->getEmail()
        ];
    }
}

Insecure PII Logging

Anti-Pattern

Logging personally identifiable information (PII) in plaintext logs.

Bad Examples

<?php
// ANTI-PATTERN: Log customer PII
$logger->info('Customer updated', [
    'customer_id' => $customer->getId(),
    'email' => $customer->getEmail(),  // PII
    'name' => $customer->getName(),    // PII
    'dob' => $customer->getDob(),      // PII
    'address' => $customer->getDefaultBillingAddress()  // PII
]);

// ANTI-PATTERN: Debug output with PII
var_dump($customer->getData());  // Contains all PII

Correct Approach

<?php
declare(strict_types=1);

use Psr\Log\LoggerInterface;
use Magento\Customer\Api\Data\CustomerInterface;

class SecureCustomerLogger
{
    public function __construct(
        private readonly LoggerInterface $logger
    ) {}

    /**
     * CORRECT: Log without PII
     */
    public function logCustomerUpdate(CustomerInterface $customer): void
    {
        $this->logger->info('Customer updated', [
            'customer_id' => $customer->getId(),
            'website_id' => $customer->getWebsiteId(),
            'group_id' => $customer->getGroupId()
            // No email, name, DOB, or address
        ]);
    }

    /**
     * CORRECT: Hash PII if logging necessary
     */
    public function logAuthenticationAttempt(string $email): void
    {
        $this->logger->info('Authentication attempt', [
            'email_hash' => hash('sha256', strtolower($email))
        ]);
    }

    /**
     * CORRECT: Scrub sensitive data from exceptions
     */
    public function logException(\Exception $e, array $context = []): void
    {
        $sanitizedContext = $this->scrubPii($context);
        $this->logger->error($e->getMessage(), $sanitizedContext);
    }

    private function scrubPii(array $data): array
    {
        $piiFields = ['email', 'firstname', 'lastname', 'dob', 'taxvat', 'telephone'];

        foreach ($piiFields as $field) {
            if (isset($data[$field])) {
                $data[$field] = '[REDACTED]';
            }
        }

        return $data;
    }
}

Email Enumeration Vulnerabilities

Anti-Pattern

Revealing whether an email exists in the system through different error messages or response times.

Bad Example

<?php
// ANTI-PATTERN: Reveals if email exists
public function resetPassword(string $email): array
{
    try {
        $customer = $customerRepository->get($email);
    } catch (NoSuchEntityException) {
        return ['success' => false, 'message' => 'Email not found'];  // PROBLEM: Enumeration
    }

    // Send reset email
    return ['success' => true, 'message' => 'Reset email sent'];
}

Correct Approach

<?php
declare(strict_types=1);

use Magento\Customer\Api\AccountManagementInterface;
use Magento\Framework\Exception\NoSuchEntityException;
use Psr\Log\LoggerInterface;

class PasswordResetService
{
    public function __construct(
        private readonly AccountManagementInterface $accountManagement,
        private readonly LoggerInterface $logger
    ) {}

    /**
     * CORRECT: Same response regardless of email existence
     */
    public function initiatePasswordReset(string $email, int $websiteId): array
    {
        try {
            $this->accountManagement->initiatePasswordReset(
                $email,
                AccountManagementInterface::EMAIL_RESET,
                $websiteId
            );
        } catch (NoSuchEntityException) {
            // Log but don't reveal
            $this->logger->info('Password reset for non-existent email', [
                'email_hash' => hash('sha256', strtolower($email))
            ]);
        }

        // Always return same message (prevents enumeration)
        return [
            'success' => true,
            'message' => 'If the email exists, you will receive a password reset link.'
        ];
    }
}

Hardcoded Customer Group IDs

Anti-Pattern

Using hardcoded customer group IDs instead of configuration or database lookups.

Bad Example

<?php
// ANTI-PATTERN: Hardcoded group IDs
if ($customer->getGroupId() === 2) {  // PROBLEM: What if group 2 doesn't exist?
    $discount = 0.10;
}

// ANTI-PATTERN: Hardcoded group assignment
$customer->setGroupId(3);  // PROBLEM: Group 3 might not be "Wholesale"

Correct Approach

<?php
declare(strict_types=1);

use Magento\Customer\Api\GroupRepositoryInterface;
use Magento\Framework\Api\SearchCriteriaBuilder;

class CustomerGroupService
{
    public function __construct(
        private readonly GroupRepositoryInterface $groupRepository,
        private readonly SearchCriteriaBuilder $searchCriteriaBuilder
    ) {}

    /**
     * CORRECT: Look up group by code
     */
    public function getGroupIdByCode(string $groupCode): ?int
    {
        $searchCriteria = $this->searchCriteriaBuilder
            ->addFilter('customer_group_code', $groupCode)
            ->setPageSize(1)
            ->create();

        $groups = $this->groupRepository->getList($searchCriteria)->getItems();

        if (empty($groups)) {
            return null;
        }

        $group = reset($groups);
        return (int)$group->getId();
    }

    /**
     * CORRECT: Use configuration for discount logic
     */
    public function getDiscountForCustomer(int $customerId): float
    {
        $customer = $this->customerRepository->getById($customerId);
        $groupId = (int)$customer->getGroupId();

        // Use catalog price rules or configuration instead of hardcoding
        return $this->scopeConfig->getValue(
            'customer_discounts/group_' . $groupId . '/discount_percent',
            ScopeInterface::SCOPE_STORE
        ) ?? 0.0;
    }
}

Improper Address Validation

Anti-Pattern

Not validating address data before save, allowing invalid country/region combinations.

Bad Example

<?php
// ANTI-PATTERN: No validation
$address->setCountryId('US');
$address->setRegionId(999);  // PROBLEM: Invalid region for US
$addressRepository->save($address);

// ANTI-PATTERN: No street line validation
$address->setStreet(['Line 1', 'Line 2', 'Line 3', 'Line 4', 'Line 5']);  // PROBLEM: Too many lines

Correct Approach

<?php
declare(strict_types=1);

use Magento\Customer\Api\AddressRepositoryInterface;
use Magento\Customer\Api\Data\AddressInterface;
use Magento\Directory\Model\ResourceModel\Region\CollectionFactory as RegionCollectionFactory;
use Magento\Framework\Exception\LocalizedException;

class AddressValidationService
{
    public function __construct(
        private readonly AddressRepositoryInterface $addressRepository,
        private readonly RegionCollectionFactory $regionCollectionFactory
    ) {}

    /**
     * CORRECT: Validate before save
     */
    public function saveAddress(AddressInterface $address): AddressInterface
    {
        $this->validateAddress($address);
        return $this->addressRepository->save($address);
    }

    private function validateAddress(AddressInterface $address): void
    {
        // Validate required fields
        if (!$address->getCountryId()) {
            throw new LocalizedException(__('Country is required'));
        }

        // Validate region for country
        if ($address->getRegionId()) {
            $this->validateRegion(
                (int)$address->getRegionId(),
                $address->getCountryId()
            );
        }

        // Validate street lines
        $street = $address->getStreet();
        if (is_array($street) && count($street) > 4) {
            throw new LocalizedException(
                __('Address can have maximum 4 street lines')
            );
        }
    }

    private function validateRegion(int $regionId, string $countryId): void
    {
        $regionCollection = $this->regionCollectionFactory->create();
        $regionCollection->addRegionIdFilter($regionId)
            ->addCountryFilter($countryId);

        if ($regionCollection->getSize() === 0) {
            throw new LocalizedException(
                __('Invalid region for selected country')
            );
        }
    }
}

Summary of Anti-Patterns

Anti-Pattern Impact Correct Approach
Direct model usage Bypasses validation, events, plugins Use service contracts
Session in service contracts Breaks API contexts Pass customer ID as parameter
Plaintext passwords Security breach Use EncryptorInterface
Skip validation Data integrity issues Use AccountManagementInterface
Generic exception handling Lost error context Catch specific exceptions
Load entire collections Performance degradation Use pagination and filtering
Session data pollution Memory bloat Store minimal data, clean up
Ignore registry Redundant DB queries Use CustomerRegistry
Log PII Privacy violations Hash or redact PII
Email enumeration Security vulnerability Same response for all emails
Hardcoded group IDs Fragile code Look up by code
No address validation Invalid data Validate before save

Assumptions

  • Target Platform: Adobe Commerce / Magento Open Source 2.4.7+
  • PHP Version: 8.2+
  • Security Standards: PCI DSS, GDPR compliance required
  • Performance Requirements: Production-level optimization

Why This Approach

Following these correct patterns ensures upgrade safety, security, performance, and compliance. Service contracts provide API stability, proper validation prevents data corruption, and secure practices protect customer data.

Security Impact

  • Password Security: Never store plaintext; use Argon2id hashing
  • PII Protection: Redact sensitive data from logs and debug output
  • Email Enumeration: Prevent attackers from discovering valid accounts
  • Session Security: Minimize session data; regenerate after authentication

Performance Impact

  • Repository Pattern: Use registry caching to avoid redundant queries
  • Collection Optimization: Paginate, filter in SQL, load only needed attributes
  • Session Minimization: Reduce session storage size for faster serialization

Backward Compatibility

  • Service Contracts: Stable across minor versions
  • Deprecation: Direct model usage deprecated since 2.3
  • Migration Path: Gradually refactor to repositories

Tests to Add

  • Unit: Test validation logic, exception handling
  • Integration: Test service contracts with various inputs
  • Security: Test password hashing, PII logging prevention

Docs to Update

  • ANTI_PATTERNS.md: This document
  • README.md: Link to anti-patterns
  • Code review checklist: Include anti-pattern checks