• Type: Improvement
    • Resolution: Fixed
    • Priority: Minor
    • 1.0.1
    • Affects Version/s: 1.0.0
    • Component/s: ymir-core
    • None

      【概要】
      Date型の入力項目プロパティにおいて、
      画面上のテキストボックスでは何も入力しない状態でPostbackしたときに、
      Pageクラスの該当プロパティのSetterが呼び出されない。
      そのとき該当プロパティのリクエスト値は空である。

      Conversationで値を持ち回しているときに、
      一度、画面入力した値をPostbackしてConversationで保持して、
      再度、画面ではその値を空にしてPostbackすると、Date型の場合は
      Setterを呼び出さないため、Conversationの値が再度復元されてしまう。

      通常これは、あまり問題にはならないが、
      String型では、リクエスト値が空でもSetterが呼び出されて、
      Conversationの値を無視するが、Date型だと違う挙動になるので、
      このギャップが何かしらの不具合を生まないか心配である。

      例えば、入力画面と確認画面があったときに、
      入力画面には日付型のプロパティが存在しているとして:
      1. 入力画面で日付入力して確認画面へ
      2. Conversationで保持
      3. 確認画面で日付が間違っていることを確認して入力画面へ
      4. 入力画面で日付を空っぽに(日付は入れなくていいやと考え直したとする)
      5. 確認画面へ行くと、Conversation保持したさっきの値が復元される
      など
      (試してないので想像の域ですが、このような心配があります)

          [YMIR-296] *[ymir-core] Date型の入力項目プロパティでリクエスト値が空の時にSetterが呼ばれない

          skirnir added a comment -

          確認ありがとうございました。closeとします。

          skirnir added a comment - 確認ありがとうございました。closeとします。

          jflute added a comment -

          確認しました。Setterが呼ばれました。
          逆にpopulateWhereNull=falseとすることで呼ばれないことも確認しました。
          ありがとうございます!

          jflute added a comment - 確認しました。Setterが呼ばれました。 逆にpopulateWhereNull=falseとすることで呼ばれないことも確認しました。 ありがとうございます!

          skirnir added a comment -

          リクエストパラメータについては、Stringとそうでない場合の挙動を合わせるため、populateWhereNull=trueの挙動をさせることにしました(r2710)。変更したい場合はpopulateWhereNull=falseと明示的に指定して下さい。

          skirnir added a comment - リクエストパラメータについては、Stringとそうでない場合の挙動を合わせるため、populateWhereNull=trueの挙動をさせることにしました(r2710)。変更したい場合はpopulateWhereNull=falseと明示的に指定して下さい。

          jflute added a comment -

          議論の結果、Setterを呼ぶことに。
          但し、@RequestParameterの要素で挙動を変えられるようにする。

          jflute added a comment - 議論の結果、Setterを呼ぶことに。 但し、@RequestParameterの要素で挙動を変えられるようにする。

          jflute added a comment -

          コメントありがとうございます。

          > 考慮すべきは、どちらのケースが多いか、だと思っています。
          そうですね、PageドリブンのWEBフレームワークで言えば、
          「Pageクラスの該当プロパティのデフォルト値をコンストラクタで指定」
          というのは今までほとんどやったことなく、逆にこれは開発者にはやって
          欲しくない行為であるため、あまり必要性は感じません。
          (やるならもっと明示的にメソッドの処理の中でベタでやって欲しい)

          それよりも
          「@RequestParameterのプロパティのSetterはリクエスト時に
           とにかくリクエストで飛んできた値をセットされる」
          という仕様の方がわかりやすいかなと思います。
          「@RequestParameter」と付いてるからには、リクエストのパラメータが
          そのまま変数に射影されているのが自然かなと。

          というのが自分の意見です。

          ともあれ、そんなに直近で問題が発生する訳ではなさそうなので、
          急ぐ問題では無さそうなので、じっくり考えていく方向でいいと思います。

          jflute added a comment - コメントありがとうございます。 > 考慮すべきは、どちらのケースが多いか、だと思っています。 そうですね、PageドリブンのWEBフレームワークで言えば、 「Pageクラスの該当プロパティのデフォルト値をコンストラクタで指定」 というのは今までほとんどやったことなく、逆にこれは開発者にはやって 欲しくない行為であるため、あまり必要性は感じません。 (やるならもっと明示的にメソッドの処理の中でベタでやって欲しい) それよりも 「@RequestParameterのプロパティのSetterはリクエスト時に  とにかくリクエストで飛んできた値をセットされる」 という仕様の方がわかりやすいかなと思います。 「@RequestParameter」と付いてるからには、リクエストのパラメータが そのまま変数に射影されているのが自然かなと。 というのが自分の意見です。 ともあれ、そんなに直近で問題が発生する訳ではなさそうなので、 急ぐ問題では無さそうなので、じっくり考えていく方向でいいと思います。

          skirnir added a comment -

          バグではないのでissue typeを変更しました。

          skirnir added a comment - バグではないのでissue typeを変更しました。

          skirnir added a comment -

          @RequestParameter(または@Populate)で、値がnullの場合にSetterが呼ばれないのは仕様です。ちなみに@Inも、インジェクトされるものがない場合はSetterが呼ばれません。

          ただし、@InについてはinjectWhereNullプロパティの値をtrueにすることでインジェクトするものがなくてもSetterを呼び出すようにはできますが、@RequestParameterや@Populateには対応するプロパティは存在しませんでした。そこで、まずは@RequestParameterと@PopulateにpopulateWhereNullプロパティを追加しました。

          というわけで、@RequestParameter(populateWhereNull=true)とすれば、リクエストパラメータが空文字であってもDateプロパティのSetterが呼び出されるようになります。

          しかしながらその一方で、「リクエストパラメータが空文字の場合、Setterを呼び出すべきか?」というディスカッションをする必要があると思いました。

          Conversationの使い方によっては久保さんの言うように「空文字列でもSetterを呼び出す」という挙動が便利なこともありますが、「値が指定されなかった場合はデフォルト値を使う」という処理を書く場合は、例えばPageクラスの該当プロパティのデフォルト値をコンストラクタで指定しておけば、(リクエストパラメータが空文字列の場合にSetterが呼ばれずに)そのままデフォルト値を使うことができるので実装が楽になります。

          考慮すべきは、どちらのケースが多いか、だと思っています。意見を聞かせてください。

          ちなみにYmir流実装では、formに対応するDtoを生成し、ConversationにはモデルとなるBean(DBFluteのEntityなど)をバインドしておき、サブミット時にConverterを使ってDtoからBeanに値をコピーするというやり方を取ります。こうすると、今回のような問題は発生しません(ConverterはDtoのプロパティ値がnullでもそれをBeanにセットするので、もともとのBeanの値が残ってしまうということは発生しない)。

          skirnir added a comment - @RequestParameter(または@Populate)で、値がnullの場合にSetterが呼ばれないのは仕様です。ちなみに@Inも、インジェクトされるものがない場合はSetterが呼ばれません。 ただし、@InについてはinjectWhereNullプロパティの値をtrueにすることでインジェクトするものがなくてもSetterを呼び出すようにはできますが、@RequestParameterや@Populateには対応するプロパティは存在しませんでした。そこで、まずは@RequestParameterと@PopulateにpopulateWhereNullプロパティを追加しました。 というわけで、@RequestParameter(populateWhereNull=true)とすれば、リクエストパラメータが空文字であってもDateプロパティのSetterが呼び出されるようになります。 しかしながらその一方で、「リクエストパラメータが空文字の場合、Setterを呼び出すべきか?」というディスカッションをする必要があると思いました。 Conversationの使い方によっては久保さんの言うように「空文字列でもSetterを呼び出す」という挙動が便利なこともありますが、「値が指定されなかった場合はデフォルト値を使う」という処理を書く場合は、例えばPageクラスの該当プロパティのデフォルト値をコンストラクタで指定しておけば、(リクエストパラメータが空文字列の場合にSetterが呼ばれずに)そのままデフォルト値を使うことができるので実装が楽になります。 考慮すべきは、どちらのケースが多いか、だと思っています。意見を聞かせてください。 ちなみにYmir流実装では、formに対応するDtoを生成し、ConversationにはモデルとなるBean(DBFluteのEntityなど)をバインドしておき、サブミット時にConverterを使ってDtoからBeanに値をコピーするというやり方を取ります。こうすると、今回のような問題は発生しません(ConverterはDtoのプロパティ値がnullでもそれをBeanにセットするので、もともとのBeanの値が残ってしまうということは発生しない)。

          jflute added a comment -

          優先度は「低」です。

          jflute added a comment - 優先度は「低」です。

          jflute added a comment -

          > 1. 入力画面で日付入力して確認画面へ
          > 2. Conversationで保持
          > 3. 確認画面で日付が間違っていることを確認して入力画面へ
          > 4. 入力画面で日付を空っぽに(日付は入れなくていいやと考え直したとする)
          > 5. 確認画面へ行くと、Conversation保持したさっきの値が復元される

          に関しては、実際に試してみて発生しましたが、
          @InのActionを指定して調整することで回避はできました。

          jflute added a comment - > 1. 入力画面で日付入力して確認画面へ > 2. Conversationで保持 > 3. 確認画面で日付が間違っていることを確認して入力画面へ > 4. 入力画面で日付を空っぽに(日付は入れなくていいやと考え直したとする) > 5. 確認画面へ行くと、Conversation保持したさっきの値が復元される に関しては、実際に試してみて発生しましたが、 @InのActionを指定して調整することで回避はできました。

          jflute added a comment -

          恐らくIntegerでも同じ話かと思います。

          またBooleanでも同じ話ですが、CheckBoxはさらにややこしくて、
          OFFのときリクエストパラメータ自体が飛んでこないんですよね...

          jflute added a comment - 恐らくIntegerでも同じ話かと思います。 またBooleanでも同じ話ですが、CheckBoxはさらにややこしくて、 OFFのときリクエストパラメータ自体が飛んでこないんですよね...

            Assignee:
            skirnir
            Reporter:
            jflute
            Votes:
            0 Vote for this issue
            Watchers:
            Start watching this issue

              Created:
              Updated:
              Resolved: