Direct SQL queries

A common mistake developers make when they first start working with Magento 2 is to interact directly with the database using raw SQL queries. Even though you might be proficient in writing advanced SQL statements, doing so in Magento can lead to a host of issues.

Directly querying the database might seem like a quick solution, but it bypasses Magento's frameworks and best practices. This can cause security vulnerabilities, make your code less maintainable, and create compatibility issues with future updates.

Let’s explore this in detail with an example.

Bad Practice: Using Direct SQL Queries

The Code

<?php

declare(strict_types=1);

namespace Vendor\\Module\\Helper;

use Magento\\Framework\\App\\ResourceConnection;

readonly class DataHelper
{
    public function __construct(
        private ResourceConnection $resource,
    ) {}

    public function getCustomerData(
        $customerId,
    ): array {
        $connection = $this->resource->getConnection();
        $tableName = $connection->getTableName('customer_entity');

        $query = "SELECT * FROM " . $tableName . " WHERE entity_id = " . $customerId;

        return $connection->fetchAll($query);
    }
}

What's Going On Here?

The code aims to retrieve customer data based on a given customer ID. It builds an SQL query by concatenating the table name and the customer ID, and then executes the query to fetch the data. At first glance, this may not appear to be too bad, but there are a number of reasons why we should avoid this approach.

Why It Sucks

• Bypassing Magento's Abstraction: Direct SQL queries ignore Magento's database abstraction layer, which handles important tasks like caching, index management, and cross-database compatibility.
• Maintainability Issues: Hardcoding table names and SQL syntax makes your code fragile. Changes in the underlying database schema could break your code.
• Lack of Events and Observers: Magento's frameworks trigger specific events during data operations. By not using Magento’s standard data access methods, other modules won't be aware of database changes, leading to inconsistent application states and potential data integrity issues.
• Security Risks: Concatenating user input directly into SQL queries exposes your code to SQL injection attacks. Even if the parameter seems safe, malicious input could exploit vulnerabilities.

Potential Exploit Example

Without proper casting or parameter binding, an attacker could pass a malicious value into the $customerId parameter. Suppose a user could pass in the $customerId from a frontend input field.

They might input:

0; DROP TABLE customer_entity; --

When this value is passed into the function, the query becomes:

SELECT * FROM customer_entity WHERE entity_id = 0; DROP TABLE customer_entity; --;

This could execute the first statement and then proceed to drop the entire customer_entity table, resulting in catastrophic data loss.

The Right Way

To avoid these issues, we should use Magento's built-in methods for database operations. By using the Select object, Models, Resource Models, and ultimately Repositories, we make our code safer, more maintainable, and aligned with Magento's best practices.

Solution 1: Using the Select Object

The first logical step to improve this code is to refactor it to use Magento’s select object.

The Code

<?php

declare(strict_types=1);

namespace Vendor\\Module\\Helper;

use Magento\\Framework\\App\\ResourceConnection;

readonly class DataHelper
{
    public function __construct(
        private ResourceConnection $resource,
    ) {}

    public function getCustomerData(
        int $customerId,
    ): array {
        $connection = $this->resource->getConnection();
        $tableName = $connection->getTableName('customer_entity');

        $select = $connection->select()
            ->from(['ce' => $tableName])
            ->where('entity_id = :entity_id');

        $bind = ['entity_id' => $customerId];

        return $connection->fetchAll($select, $bind);
    }
}

What's Going On Here?

• Type Hinting: The $customerId parameter is type-hinted as an int, which drastically reduces the risk of SQL injection.
• Using the Select Object: We create a Select object using $connection->select(), which safely builds the SQL query.
• Parameter Binding: Instead of injecting the customer ID directly into the query, we use a placeholder :entity_id and provide the actual value through the $bind array. When the actual values are bound to placeholders like :entity_id, the database treats them strictly as data, not as executable code. This means that any malicious input cannot alter the structure of the SQL command, which effectively prevents SQL injection attacks.

Solution 2: Using Models and Resource Models

While the above solution is a significant improvement, a more appropriate way to interact with the database in Magento 2 is by using Models and Resource Models.

The Code

<?php

declare(strict_types=1);

namespace Vendor\\Module\\Helper;

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

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

    public function getCustomerData(
        int $customerId,
    ): Customer {
        $customer = $this->customerFactory->create();
        $this->customerResource->load($customer, $customerId);

        return $customer;
    }
}

What's Going On Here?

• Using the Customer Model Factory: We use CustomerFactory to create a new instance of the Customer model.
• Loading Data with the Resource Model: We use CustomerResource to load the customer data into the model instance based on the $customerId.
• Type Hinting and Return Types: By specifying return types, we improve code clarity and reduce errors.
• Magento's Event System: Loading models through resource models ensures that all events and observers tied to customer operations are triggered.

Why This Is Better

• Alignment with Magento's Architecture: Models and Resource Models are fundamental components of Magento's MVC architecture.
• Automatic Handling of Events: Operations trigger the necessary events and observers, which maintains system integrity.
• Maintainability: By abstracting database interactions through models, the code is less susceptible to changes in the database schema.
• Reusability: Models can be reused across different parts of the application, promoting DRY (Don't Repeat Yourself) principles.

Solution 3: Using Repositories

Taking it a step further, Magento 2 introduces service contracts and repositories, which provide an even cleaner way to interact with data models. It is preferred to use a repository whenever one is available, as it is the highest-level of abstraction in Magento.

The Code

<?php

declare(strict_types=1);

namespace Vendor\\Module\\Helper;

use Magento\\Customer\\Api\\CustomerRepositoryInterface;
use Magento\\Customer\\Api\\Data\\CustomerInterface;
use Magento\\Framework\\Exception\\LocalizedException;
use Magento\\Framework\\Exception\\NoSuchEntityException;

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

    /**
     * @throws NoSuchEntityException
     * @throws LocalizedException
     */
    public function getCustomerData(
        int $customerId,
    ): CustomerInterface {
        return $this->customerRepository->getById($customerId);
    }
}

What's Going On Here?

• Using Service Contracts: We inject CustomerRepositoryInterface, adhering to Magento's service contracts.
• Fetching Customer Data: We use the getById method to retrieve customer data, which returns a CustomerInterface instance.
• Type Safety: The method returns a CustomerInterface, ensuring that the returned data adheres to a specific contract.

Why This Is Even Better

• Simplified Code: Using repositories reduces the amount of code needed to perform common operations.
• Consistency: Service contracts provide a consistent API for interacting with Magento entities.
• Future-Proofing: Relying on service contracts makes your code more resilient to underlying changes in Magento's architecture.
• Testing and Maintenance: Repositories make it easier to write unit tests and maintain the codebase.

Recap

Avoiding direct SQL queries in Magento 2 is crucial for building secure and maintainable applications. By leveraging Magento's built-in frameworks — such as the Select object, Models and Resource Models, and Repositories — you ensure that your code is secure, maintainable, future-proof, and aligned with Magento’s best practices.