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] Change of FieldType->getStorageType() crashes custom field types #6413

Closed
ToBe998 opened this issue Feb 24, 2017 · 6 comments
Closed
Assignees
Labels
blocking release bug A bug that has been verified

Comments

@ToBe998
Copy link
Contributor

ToBe998 commented Feb 24, 2017

There has been a change in the getStorageType() method of the FieldType class. in Bolt 3.3-beta2

The new FieldTypeBase->getStorageType() method converts the string name of a field's type to it’s Doctrine class. If you override this method (like the docs say and all current extensions do) to return a string, this will crash as we need a Doctrine type class and not a string.

The old style as shown in the documentation shows that a custom field type should have a method like this:

	/**
	 * @return string
	 */
	public function getStorageType()
	{
		return 'text';
	}

The new FieldTypeBase however expects that method to do the necessary conversion, which our override clearly does not. (The internal types already have a changed method like shown here)

    /**
     * {@inheritdoc}
     */
    public function getStorageType()
    {
        return Type::getType('text');
    }

This means, all current extensions need to change their getStorageType() method. If not, you will get an error message (not cought by bolt's logging. I had to add a ini_set("display_errors", 1) in my app.php:

Fatal error: Call to a member function convertToPHPValue() on a non-object in /xxxxx/vendor/bolt/bolt/src/Storage/Field/Type/FieldTypeBase.php on line 123

This is most probably a bug as it would hurt BC a bit to much, right?

Details

  • Relevant Bolt Version: 3.3.0-beta2
  • Install type: Composer install
  • PHP version: 5.6
  • Used web server: Apache 2.4
@bobdenotter
Copy link
Member

Thanks!

@GwendolenLynch
Copy link
Contributor

GwendolenLynch commented Feb 24, 2017

Yep, changed in this commit 6bb1e17 on 26 May 2015 … about 2 years ago

@rossriley rossriley self-assigned this Feb 24, 2017
@ToBe998
Copy link
Contributor Author

ToBe998 commented Feb 24, 2017

Then i must have missed something. Fact is, that until 3.3 you could (and should according to documentation) return a string in your custom field's getStorageType() method and since 3.3-beta2 this will cause a critical error in below base method since you return a string and not a doctrine class.

At least that's where my debugging analysis ended. And since the base classes method returns a Doctrine type class and all current extensions override that method and return a string instead, the cause seemed clear to me. Did i miss something?

This is were the error happens:

    /**
     * {@inheritdoc}
     */
    public function hydrate($data, $entity)
    {
        $key = $this->mapping['fieldname'];
        $type = $this->getStorageType();
        $val = isset($data[$key]) ? $data[$key] : null;
        if ($val !== null) {
            $value = $type->convertToPHPValue($val, $this->getPlatform());
            $this->set($entity, $value);
        }
   }

And this is the message:

convertToPHPValue() on a non-object in /xxxxx/vendor/bolt/bolt/src/Storage/Field/Type/FieldTypeBase.php on line 123

Line 123 is the line with $value = $type->convertToPHPValue($val, $this->getPlatform()); in it.

@GwendolenLynch
Copy link
Contributor

Yeah, totally my bad … I was reading the code & issue in the airport and then saw Ross' comments in Slack hours later.

I think he's got it under control 👍

@GwendolenLynch
Copy link
Contributor

Fixed in #6418 … my humble apologies again, @ToBe998 for poor reading/triage on my part … I genuinely feel bad.

P.S. Obviously as per normal, if we've derped the fix … reopen and we'll kick swat the bug a.s.a.p.

@ToBe998
Copy link
Contributor Author

ToBe998 commented Feb 26, 2017

Everything is fine. Glad to see it fixed or cleared up. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking release bug A bug that has been verified
Projects
None yet
Development

No branches or pull requests

4 participants