strange actual value

[pupkinV]

Метод getActualVal взвращает raw (!) импорт был, но не удался.
Может я чегойта не понимаю, но если я в форме впишу

$form =
    Form::create()->
        add(
            Primitive::integer('limit')->
            setDefault(20)->
            setMin(10)->setMax(50)
        )->
        import(array('limit' => 10000));  // или "йа строка"

$this->result =
    Criteria::create(Person::dao())->
        setLimit($form->getActualValue('limit'))->
        getList();

получу 10000, но вот почему-то я ожидал получить 20 пользователей.

Причем getSafeValue вернет null.
Тесты не запускал, ибо не пользуюсь ими пока (не дорос), но код тестов просмотрел и ничего полезного там не нашел.
В onphp-ucheba это выпилино окло года назад (недо-beta 0.1), а до новой версии на днях добрался. =)

[dovg]

Может соответствующий тест добавить?

[suquant]

Да надо тест добавить, явно так не должно работать

[AlexeyDsov]

API ломается

public function getSafeValue()
{
    if ($this->imported)
        return $this->value;

    return $this->default;
}

Чем он не подходит, так что хочется getActualValue сделать почти таким же как он?

getActualValue возвращает либо импортированное значение если импорт прошел успешно либо значение переданное для импорта либо если не было ничего импортировано, то дефолтное. Не думаю что это security hole, скорее оно так задумано, что бы в форме выводить либо верное значение либо то какое ввел пользователь.

[pupkinV]

я видимо неудачный пример привел. ты отдаешь отчет в том примитив инт. может вернуть массив или строку.

Чем он не подходит...

ничем. в приведенном примере код надо допилить до

$form->getValue('num') ? $form->getValue('num') : $form->get('num')->getDefault()

тогда перепиливаем safe и проверяем не $this->imported, а ! empty($this->value)

[pupkinV]

да и причем именно empty ибо следующий шаг это для листов и примитива сет по дефолту должен возвращаться массив.

[stev]

Если по мне так это не нормально, это дыра в безопасности.
Форма должна выступать в роли барьера, на нее вся надежна а она значит берет и просто возвращает Raw

проблема в том что если в форме указал default ( как в примере )
то что у меня произойдет при переводе формы в объект ?

вместо ожидаемого мною дефолта я получу не просто пустое значение так куда хуже неправильное! O_o)
что мягко говоря смущает.

[AlexeyDsov]

не могу расшифровать мысль:

ты отдаешь отчет в том примитив инт. может вернуть массив или строку.

Какая дыра в безопастности? Никто не знает как метод работает? Или его заставляют использовать? Можно тогда добавить комментарий к нему.

Если нехватает какого-то метода - так может написать новый, вместо подгонки старого под желаемое?

[pupkinV]

pupkinV@f650741
здесь импорт фиксится, но я не стал пока это выкладывать ибо конфликтует с Георгием и его громадным импортом ошибок в примитывы.

Если нехватает какого-то метода - так может написать новый

только не это! ибо три существующих не всегда оправдывают свои названия. Хотя. getPupkinValue, getStevValue, getSolowebValue, getDovgValue

[pupkinV]

Может дождемся тестов? Я обещаю, что на днях разберусь как их запускать и как их надо писать и покрою это тестами. Но видать проблема в том что слово actual мы по разному понимаем. Меня интересует либо валидное значение пользователя, либо мое дефолтное. Всё! Остальное в $_REQUEST['any'].

[stev]

Алексей,
ход твоих мыслей настораживает

так может написать новый, вместо подгонки старого под желаемое?

Вот комментарии к методам не помешали бы, но они не решат вопрос, так как примитивы используются для импорта в объект. После импорта ожидается что у меня в форме все хорошо и данные безопасны для сохранения в базу и логично т.к все должно соответствовать правилам формы.

Если это не так то значит правила не правильно указаны.

Если нужна отличная логика, с забором не валидированного значение Raw это уже часный случай.

[AlexeyDsov]

Пообсуждав еще предлагаю следующий вариант:

• getActualValue остается как есть, но объявляется deprecated (плохо в нем не невалидные данные, а что микс return'а между строго-строковым значением $rawValue и значением любого типа (от объекта до инта) $value и $defaultValue - и получается что нужно всегда проверять что же этот метод вернул, а это не айс само по себе.
• добавляем метод getValueOrDefault (либо выдвиньте другое "понятное" название не пересекающееся с текущими) который работает как нужно - возвращает value если оно есть или дефолтное если оно есть иначе null.

[suquant]

Да собсно это совместное решенеи с @AlexeyDsov сделать именно так.

Идеи, возражения ребята ?

[dovg]

А мне больше по нраву предложение из соседнего реквеста - если попытка импорта не удалась, то isImported() == false.
Кейз с тем, что нам нужны невалидные данные из первого источника ИМХО надуман. Небольшим изменением поведения в importMore() можно пренебречь.

[pupkinV]

getActualValue остается как есть

а варианты рассматриваются? может спросить сообщество? мне кажись, что люди ожидали именно то, что реализует мой пулл.
Если он действительно что-то ломает, то очень хочется найти это и почитать.

[suquant]

isImported тоже трактуется не однозначно, толи это была-ли попытка импора (пытались ли вообще что то заимпортить) или-же заимпортилось ли валидное значение значени.

Мы сейчас @dovg о двух разных вещах говорим, наверное.

[stev]

@dovg это немного другое, к этому нужно отдельно подходить

isImported() == false.

тк, это пересекается с логикой трактования - что-есть ошибка при импорте. Это другое.

[stev]

Есть те кто сидит на местере на продакшигне ?

[anisimovt]

Я не понимаю что такое "была ли попытка импорта". Конечно была, примитив же есть, его форма не пропустит и попробует импортнуть в любом случае, т.е. после вызова import imported по логике попытки надо ставить в true всегда

[anisimovt]

getActualValue можно выставить в depricated да, оно дублирует getSafeValue, кейса получения raw value по такой логике в реальном коде я не представляю

[dovg]

Это другое.

Что другое? ИМХО isImported вне формы вообще не нужен.

Была ли попытка импорта - это крайне спорно. Например для boolean примитивов в html form отсутствие значение всегда трактуется как false, так что попытка была.

[suquant]

@anisimovt > Я не понимаю что такое "была ли попытка импорта"

Это когда у тебя форма есть но:

• Для примитива не было данных в скопе
• Либо вообще не было import* для формы

А сдругой стороны трактовать то можно и по другому, к примеру, был лы удачно заимпорчен примитив. те оказались ли данные для примитива валидными и импорт прошел успешно, чувствуете разницу :)

Ну понятное дело что чтобы однозначно узнать прошел ли импорт валидно мы также смотрим errors

[dovg]

@soloweb для boolean примитивов по существующей логике данные в скопе есть всегда. Прошу это учитывать )

[anisimovt]

Для примитива не было данных в скопе - импорт всё равно был, попытка была, ошибка проставляется

Либо вообще не было import* для формы - это уж пусть форма скажет, а не примитив

[suquant]

@anisimovt Либо вообще не было import* для формы - это уж пусть форма скажет, а не примитив

ок а если так Primitive::any()->import(...) нету формы :)
2 уровня, уровень формы и уровень примитива

[suquant]

@dovg у boolean вообще все весело

public function isImported()
{
    return ($this->imported && $this->value);
}

те isImported не просто возвращает а еще и в зависимости от того есть ли у нас True в value - магия !

[dovg]

@soloweb а это как раз на случай importMore. Никакой магии )
Например, если в get нет значения, то он может прийти в post. А если не пришел вообще, то false/

[pupkinV]

#157 (comment)
это к слову о том, что алиас самостоятелен. меня не сразу насторожило, что я правки в двух местах сделал. это злое дублирование логики

[suquant]

@dovg > Например, если в get нет значения, то он может прийти в post. А если не пришел вообще, то false/

В смысле разве isImported не показатель того что была попытка импорта, после чего этот флажок становится в положение True ???

Тогда причему тут откуда он пришел, была поптыка ставим True и никакой там магии, все должно быть железно :) разве нет ?

[dovg]

@soloweb

Я может косоязычно выразился.

Если ты в html сделаешь <input type="checkbox" ... >, то в случае если он не checked, в запросе не будет ничего, а если checked - то будет.
Текущее поведение для boolean отлично покрывает реальный мир.

[suquant]

@dovg > Если ты в html сделаешь , то в случае если он не checked, в запросе не будет ничего, а если checked - то будет.

Погоди мы сейчас о логике isImported а он показывает состояние была-ли вообще попытка импорта? Правильно ?

[dovg]

@soloweb в случае boolean - нет. :) Попытка импорта для boolean примитива была всегда. Таков реальный мир )

[suquant]

@dovg ну я люблю когда все очевидно и без магии, смотри у нас есть ключь imported, даже если в случае boolean будет попытка его заимпортить то оно всегда втсанет в положение True и функция isImported вернет то что надо.

Тут некчему еще доп проверка, наоброт это место для неочевидного бага.

[suquant]

@dovg ок вот конкретный пример

$bool = Primitive::boolean(...)->importValue(false);

var_dump($bool->isImported() ); # Что выведет ? false хотя попытка импорта то была, и это очевидно ?

[dovg]

@soloweb Примитивы писались для html форм в первую очередь AFAIK.
Там нужно именно такое поведение, которое реализовано сейчас. Твой кейз еще раз говорит нам о том, что isImported() снаружи трогать не следует.

[suquant]

@dovg это почему, я вот занимался реализацие widget-ов для автогенерации и именно из-за таких и многих других неочевидностей я этого сделать не смог, а onPHP инструмент который должен решать широкий спектр задач присущих веб-фреймворку.

[stev]

Парни @soloweb и @dovg все верно, но давайте это перенесем в другой топик

хотелось в этом топике разобрать казус с использованием
getValue, getActualValue, getSafeValue getChoiceValue ..

[stev]

Есть следующее предложение:
use case https://gist.github.com/4338235

т.е если объективно посмотреть то можно свети все к трем методам со следуюшим значением:

// getValue - есть правильное актуальное значение (собственно что от формы и требуется)
// getActualValue - только правильное значение но без (default)
// getRawValue - исходное значение

почему getValue(), потому что данный метод имеет основное значение при конвертации FormUtils

остальное - частный случай.

ваше мнение товарищи?

[suquant]

Это ломает BC @stev :) так нельзя делать

Смотри то что раньше должно было вернуть null станет возвращать default

[stev]

@soloweb это и коню ясно :)))
изменения 1.1 тоже ломают BC для 1.0 .. и что?

это пример того что хотелось бы видеть в новой версии

[suquant]

@stev я пока за #159 (comment)

Потом в версии 1.2 я бы вообще выпилил getActualValue и отсавил бы вместо него метод getValueOrDefault()
Давайте думать как избавлятся от лишней магии чем сажать новые :)

[stev]

@soloweb #159 смена шила на мыла. логика в другом

по факту, если указал в приметиве default - значит ты его будешь использовать если у тебя там нет нечего
если тебе не нужен default - следовательно и указывать его не нужно.

допускаю что может понадобиться без default.

сейчас же получается, что для получения, мне необходимо что-то делать.. ручками. от случая к случаю. Притом что я конкретно указал что есть default. Спрашивается - зачем? когда можно навести порядок и это не делать

[suquant]

@stev > по факту, если указал в приметиве default - значит ты его будешь использовать если у тебя там нет нечего
если тебе не нужен default - следовательно и указывать его не нужно.

Мы делаем интсрумент, зачем нам додумывать что там может понадобится, нам надо выплюнуть value это ведь просто геттер, зачем туда логику вкрячивать ? getValue() должен отдавать значение value и никак иначе, в противном случае это магия :)

[stev]

@soloweb магия есть сейчас!

в моем понимании getValue - должен вернуть не поле класса value, а правильное значение (собственно value - так и трактуется), об этом можно судить по тому что оно фигурирует в других местах FW

Если исходить, что это просто возврат поля - value, тогда с точностью наоборот получается:

// getActualValue - есть правильное актуальное значение (собственно что от формы и требуется)
// getValue - только правильное значение но без (default)
// getRawValue - исходное значение

а getActualValue или getValueOrDefault - второе дело.. суть одна

но тогда на первое место выходит метод getActualValue (getValueOrDefault)
и для автоматизации придется править FormUtils, потому что хотелось бы избежать - делать очевидные не нужные движения в коде связанные с default

хоть он и наоборот, но меня на данный момент устроит этот вариант. (с изменениями в FormUtils)

[AlexeyDsov]

@stev Такое изменения BC нах. Никто никакой проект под новую версию с таким изменением не сможет переписать.

[AlexeyDsov]

Моё мнение остановилось на варианте:

• getVakue - возвращает текущее значение
• getActualValue - не меняется, объявляется deprecated
• getValueOrDefault - возвращает текущее значение, если нет то дефолтное, если нет null
• exportValue - возвращает текущее значение в виде строки для импорта (то есть если это значение заимпортить снова в примитив, то value в нем снова будет таким же)
• getRawValue - возвращает значение используемое при импорте, т.е. не фильтрованное и это всегда строка

Теперь еще один вопрос на засыпку возник - а как вы вообще собираетесь использовать default? На примере PrimitiveIdentifier.

[stev]

@AlexeyDsov к #159 (comment) - с этим понятно

