[NFR] \Phalcon\Mvc\Model doesn't save an empty string

[iforp]

$note = new Notes();
$note->note = '';
$note->save();
var_dump($note->getMessages());

array (size=1)
    0 => object(Phalcon\Mvc\Model\Message)[49]
        protected '_type' => string 'PresenceOf' (length=10)
        protected '_message' => string 'note is required' (length=16)
        protected '_field' => string 'note' (length=4)
        protected '_model' => null

But empty string is valid value

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[phalcon]

Most of functions to filter/sanitize data return empty strings instead null values:

var_dump(strip_tags('<h1></h1>')); // string(0) ""
var_dump(strip_tags(null)); // string(0) ""
var_dump(trim(null)); // string(0) ""
var_dump(htmlspecialchars(null)); // string(0) ""
var_dump(filter_var(null, FILTER_SANITIZE_EMAIL)); // string(0) ""
var_dump(filter_var(null, FILTER_SANITIZE_URL)); // string(0) ""
var_dump(filter_var(null, FILTER_SANITIZE_MAGICQUOTES)); // string(0) ""

If Phalcon does not handle empty string the same as null values, the application could lead allow unexpected data as valid

[phalcon]

If you want you could globally remove the automatic not null validation (this applies for all models):

\Phalcon\Mvc\Model::setup(array(    
    'notNullValidations' => false
));

And implement your own validation in every model:

use Phalcon\Mvc\Model\Message;

class Notes extends Phalcon\Mvc\Model
{
    public function validation()
    {
        $notNullAttributes = $this->getModelsMetaData()->getNotNullAttributes($this);
        foreach ($notNullAttributes as $field) {
            if (!isset($this->$field) || $this->$field === null) {
                $this->appendMessage(throw new Message($field . ' is required'));
                return false;
            }
        }
        return true;
    }
}

[nikolasha]

This is a problem. Sometimes really comfortable prohibit NULL value, but allow an empty string. I think, by default, the Phalcon must check the exactly NULL value. If an empty string is also unacceptable, then the developer must explicitly say so. At least it is more obvious and consistent with the logic of the database.

P.S.
!isset($this->$field) || $this->$field === null this is equivalent to just !isset($this->$field) :)

[iforp]

@nikolasha is absolutely right.

var_dump(strip_tags('<h1></h1>')); // string(0) ""
and your other examples is expected behavior, but error on saving valid empty string is unobvious.

If an empty string is also unacceptable, then the developer must explicitly say so.

Implementation of own validation is voodoo magic in this case.

[phalcon]

This is not a problem, most of the cases in web applications we're receiving data from forms and external data:

$post->name = strip_tags($_POST['name']); // returns empty string
$post->content = strip_tags($_POST['content']); // returns empty string
$post->save(); //this will save with two fields empty, is that acceptable? is that the common case?

$user->name = strip_tags($_POST['name']); // returns empty string
$user->email = filter_var($_POST['email'], FILTER_SANITIZE_EMAIL); // returns empty string
$user->save(); //this will save with two fields empty, is that acceptable? is that the common case?

in a REST request:

http://localhost/api/products?name=

$products->name = trim($_GET['name']);
$products->save(); //this will save with and empty name, is that acceptable? is that the common case?

Do you want to implement string length validators for every char/varchar field in your model?
Is this something that would make the life of a developer easier, have to validate if every char/varchar field is null or empty string to satisfy less than 20% of cases?
Do you want to convert every empty string to null to make the "null validator" work?

If that is what you want, as I said you could disable the automatic validation and implement one that satisfy the level of explicitness you need. But this is something that is not going to be changed in Phalcon because it makes the programming more hard in most cases.

[iforp]

In my application I want to control data types. Null and empty string is not the same type.

For example in a REST request:

http://localhost/api/products?name=

var_dump($_GET['name']); // string '' (length=0)

vs

http://localhost/api/products

var_dump($_GET['name']); // null

and what about this case

http://localhost/api/products?name[]=

$products->name = trim($_GET['name']);
$products->save(); // this will save with name "Array", is that acceptable? is that the common case?

So your examples are contrived.
As a developer I would expect this behavior and would check data, not just trim and strip_tags.

Why I can not save data in the exact format in which I need it?
Is your recipe makes the programming more easy just to save empty string?

In the last resort it should be something like this:

