Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]: Property source is overwritten in models #16514

Closed
KirDE opened this issue Jan 17, 2024 · 4 comments · Fixed by #16516
Closed

[BUG]: Property source is overwritten in models #16514

KirDE opened this issue Jan 17, 2024 · 4 comments · Fixed by #16516
Assignees
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: high High

Comments

@KirDE
Copy link

KirDE commented Jan 17, 2024

Questions? Forum: https://phalcon.io/forum or Discord: https://phalcon.io/discord

Describe the bug
If filed in database called source, content of it is overwritten with table name.
May be related to #11253
Could have something with method setSource() in a Model?.

To Reproduce
Get data from any table with field called source - content will be replaced with table name.
It doesn't matter, if $this->setSource('tablename') is called in initialize or table name is detected automatically from model name.

Expected behavior
source should contain data from database.

Screenshots
If applicable, add screenshots to help explain your problem.

Details

  • Phalcon version: tested with 5.5 and 5.6. On 5.4 bug doesn't exist
  • PHP Version: 8.2.13
  • Operating System: Rocky 9
  • Installation type: installing via package manager
  • Zephir version (if any): -
  • Server: Apache/2.4.57 (Rocky Linux)
  • Other related info (Database, table schema): mariadb 11.2, tested with 10.11 too.

Additional information
Checked other keywords, they are not affected (transaction, group).
Workaround for those who facing the bug - use version 5.4 or replace source with another field name.

@KirDE KirDE added bug A bug report status: unverified Unverified labels Jan 17, 2024
@niden
Copy link
Member

niden commented Jan 17, 2024

Related to this:

#15953

There is also a test in the database that checks for this in particular. Have a look here:

https://github.com/phalcon/cphalcon/blob/master/tests/database/Mvc/Model/SaveCest.php#L585

I checked the code once more and there is no $source property in the model class to conflict with any field name.

Can you test this again or provide a script that we can run against the testing suite?

@niden niden self-assigned this Jan 17, 2024
@KirDE
Copy link
Author

KirDE commented Jan 18, 2024

Just created a new Phalcon empty app, to reproduce the problem. It happens not during model save but already on model creation from database.

Here is a simple code:
TestController.php

<?php

use Phalcon\Http\Response;

class TestController extends \Phalcon\Mvc\Controller
{
    public function indexAction()
    {
        $result = Test::find();

        $response = new Response();
        $response->setJsonContent(
            $result
        );
        return $response;
    }
}

Model Test.php

<?php

use Phalcon\Mvc\Model;

class Test extends Model
{
    public $id;
    public $name;
    public $source;
    public $description;
}

SQL Table test

CREATE TABLE IF NOT EXISTS `test` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `name` varchar(50) DEFAULT NULL,
  `source` varchar(50) DEFAULT NULL,
  `description` varchar(50) DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=4 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;

INSERT INTO `test` (`id`, `name`, `source`, `description`) VALUES
	(1, 'one', 'internet', '234'),
	(2, 'two', 'github', '123'),
	(3, 'three', 'google', '553453');

Script output with Phalcon 5.6:
[{"id":1,"name":"one","source":"test","description":"234"},{"id":2,"name":"two","source":"test","description":"123"},{"id":3,"name":"three","source":"test","description":"553453"}]

Script output with Phalcon 5.4:
[{"id":1,"name":"one","source":"internet","description":"234"},{"id":2,"name":"two","source":"github","description":"123"},{"id":3,"name":"three","source":"google","description":"553453"}]

@niden
Copy link
Member

niden commented Jan 18, 2024

@KirDE Thank you for this. I was able to reproduce it.

The issue is in the toArray() method of the model. You are setting a model as a JSON payload, which calls the jsonSerialize in the model which in turn calls the toArray().

The recent change of #16491 is related, since it takes into account getters to populate the array. As a result, it finds the getSource() method and thinks it is a getter, so it populates it with that.

I am going to put a conditional there to ignore the getSource() method when the property is source.

Fix incoming.

@niden niden mentioned this issue Jan 18, 2024
5 tasks
@niden niden linked a pull request Jan 18, 2024 that will close this issue
5 tasks
@niden niden added status: high High 5.0 The issues we want to solve in the 5.0 release and removed status: unverified Unverified labels Jan 18, 2024
@niden niden added this to Phalcon v5 Jan 18, 2024
@niden niden moved this to In Progress in Phalcon v5 Jan 18, 2024
@niden
Copy link
Member

niden commented Jan 18, 2024

Resolved in #16516

Thank you @KirDE

@niden niden closed this as completed Jan 18, 2024
@niden niden moved this from In Progress to Implemented in Phalcon v5 Jan 18, 2024
@niden niden moved this from Implemented to Released in Phalcon v5 Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: high High
Projects
Status: Released
Development

Successfully merging a pull request may close this issue.

2 participants