Мы у себя поменяли getActualValue - так как текущая реализация странная и ее в таком виде не могли использовать.
Для совместимости, можно сделать как ты говоришь
о чем собственно, и было написано #159 (comment)

Ок, на данный момент устраивает.

что касается default для PrimitiveIdentifier.
Если не пришел Id и по каким-то причинам не получается требовать, а примитив должен вернуть обязательно ObjectIdentifier - то используется default

• в чем сложность вопроса ?

[pupkinV]

Товарищи, парни, сообщество! Зачем вы на_много_написали_!?
#156
+ $this->imported = false;
если не удалось, спрятать голову в пясок и делать вид, что все ок. Причем только для енумов. Идентифаеры не касается?

// Lang: RU = 1; EN = 2;
var_dump(
    Form::create()->
        add(Primitive::enum('l')->
            of('Lang')->
            setDefault(Lang::create(Lang::RU))
        )->
        import(['l' => 5])->
        getActualValue('l')
);

вернет RU и по моему это очевидно.
Если, правда, глянуть шире, то у многих разработчиков нет времени проверять, что после принятия моего пула не полетит все к... не сломается. Но все отлично понимают, что в своем проекте это надо випилить поскорее, пока этот косяк не стал известен не честным на руку и желающим опрокинуть сайт на больших таблицах (из первого примера).
Каюсь, грешен -- использовал gAV не заглядывая в код, по примеру до меня писавших. Но предлогаемое изменение прошло без "сучка и торчка" и у меня и на работе ру (а это hi load).
Действительно если есть опасения, что за один присест не справится, делаем в два: создаем новый; со временем выпиливаем старый.
Опять "но": getAnyOrManyIfTryIsOk... -- режет слух. Пока правда ничего на ум приходит

• getExpectedValue,
• get (Magic|NoMagic) Value

[suquant]

@pupkinV Василий мне кажется из названия getValueOrDefault очевидно что оно делает нет ?
В отличие от getActualValue, тут не однозначно и может быть расстрактованно по разному.

[pupkinV]

@soloweb ты за getValueOrDefault как я понимаю? Я жду до выходных. Неохота еще раз перепиливать метод если потом кто-нибудь опомнится и решит, что "так делать нельзя". Заявки принимаются =).

Кстати авторы "Чистый код" и "Рефакоринг" (ссылаясь первый на второго :) советуют писать именно то, что делает метод, даже если название длинно. Современные IDE выполняют авто-подстановку и о длине метода не стоит беспокоится. Код страшноват порой.

gAV говорило само за себя. Жаль не развидеть увиденного. Оставлять "для совместимости" gav надо оградив его от людей комментариями, нотисом в почту, красной сигнальной ракетой. И это hole.

[pupkinV]

Уперся в свою малоопытность. В PrimitiveAlias::getActualValue является deprecated by getFormValue в котором не очень понятны намерения автора.
Если это допилить до кода понятного не только машине (отличительное правило программиста), то это

if ($this->primitive->isImported())
    return $this->primitive->getRawValue();

if ($this->primitive->getValue() === null)
    return null;

return $this->primitive->exportValue();

Внимание вопрос: имею ли я право написать * since version 1.0 by getValueOrDefault? Или это новая ветка обсуждения типа открыть кавычки -- что это и зачем оно нам? -- закрыть кавычки?
Тестов для getFormValue нет.

[AlexeyDsov]

Казалось бы не должно быть raw тута

[pupkinV]

эюя себе! эко я себе в спину ножом. --amend не возбраняется? для чистоты коммитов.

[AlexeyDsov]

со своими бранчами можно делать что хочешь, github почти всегда подцепляет

[suquant]

да и может быть все-е вместо $this->value[0] сделать isset($this->value[0]) ?

[AlexeyDsov]

тут специфичный примитив. по идее если есть массив, то у него обязательно есть 0-ой элемент. а вот raw может быть не так

[AlexeyDsov]

таки фикс на это ждать/не ждать что б мержить можно было?

[pupkinV]

готово, но вот не уверен насчет тестов. я обещался, но переключился на ветку Георгия с миграцией.
мы сможем позже влить ветку с тестами или ждем меня?

[AlexeyDsov]

Замержил. В doc/changelog мне нужно занести автора в формате Ivan I. Ivanov.

[pupkinV]

Vasily Alexandrovich Surikov. Ya pravilno ponyal?

[AlexeyDsov]

Отлично. Чейнджлог поменял и только теперь сообразил что одноименный метод надо бы еще добавить в PlainForm.

[pupkinV]

еще пулл?

[AlexeyDsov]

ну этот уже замержен