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

If I set up the default values to some specific columns, these columns can not act the same expectation in partial bean mode. #754

Closed
ghost opened this issue Nov 11, 2019 · 4 comments

Comments

@ghost
Copy link

ghost commented Nov 11, 2019

When I upgrade my code to Redbean 5.4 and turn on the partial bean mode, I found this issue I mentioned.

As Redbean recommends, I set up the default values to some specific columns before I use them.

public function dispense()
{
    $defaultValues = self::getDefaults();
    foreach ($defaultValues as $key => $value) {
        $this->{$key} = $value;
    }
}

And then I use the partial bean mode to store the columns updated rather than the entire bean.

public function store(
    OODBBean $bean,
    $yesNoBeans = true,
    bool $unfreezeIfNeeded = false
) {
    $this->checkType($bean);

    if (!environment('production')) {
        R::usePartialBeans($yesNoBeans);
    }

    return R::store($bean, $unfreezeIfNeeded);
}

But I found if I set up the default values, it will change the changelist in the bean which is the foundation of the partial bean mode.

$partial = ( $this->partialBeans === TRUE || ( is_array( $this->partialBeans ) && in_array( $table, $this->partialBeans ) ) );
if ( $partial ) {
    $mask = $bean->getMeta( 'changelist' );
    $bean->setMeta( 'changelist', array() );
}

I try to go through the code of Redbean. I found that when we use R::load or R::find, it will call the dispense function which will influence the changelist.

Maybe we can make a better mechanism for setting up the default values with the partial bean mode working well?

Below is the part of my test code.

    /**
     * To check whether it opens partial beans or not.
     *
     * @param string     $type       the type of repository
     * @param bool|array $yesNoBeans partial bean setting
     *
     * @return bool
     */
    private function _isPartialBeans($type, $yesNoBeans): bool
    {
        return
            $yesNoBeans === true ||
            (
                is_array($yesNoBeans) &&
                in_array($type, $yesNoBeans, true)
            );
    }

    /**
     * The test process.
     *
     * @param string     $table      the table name
     * @param bool|array $yesNoBeans partial bean setting
     * @param string     $columnOne  the column in the table
     * @param string     $columnTwo  the column in the table
     * @param mixed      $object     the callable param
     *
     * @throws SQL
     *
     * @return void
     */
    private function _test(
        string $table,
        $yesNoBeans,
        string $columnOne,
        string $columnTwo,
        $object
    ): void {
        // Check the partial beans setting
        $isPartialBeans = $this->_isPartialBeans($table, $yesNoBeans);

        // Initial the variables
        $generateTestRecord = null;
        $oldRecord = null;
        $newRecord = null;
        $afterStoreRecord = null;

        // Generate the test record
        $valueOne = 'First';
        $valueTwo = 'Second';
        $generateTestRecord = R::dispense($table);
        $generateTestRecord->{$columnOne} = $valueOne;
        $generateTestRecord->{$columnTwo} = $valueTwo;
        $id = R::store($generateTestRecord);

        // Load the test record and assert
        $oldRecord = R::load($table, $id);
        $this->assertEquals($valueOne, $oldRecord->{$columnOne});
        $this->assertEquals($valueTwo, $oldRecord->{$columnTwo});

        // Prepare to change the properties
        $newValueOne = "New {$valueOne}";
        $newValueTwo = "New {$valueTwo}";
        $oldValueOne = $oldRecord->{$columnOne};
        $oldRecord->{$columnOne} = $newValueOne;
        $oldValueTwo = $oldRecord->{$columnTwo};

        // Change the database directly
        R::exec(
            "UPDATE {$table} SET {$columnTwo} = ? WHERE id = ?",
            [
                $newValueTwo,
                $id,
            ]
        );
        $newRecord = R::load($table, $id);
        $this->assertEquals($oldValueOne, $newRecord->{$columnOne});
        $this->assertEquals($newValueTwo, $newRecord->{$columnTwo});

        // Change the database by function
        $params = [is_array($object) ? [$oldRecord] : $oldRecord];
        if (is_object($object) || is_array($object)) {
            $params[] = $yesNoBeans;
        }
        call_user_func_array(
            !is_array($object) ? [$object, 'store'] : $object,
            $params
        );

        // Check the data
        $afterStoreRecord = R::load($table, $id);
        $this->assertEquals($newValueOne, $afterStoreRecord->{$columnOne});
        $this->assertEquals(
            $isPartialBeans ? $newValueTwo : $oldValueTwo,
            $afterStoreRecord->{$columnTwo}
        );

        //Clean the data
        R::trash($table, $id);
        unset(
            $generateTestRecord,
            $oldRecord,
            $newRecord,
            $afterStoreRecord
        );
    }
