Class as request type for message queue with camel-case properties in constructor not instantiated properly

smartexcan commented 4 weeks ago

Summary

Using a class for the message queue request type, and having the properties in the constructor using camel-case names, the object cannot be instantiated properly as the object data keys are converted to snake-case.

Tested/reporting on 2.4.6-p6, though i don't think it has changed in a while.

Examples

<?php
namespace \Test\Module\Model;

class SyncRequestData
{
    /**
     * @var int
     */
    private $storeId;

    /**
     * @var int[]
     */
    private $sessionIds;

    /**
     * @var int
     */
    private $websiteStoreId;

    /**
     * @param int $storeId
     * @param int[] $sessionIds
     * @param int $websiteStoreId
     */
    public function __construct(
        int $storeId,
        array $sessionIds,
        int $websiteStoreId
    ) {
        $this->storeId = $storeId;
        $this->sessionIds = $sessionIds;
        $this->websiteStoreId = $websiteStoreId;
    }

    /**
     * @return int
     */
    public function getStoreId(): int
    {
        return $this->storeId;
    }

    /**
     * @return int[]
     */
    public function getSessionIds(): array
    {
        return $this->sessionIds;
    }

    /**
     * @return int
     */
    public function getWebsiteStoreId(): int
    {
        return $this->websiteStoreId;
    }
}

(Magento\Framework\MessageQueue\MessageEncoder calls the service input and output processors)

Magento\Framework\Webapi\ServiceOutputProcessor::convertValue converts the class to the following:

{"store_id":3,"session_ids":[1],"website_store_id":1}

Then in \Magento\Framework\Webapi\ServiceInputProcessor::getConstructorData, the keys do not match since they were converted to snake-case, but they are expected in camel-case, so an empty array is passed to objectmanager to create the object, resulting in an error due to not passing any arguments to constructor.

The only way I can currently get around this issue is having the class extend AbstractSimpleObject or DataObject and adding setters (or using the data constructor argument)

Proposed solution

No response

Release note

No response

Triage and priority

[ ] Severity: S0 - Affects critical data or functionality and leaves users without workaround.
[ ] Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
[X] Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
[ ] Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
[ ] Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.

m2-assistant[bot] commented 4 weeks ago

Hi @smartexcan. Thank you for your report. To speed up processing of this issue, make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce.

For more details, review the Magento Contributor Assistant documentation.
Add a comment to assign the issue: @magento I am working on this
To learn more about issue processing workflow, refer to the Code Contributions.

Join Magento Community Engineering Slack and ask your questions in #github channel. :warning: According to the Magento Contribution requirements, all issues must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting. :clock10: You can find the schedule on the Magento Community Calendar page. :telephone_receiver: The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

m2-assistant[bot] commented 2 weeks ago

Hi @engcom-Hotel. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

[ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
[ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue.
[ ] 3. Add Area: XXXXX label to the ticket, indicating the functional areas it may be related to.
[ ] 4. Verify that the issue is reproducible on 2.4-develop branch
Details
- If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
[ ] 5. Add label Issue: Confirmed once verification is complete.
[ ] 6. Make sure that automatic system confirms that report has been added to the backlog.

engcom-Hotel commented 2 weeks ago

Hello @smartexcan,

We request you to please provide more details on below:

Steps to reproduce
Actual Result
Accepted Result

Thanks

smartexcan commented 2 weeks ago

Created a module to demonstrate the issue. https://github.com/smartexcan/MQTestModule

See the included readme for steps to run, then check logs for class instantiation error.

smartexcan commented 2 weeks ago

As a side note, the InventorySales module uses an almost identical process, but the fields in their request data class are single words, which don't get changed to snake-case.

See https://github.com/magento/inventory/blob/develop/InventorySales/Plugin/Catalog/Model/SkuDataForReservationUpdate.php

engcom-Hotel commented 2 weeks ago

Hello @smartexcan,

Thanks for the prompt action on this issue. We have tried to proceed with the steps mentioned in the README.md, and got the below error message in the logs. Are you talking about this error only, or if possible please share the actual error you are getting in your logs?

[2024-10-09T07:34:50.220083+00:00] main.CRITICAL: Type Error occurred when creating object: Vendor\Example\Model\SyncRequestData, Vendor\Example\Model\SyncRequestData::__construct(): Argument #1 ($storeId) must be of type int, null given, called in /lib/internal/Magento/Framework/ObjectManager/Factory/AbstractFactory.php on line 121 [] []

==> var/log/debug.log <==
[2024-10-09T07:34:50.220083+00:00] main.CRITICAL: Type Error occurred when creating object: Vendor\Example\Model\SyncRequestData, Vendor\Example\Model\SyncRequestData::__construct(): Argument #1 ($storeId) must be of type int, null given, called in /lib/internal/Magento/Framework/ObjectManager/Factory/AbstractFactory.php on line 121 [] []

Thanks

smartexcan commented 2 weeks ago

Yes that would be the error I was getting as well.

As mentioned in the first post, the properties got converted from camel case to snake case when serialized, but aren't converted back to camel case during reconstruction, which results in no params being sent to the object constructor.

Workarounds for it would be using single word property names, grouping them in a data property, or extending from AbstractSimpleObject or DataObject

engcom-Hotel commented 2 weeks ago

Thanks @smartexcan for the confirmation.

We are confirming this issue for further processing. The community can also raise PRs to fix certain issues as part of the process. Therefore, I suggest creating a community PR to address this fix.

Thanks

github-jira-sync-bot commented 2 weeks ago

:white_check_mark: Jira issue https://jira.corp.adobe.com/browse/AC-13230 is successfully created for this GitHub issue.

m2-assistant[bot] commented 2 weeks ago

:white_check_mark: Confirmed by @engcom-Hotel. Thank you for verifying the issue.
Issue Available: @engcom-Hotel, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

magento / magento2