class Notes extends \Phalcon\Mvc\Model
{
    public $id;
    public $note;
    public function initialize()
    {
        $this->allowEmptyString(array('note'));
        // or deny, if it allowed by default
        $this->denyEmptyString($fieldsList);
    }
}

[phalcon]

Data is stored as they come in the same format, the framework doesn't make any automatic transformation.

Data is usually filtered/sanitized before be stored in the database. As you can see most validations/filters/sanitizers return empty strings. If an application doesn't require filter/sanitize/validate data before store them that is simply wonderful! Users are doing a perfect job, and hackers are not allowed :)

If you're receiving an array you must be aware of that. Applying a string filter over an array doesn't make sense. I don't know if it's necessary point out that.

I understand this behavior is not perfect, but it has a good intention and removing it is a greater problem than an improvement. Why not just disable the automatic validation and implement a validation system that fits your needs?

More options:

1. Re-implement the automatic validation system
You could add a base model to your system, this gives you the freedom to implement any validation system, you can even translate the messages produced instead of using the hardcoded ones:

use Phalcon\Mvc\Model\Message;

class BaseModel extends Phalcon\Mvc\Model
{
    public function validation()
    {
        $notNullAttributes = $this->getModelsMetaData()->getNotNullAttributes($this);
        foreach ($notNullAttributes as $field) {
            if (!isset($this->$field) || $this->$field === null) {
                $this->appendMessage(throw new Message($field . ' is required'));
                return false;
            }
        }
        return true;
    }    
}

Use this class as base in your models:

class Notes extends BaseModel 
{
}

2. Convert empty strings into raw values
Alternatively, you can convert the empty strings to raw values skipping the automatic "not null" validator:

class Notes extends Phalcon\Mvc\Model
{
    public function beforeValidation()
    {
        if ($this->note === '') {
            $this->note = new Phalcon\Db\RawValue('');
        }
    }
}

3. Change the field to allow null and move this validation to the application layer
If it's acceptable, marking the not null field in the table to allows nulls, implementing this validation in your application:

use Phalcon\Mvc\Model\Message;

class Notes extends Phalcon\Mvc\Model
{
    public function validation()
    {
        if ($this->note === null) {
            $this->appendMessage(throw new Message('Note cannot be null'));
            return false;
        }
    }
}

[iforp]

Thanks for your answers. In these ways I can solve my current problem.
But the real problem is the lack of flexibility.

For example look at the Yii solution

public function rules()
{
    return array(
        array('name', 'required'),
        array('name', 'length', 'min'=>3, 'max'=>12),
        array('note', 'type', 'type' => 'string'),
    );
}

[iforp]

2 . Convert empty strings into raw values

$note = new Notes();
$note->note = new Phalcon\Db\RawValue('');
$note->save();
// PDOException: Syntax error or access violation

cause it generate SQL query like this

INSERT INTO `notes` (`note`, `id`) VALUES (, null)

The correct usage is

$note->note = new Phalcon\Db\RawValue('""');

[nikolasha]

@phalcon, this is understandable and commendable that you are trying to make life easier for developers. But here it is necessary to know the measure. In practice, such a decision would only reduce flexibility of the framework, and non-obvious behavior. And in return, will not bring a noticeable benefit. Let me explain why. Typically, the string data required, require a more rigorous test than the test for an empty string. You want to check that the name contains valid characters, email address matches the format, password is long enough, etc. In these cases, an empty string problem is solved by itself. If the data does not need to be checked, it did not really matter that tells the user - a empty string or a stupid value.

Incidentally, the same problem will be with a date which does not allow NULL value and is set in the database automatically. In general, there is also the problem of NOT NULL fields that have default values ​​in the database.

A few more thoughts:

1. Disable check for NULL value for all fields at once is a bad decision. Setting should be more flexible.
2. Even if you leave it as is, then you must rename notNullValidations to notEmptyValidations :)

[nikolasha]

Perhaps it would be a good solution:

$this->allowEmptyString(array('note'));

[luiz-brandao]

@phalcon I also agree that an empty string IS a valid value and as such should be allowed

[hugoduraes]

+1

[sergiobc]

Confirm the issue.
When I faced the error message, I first thought it was a bug. Then reading this post I was surprised to find out this is a feature.
Agree with the previous posters, that empty string is a special case for validation. And default constraints should match the underlying database rules, since the framework uses DB field metadata to validate the values.

[nexik]

👍

If I want field to be required I would rather write something like this:

/**
 * @Column(type="string", length=80, nullable=false, required=true)
 */

[rebelmiles]

I think this is a problem too. I have it it several times and had to work around it.

The latest concerns a CHAR field which is a primary key but can contain an empty string.

I should not have to restructure a database to "fit in" with how phalcon thinks things "should be done".

[DaanBiesterbos]

@rebelmiles You don't have to. You can assign empty values like this: $robot->foo = new \Phalcon\Db\RawValue('""');
But I do agree with you. The solution is not pretty. I would like to see a better solution.

[rebelmiles]

@DaanBiesterbos I tried that solution. I think "not pretty" is a gross understatement.

[jimmycdinata]

I agreed with @nikolasha . It will be great if Phalcon allowed empty string as valid value.

+1

[informagician]

does anyone have any updates/progress on this issue?

[vlad-khramov]

@phalcon, you comment #440 (comment) helps, but i think it hacks.

In this case phalcon orm trying to do validations of business logic, but it not problem of orm.

All cases in #440 (comment) are acceptable. It task of business logic to allow or not allow empty name, email.

Another example:
model User (name, comment not null, default ''). I save user (name='robot', comment=''). Than

$user = User::findFirst("name='robot'");
$user->name='robot2';
$user->save();//!!!!!!

Programmer must write validations to not allow empty values in some fields, not to allow empty values for all fields.

[Nevario]

+1 I would also like to see a cleaner way to allow an empty string.

[virgofx]

+1 Empty strings should be allowed

[phalcon]

Empty strings are allowed, who said they aren't allowed?

[vlad-khramov]

@phalcon, example:

class Robot extends \Phalcon\Mvc\Model {
    public $name;//not null, default ''
    public $comment;//not null, default ''
}
$r = Robot::findFirst(1); //(name='robot', comment='')
$r->name = 'robot2';
$r->save();
var_dump($r->getMessages());

Result:

array (size=1)
  0 =>
    object(Phalcon\Mvc\Model\Message)[37]
      protected '_type' => string 'PresenceOf' (length=10)
      protected '_message' => string 'comment is required' (length=19)
      protected '_field' => string 'comment' (length=7)
      protected '_model' => null
      protected '_code' => int 0

[phalcon]

@quantum13

use Phalcon\Mvc\Model;

class Robot extends Model
{
    public $name;//not null, default ''
    public $comment;//not null, default ''
}

Model::setup(['notNullValidations' => false]);

$r = Robot::findFirst(1); //(name='robot', comment='')
$r->name = 'robot2';
$r->save();
var_dump($r->getMessages());

Result:

array(0) {
}

[andresgutierrez]

This is implemented in 2.0.3

<?php

namespace Some;

use Phalcon\DI,
    Phalcon\Db\Column,
    Phalcon\Mvc\Model,
    Phalcon\Events\Manager as EventsManager,
    Phalcon\Db\Adapter\Pdo\MySQL as Connection,
    Phalcon\Mvc\Model\Manager as ModelsManager,
    Phalcon\Mvc\Model\Metadata\Memory as ModelsMetaData;

$eventsManager = new EventsManager();

$di = new DI();

$connection = new Connection(array(
    "host"     => "localhost",
    "username" => "root",
    "password" => "",
    "dbname"   => "phalcon_test",
));

$connection->setEventsManager($eventsManager);

$eventsManager->attach('db',
    function ($event, $connection) {
        switch ($event->getType()) {
            case 'beforeQuery':
                echo $connection->getSqlStatement(), " [", join(', ', $connection->getSqlVariables()), "]\n";  
                break;
        }
    }
);

$modelsManager = new ModelsManager();
$modelsManager->setDi($di);

$metaData = new ModelsMetadata();

$di['db'] = $connection;
$di['modelsManager'] = $modelsManager;
$di['modelsMetadata'] = $metaData;

class Robots extends Model
{
    public function initialize()
    {
        $this->allowEmptyStringValues(['name', 'text', 'datetime']);
    }
}

$robot = new Robots();
$robot->name = "";
$robot->text = "";
$robot->datetime = "";
$robot->save();
print_r($robot->getMessages());

[cottton]

sarcasm : thanks that i had to search to find out what happen with the empty strings /sarcasm

Bad solution imo. And as i read im not the only one.

2017 - Extra search|code|work|time to fix an issue with a model that just should insert an empty string.

Not so well done.

Not only that you force ppl to search for a solution. You force ppl to add code to fix a problem that actually should not be problem.

Adding code may bring more problems