@Lynesth
Copy link
Collaborator

Lynesth commented Nov 14, 2019

Hi,

I'm not sure I understand, everything seems to work as expected.

Do you want to set defaults value only if you dispense a new bean, not when you load/find/etc an existing one ?

If that's the case you can simply change your dispense event like this:

public function dispense()
{
    if (empty($this->bean->id)) {
        $defaultValues = self::getDefaults();
        foreach ($defaultValues as $key => $value) {
            $this->{$key} = $value;
        }
    }
}

@ghost
Copy link
Author

ghost commented Nov 14, 2019

Hi,

I'm not sure I understand, everything seems to work as expected.

Do you want to set defaults value only if you dispense a new bean, not when you load/find/etc an existing one ?

If that's the case you can simply change your dispense event like this:

public function dispense()
{
    if (empty($this->bean->id)) {
        $defaultValues = self::getDefaults();
        foreach ($defaultValues as $key => $value) {
            $this->{$key} = $value;
        }
    }
}

I will try. Thanks a lot.

@ghost
Copy link
Author

ghost commented Nov 14, 2019

Hi,

I'm not sure I understand, everything seems to work as expected.

Do you want to set defaults value only if you dispense a new bean, not when you load/find/etc an existing one ?

If that's the case you can simply change your dispense event like this:

public function dispense()
{
    if (empty($this->bean->id)) {
        $defaultValues = self::getDefaults();
        foreach ($defaultValues as $key => $value) {
            $this->{$key} = $value;
        }
    }
}

I tried. But I think I can't influence the whole process. Based on the below code, when I call R::load('some type', $id), it will set default values in $this->dispense( $type ).

public function load( $type, $id )
{
    $rows = array();
    $bean = $this->dispense( $type );
    if ( isset( $this->stash[$this->nesting][$id] ) ) {
        $row = $this->stash[$this->nesting][$id];
    } else {
        try {
            $rows = $this->writer->queryRecord( $type, array( 'id' => array( $id ) ) );
        } catch ( SQLException $exception ) {
            if (
                $this->writer->sqlStateIn(
                    $exception->getSQLState(),
                    array(
                        QueryWriter::C_SQLSTATE_NO_SUCH_COLUMN,
                        QueryWriter::C_SQLSTATE_NO_SUCH_TABLE
                    ),
                    $exception->getDriverDetails()
                )
            ) {
                $rows = array();
            } else {
                throw $exception;
            }
        }
        if ( !count( $rows ) ) {
            return $bean;
        }
        $row = array_pop( $rows );
    }
    $bean->importRow( $row );
    $this->nesting++;
    $this->oodb->signal( 'open', $bean );
    $this->nesting--;
    
    return $bean->setMeta( 'tainted', FALSE );
}

I think in the below code, we should add one line code $this->__info['changelist'] = array();.

public function importRow( $row )
{
    $this->properties = $row;
    $this->__info['sys.orig'] = $row;
    $this->__info['changed'] = FALSE;
    return $this;
}

@Lynesth
Copy link
Collaborator

Lynesth commented Nov 15, 2019

Made some big edits to that answer, sorry about that.

Ok I understand what your problem is.
Nevertheless, #755 is not a solution to fix it.

Let me strike that, #755 does fix the issue and doesn't seem to break any BC.

@gabordemooij
My earlier proposal didn't work because the dispense event is fired before the bean's properties are filled with what has been retrieved from the database.

Using the proposal from #755 fixes the problem encountered here but doesn't prevent the code from the dispense event to fire needlessly.
Should we create two different events, like a dispense_new and a dispense_for_load or something along those lines ? We could also keep the previous event dispense for BC.

@ghost ghost closed this as completed Nov 18, 2019
gabordemooij added a commit that referenced this issue Nov 18, 2019
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants