Skip to content

Импорт пустых строк в PrimitiveString #166

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

Closed
sryabov opened this issue Dec 25, 2012 · 13 comments
Closed

Импорт пустых строк в PrimitiveString #166

sryabov opened this issue Dec 25, 2012 · 13 comments

Comments

@sryabov
Copy link

sryabov commented Dec 25, 2012

В настоящий момент при импорте пустой строки в любом случае (даже если не задана минимальная длина строки через setMin) не получаем в value пустую строку, а получаем null, хотя это разные значения (в том числе и для БД). Есть предложение изменить на следующий код:

public function import($scope)
{
    if (!BasePrimitive::import($scope))
        return null;

    if (!is_scalar($scope[$this->name]) || is_bool($scope[$this->name]))
        return false;

    $this->value = (string) $scope[$this->name];

    $this->selfFilter();

    if (is_string($this->value)) {
        $length = mb_strlen($this->value);

        if(
            ($this->max === null || $length <= $this->max)
            && ($this->min === null || $length >= $this->min)
            && ($this->pattern === null || preg_match($this->pattern, $this->value))
        )
            return true;
    }

    $this->value = null;

    return false;
}
@pupkinV
Copy link
Contributor

pupkinV commented Dec 25, 2012

!BasePrimitive::import($scope) будет true т.к. импорт вернет null. Это не спасет Вас если для Вас это проблема.
Если я правильно понял, то проблема тут еще и в том, что imported для пришедшей пустой строки false?

@stev
Copy link
Contributor

stev commented Dec 25, 2012

тонкое чувство юмора присутствует 👍 ))

@AlexeyDsov
Copy link
Member

Так или иначе оно сломает BC, если сделать так как хочет @sryabov

@pupkinV
Copy link
Contributor

pupkinV commented Dec 25, 2012

ключевое слово "хочет" =)
@sryabov можно Вас попросить привести пример: как это мешает работе и как вы обошли первую проверку в предлагаемом методе. Видать это не всё, что Вы хотели преложить?

@sryabov
Copy link
Author

sryabov commented Dec 25, 2012

Ах да, проморгал что в BasePrimitive import тогда тоже нужно изменить на (почему-то показалось что я его здесь уже видел в таком варианте, видимо в чьей-то ветке был):

protected function import($scope)
{
    if (isset($scope[$this->name])) {
        $this->raw = $scope[$this->name];

        return $this->imported = true;
    }

    $this->clean();

    return null;
}

в ином случае происходит потеря данных в виде пустой строки которая так же является информацией (надеюсь ни кто не будет спорить что пустая строка все же по своей информативности сильно отличается от отсутствия значения =).

По идее для сохранения совместимости можно установить начальное значение min для PrimitiveString в 1, что даст тот же результат, но будет возможность его обнулить и не терять информацию. Иначе получается какая-то магия - пустую строку в форму передали, а примитив считает что нет.

@sryabov
Copy link
Author

sryabov commented Dec 25, 2012

Что касается кейса - то в текущем состоянии если будет передана пустая строка, то при импорте данных в форму она не попадет в примитив и соответственно при FormUtils::form2object в существующий объект соответствующее свойство не будет заполнено переданной пустой строкой.

@suquant
Copy link
Member

suquant commented Dec 25, 2012

Это как раз переделки этого реквеста #101
Там много вопросов такого плана и вы не первый кто ими задается

@AlexeyDsov
Copy link
Member

@sryabov не думаю что мы можем взять и поменять поведение PrimitiveString на другое. Кто его использует ожидают что он работает так как он работает - если пустая строка, то null. Как решение проблемы для себя - сделать примитив вроде вот такого:

class PrimitiveStringMine extends PrimitiveString
{
    public function import($scope)
    {
        if (
            isset($scope[$this->getName()])
            && is_scalar($value = $scope[$this->getName()])
            && empty($value)
        ) {
            $this->raw = $value;
            $this->value = $value;
            $this->imported = true;
            return true;
        }

        return parent::import($scope);
    }
}

По тестам

$prm = new PrimitiveStringMine('str');
$flag = $prm->import(['str' => '']);
var_dump($flag, $prm->getValue());

$flag = $prm->importValue('');
var_dump($flag, $prm->getValue());

$flag = $prm->importValue(null);
var_dump($flag, $prm->getValue());

У меня такой результат

bool(true)
string(0) ""
bool(true)
string(0) ""
NULL
NULL

@sryabov
Copy link
Author

sryabov commented Dec 26, 2012

@AlexeyDsov к сожалению использование альтернативного PrimitiveString не спасет форму генерируемую через makeForm в Proto. Я не совсем представляю в каких случаях нужна потеря данных и собственно начал копать этот примитив только по причине того, что в следующем коде получил неожиданный результат. Возможно я что-то не так понимаю в логике onPHP и вы сможете меня подтолкнуть на правильное решение.

Допусти есть мета:

<class name="Person">
    <properties>
        <identifier name="id" type="Integer" />
        <property name="name" type="String" size="64" required="true" />
        <property name="comment" type="String" size="255" />
    </properties>
    <pattern name="StraightMapping" />
</class>

Есть созданный ранее объект:
{id: 1, name: 'Вася Иванов', comment: 'Какой-то комментарий'}

id мы получаем через URL, в таком случае сделать сохранение изменений для меня выглядит логичным так:

$person = Person::dao()->getById($id);

$form = 
    Person::proto()->
        makeForm()->
            drop('id')->
        import($request->getPost());

if(!$form->getErrors()) {
    FormUtils::form2object($form, $person);
    Person::dao()->take($person);
}

Но если в POST мы получаем {name: 'Вася Иванов', comment: ''}, то comment не будет обновлен. И вообще никаких следов того, что передавали пустой comment в Form не будет, и единственным способом будет прохождение по POST в поисках пустых строк, что кажется несколько странным - т.к. вроде POST уже был импортирован в форму.

@AlexeyDsov
Copy link
Member

Форму генерируемую через makeForm можно поправить в makeForm:

class ProtoPersion extends AutoProtoPerson
{
    public function makeForm($prefix = null)
    {
        $form = parent::makeForm($prefix);
        $form->drop('comment')->add(new PrimitiveStringMine('comment'));
        return $form;
    }
}

Вполне нормальная практика добавить регулярку на string примитив или добавить какой-то фильтр который должен всегда использоваться.

@sryabov
Copy link
Author

sryabov commented Dec 26, 2012

Такое придется делать на все строки и при этом еще сохранить max длину строки...

@dovg
Copy link
Member

dovg commented Dec 26, 2012

@AlexeyDsov +1

@sryabov чтобы не писать много, можно сделать декоратор.

$comment = $form->get('comment');
$form->drop('comment')->add(new PrimitiveStringMine($comment));

@sryabov
Copy link
Author

sryabov commented Dec 26, 2012

Тогда уж что-то типа

PrimitiveStringMine::updatePrimitive($form, 'comment');

а в нем уж get, drop и add new... так хоть короче будет и не придется код дублировать для каждой строки, хотя конечно тоже не айс строить конструкции для избежания потери данных =)

@ssserj ssserj closed this as completed Dec 27, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants