Tapestry 5
  1. Tapestry 5
  2. TAP5-725

Last translator in configuration becomes default translator

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Duplicate
    • Affects Version/s: 5.1.0.5
    • Fix Version/s: None
    • Component/s: tapestry-core
    • Labels:
      None

      Description

      I've created simple translator TrimTranslator, which works on String fields. It is same as StringTranslator. Only difference is it removes all white spaces at beginning and end of string. I will attach java source file of it.

      When I've contributed this translator to TranslatorSource service, new translator became default for type String. Every string value incoming from client is now trimmed. It's wrong. I expect new translator will work only when I explicitly declare it for form field component.

      As I see, historically (more than 1 year ago) there was 2 distinct services TranslatorSource and TranslatorDefaultSource. It was clear.
      Later they was joined to one. Why? Current TranslatorSource works fine only if there is one translator for every type or we want to override default translator. There is no possibility to create new alternative translators.

      Proposal of resolution:

      Maybe default translators for type should be distinguished by it's name property, which is unique, not only type?
      Default translators should have name same as name of type. So, "string" is default for java.lang.String, "boolean" is default for java.lang.Boolean, and so on. Any other like "trim" or "yesno" (example in jumpstart app) should need explicit declaration.
      It boils down to replacing in constructor of TranslatorSourceImpl:

      typeToTranslator.put(t.getType(), t);

      with:

      if (t.getType().getSimpleName().equalsIgnoreCase(t.getName())

      { typeToTranslator.put(t.getType(), t); }
      1. TrimTranslator.java
        0.9 kB
        Lukasz Jazgar

        Activity

        Hide
        Igor Drobiazko added a comment -

        This is an expected behavior because you override the mapping String <> StringTranslator by String <> TrimTranslator.
        You cannot define tow different translators for a single field type.

        Your would your proposal match an appropriate translator for a field? Would you use the name of the page property to match the translator? This will not work if you have more than 2 fields of the same type.

        You are misusing the translators. You should use Validators instead. I think we can close the issue.

        Show
        Igor Drobiazko added a comment - This is an expected behavior because you override the mapping String < > StringTranslator by String < > TrimTranslator. You cannot define tow different translators for a single field type. Your would your proposal match an appropriate translator for a field? Would you use the name of the page property to match the translator? This will not work if you have more than 2 fields of the same type. You are misusing the translators. You should use Validators instead. I think we can close the issue.
        Hide
        Lukasz Jazgar added a comment -

        > This is an expected behavior because you override the mapping String <-> StringTranslator by
        > String <-> TrimTranslator.
        > You cannot define tow different translators for a single field type.

        Why?!
        There is "translate" parameter in form fields components. What is it for, if I can't choose translator from among few different?

        Idea of translators, as I remember from Tapestry 4, assumes that you can write translator and turn it on for chosen fields. How it can be done, if there is only one translator for type?

        For example, it could be written such translators,
        For String:
        trim - removing whitespaces at beginning and end (only for client->property direction)
        upper - making value uppercase (only for client->property direction)
        lower - making value lowercase (only for client->property direction)
        iban - IBAN - International Bank Account Number. For direction client->property removing all whitespaces. For direction property->client, adding spaces after each 4 digits.

        For Double and Float:
        fractionDigits=n- For direction client->property rounding number to have n fraction digits. For opposite direction, formatting number by DecimalFormat "##0.00"

        And so on, and so on.

        Year ago (before 07.02.2008), it was correct. There was 2 separate mappings:
        1. type -> Translator - meaning "default translator for type", managed by TranslatorDefaultSource
        2. name ->Translator - used in case of using translate parameter, managed by TranslatorSource
        Now, both of them are managed by TranslatorSource, but there is no possibility to change one without influence to the other.

        > Your would your proposal match an appropriate translator for a field?
        No. My proposal only assumes simple rule - default translator for type String is that with name "string".
        It is only idea, which comes to my brain, correcting this without changing API.

        > You are misusing the translators. You should use Validators instead.
        I disagree. Validators are to check constraints. What I want to do, is define mapping between client view/input and model and this is work for Translators.

        Show
        Lukasz Jazgar added a comment - > This is an expected behavior because you override the mapping String <-> StringTranslator by > String <-> TrimTranslator. > You cannot define tow different translators for a single field type. Why?! There is "translate" parameter in form fields components. What is it for, if I can't choose translator from among few different? Idea of translators, as I remember from Tapestry 4, assumes that you can write translator and turn it on for chosen fields. How it can be done, if there is only one translator for type? For example, it could be written such translators, For String: trim - removing whitespaces at beginning and end (only for client->property direction) upper - making value uppercase (only for client->property direction) lower - making value lowercase (only for client->property direction) iban - IBAN - International Bank Account Number. For direction client->property removing all whitespaces. For direction property->client, adding spaces after each 4 digits. For Double and Float: fractionDigits=n- For direction client->property rounding number to have n fraction digits. For opposite direction, formatting number by DecimalFormat "##0.00" And so on, and so on. Year ago (before 07.02.2008), it was correct. There was 2 separate mappings: 1. type -> Translator - meaning "default translator for type", managed by TranslatorDefaultSource 2. name ->Translator - used in case of using translate parameter, managed by TranslatorSource Now, both of them are managed by TranslatorSource, but there is no possibility to change one without influence to the other. > Your would your proposal match an appropriate translator for a field? No. My proposal only assumes simple rule - default translator for type String is that with name "string". It is only idea, which comes to my brain, correcting this without changing API. > You are misusing the translators. You should use Validators instead. I disagree. Validators are to check constraints. What I want to do, is define mapping between client view/input and model and this is work for Translators.
        Hide
        Bryan Lewis added a comment -

        I had a similar request and HLS suggested it was a JIRA-worthy bug. See thread:

        http://www.mail-archive.com/users@tapestry.apache.org/msg39587.html

        I too would like to be able to specify a translator on a per-field basis, as I was able to do in Tap 3 and 4. At the moment I work around it by declaring an extrr wrapper class like PhoneNumberHolder or CurrencyHolder, where I stuff my value at the start of the page and extract it at submit. The distinct class gives me something to bind the translator to.

        Show
        Bryan Lewis added a comment - I had a similar request and HLS suggested it was a JIRA-worthy bug. See thread: http://www.mail-archive.com/users@tapestry.apache.org/msg39587.html I too would like to be able to specify a translator on a per-field basis, as I was able to do in Tap 3 and 4. At the moment I work around it by declaring an extrr wrapper class like PhoneNumberHolder or CurrencyHolder, where I stuff my value at the start of the page and extract it at submit. The distinct class gives me something to bind the translator to.
        Hide
        Geoff Callender added a comment -

        I've just had the same issue when I tried to implement a PhoneNumberTranslator but as described above it overrode the StringTranslator globally.

        The aim of my PhoneNumberTranslator is to parse all sorts of client input, removing spaces, hyphens, brackets etc. into a standard format for saving to the DB. On output it formats the phone number based on the rules of the country, adding spaces etc. as required. IMHO this is the role of a Translator, not a Validator.

        As a hack workaround, I followed Robert Zeigler's suggestion to register my translator as a CharSequence translator rather than a String translator. It works, but it's clearly a hack. See http://thread.gmane.org/gmane.comp.java.tapestry.user/72777/focus=72821

        I think we should be able to contribute any number of translators without fear of them globally overriding any others.

        Show
        Geoff Callender added a comment - I've just had the same issue when I tried to implement a PhoneNumberTranslator but as described above it overrode the StringTranslator globally. The aim of my PhoneNumberTranslator is to parse all sorts of client input, removing spaces, hyphens, brackets etc. into a standard format for saving to the DB. On output it formats the phone number based on the rules of the country, adding spaces etc. as required. IMHO this is the role of a Translator, not a Validator. As a hack workaround, I followed Robert Zeigler's suggestion to register my translator as a CharSequence translator rather than a String translator. It works, but it's clearly a hack. See http://thread.gmane.org/gmane.comp.java.tapestry.user/72777/focus=72821 I think we should be able to contribute any number of translators without fear of them globally overriding any others.
        Hide
        Howard M. Lewis Ship added a comment -

        This issue is a side effect of the larger issue: that there isn't a proper way to override built-in Translators (TAP5-1120)

        Show
        Howard M. Lewis Ship added a comment - This issue is a side effect of the larger issue: that there isn't a proper way to override built-in Translators ( TAP5-1120 )

          People

          • Assignee:
            Howard M. Lewis Ship
            Reporter:
            Lukasz Jazgar
          • Votes:
            4 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development