TextChoiceProvider - getId() value not returned in toChoices()

[vertex-github]

Is my understanding correct that any value returned in TextChoiceProvider.getId() can be any of the types that org.json.JSONWriter will work with (including an Object that implements JSONString) - I looked at the code and this seems to be the case.

My getId() method is invoked for an Entity object, and I return the String.ID listed below:

2014-04-03 12:50:07,244 INFO  [qtp2023938592-124] com.SendPage - EmailAddressProvider.getId(): Object: EmailAddressHolder{[email protected], isContact=true}, StringId: {"email":{"emailAddress":"[email protected]"},"isContact":true}

Later on, I get a callback to my TextChoiceProvider.toChoices() with the following:

2014-04-03 12:50:09,920 INFO  [qtp2023938592-127] com.SendPage - EmailAddressProvider.toChoices : StringId from MultiChoice: {"email":{"emailAddress":"[email protected]"}
2014-04-03 12:50:09,920 INFO  [qtp2023938592-127] com.SendPage - EmailAddressProvider.toChoices : StringId from MultiChoice: "isContact":true}

It looks like the MultiChoice has split the JSON String - this obviously prevents correct Entity object construction since the JSON is corrupt. Is MultiChoice intended to work with JSON strings as IDs?

[vertex-github]

Relevant methods in EmailAddressProvider extends TextChoiceProvider<EmailAddressHolder>

        @Override
        public Collection<EmailAddressHolder> toChoices( Collection<String> ids )
        {
            for( String id : ids )
            {
                log.info( "EmailAddressProvider.toChoices : StringId from MultiChoice: {}", id );
            }

            Gson gson = new Gson();
            List<EmailAddressHolder> choices = new ArrayList<>();
            for( String id : ids )
            {
                EmailAddressHolder holder = gson.fromJson( id, EmailAddressHolder.class );
                choices.add( holder );
            }

            return choices;
        }

        @Override
        protected Object getId( EmailAddressHolder choice )
        {
            Gson gson = new Gson();
            String jsonString = gson.toJson( choice );
            log.info( "EmailAddressProvider.getId(): Object: {}, StringId: {}", choice, jsonString );
            return jsonString;
        }

[vertex-github]

I see the problem:

Select2MultiChoice.java, line 58:

    @Override
    protected void convertInput() {

    String input = getWebRequest().getRequestParameters().getParameterValue(getInputName()).toString();

    final Collection<T> choices;
    if (Strings.isEmpty(input)) {
        choices = new ArrayList<T>();
    } else {
        choices = getProvider().toChoices(Arrays.asList(input.split(",")));
    }

    setConvertedInput(choices);
    }

Could this be changed to check for JSON objects, and then split between valid objects?

[tgoetz]

Patches or pull requests are always welcome ;-)

[tgoetz]

But: can't you provide an (unique) id value for your EmailAddressHolder that enabled reconstruction in toChoices()? Meaning: where do you keep your list of EmailAddressHolders? Wouldn't it be enough to provide e.g. the list index as id?

[vertex-github]

Unfortunately not in this case as the source for the email addresses is coming from two sources - a list of the Users contacts, and a list of emails that the User has sent things to through the platform. The two groups of emails are unrelated. Ive already forked and Im testing now.

[tgoetz]

ListUtils.union(list1, list2); ?

[vertex-github]

One list is obtained from the Contact domain Entity (which contains email addresses, amongst other things). The other list is obtained from a list of emails that the user has sent things to. While they are stored in the DB, they are unrelated to the Contact entity. I do however form a single List of 'holder' objects that I pass to the Select2MultiChoice via the IModel. That holder object needs to store Entity.ID + Entity.class (or a boolean flag stating that the ID belongs Contact vs Recipient) in the MultiChoice ID so that I can recover the correct entity in toChoices(). A simple String.split(",") doesn't work since the JSON for an Object with more than 1 field has a comma in it.

[tgoetz]

So then, why can't you use the list index of your list of holder objects to generate the id?

public abstract class MyChoiceProvider<T> extends TextChoiceProvider<T>
{
  private IModel<? extends List<? extends T>> choices;

  public MyChoiceProvider(IModel<? extends List<? extends T>> choices)
  {
    this.choices = choices;
  }

  @Override
  protected String getDisplayText(T choice)
  {
    return choice.toString();
  }

  @Override
  protected Object getId(T choice)
  {
    return choices.getObject().indexOf(choice);
  }

  @Override
  public Collection<T> toChoices(Collection<String> ids)
  {
    List<T> result = new ArrayList<>(ids.size());
    for (String id : ids) {
      result.add(choices.getObject().get(Integer.valueOf(id)));
    }
    return result;
  }

}

[vertex-github]

Thanks for the suggestion. This might work but I'm wary of storing index IDs in the client - I've got a two way component sync going on here - the Select2MultiChoice and a popup AddressBook (which updates to show which emails have been selected). I just submitted a pull request to support JSON String IDs, but Ill keep your suggestion in mind.

Fix offered: https://github.com/ivaynberg/wicket-select2/pull/87

[tgoetz]

Wicket does the same: storing the id of the list index in the client, see org.apache.wicket.markup.html.form.ChoiceRenderer#getIdValue. I can't see anything wrong with that approach, what is your concern?

[vertex-github]

If the list indexes are stored on the client, and the list of emails / recipients / contacts is changed on the server (via another window/tab), then the list index as stored in the client will no longer correspond to the correct email address (unless Im missing something here, its safer to store the email address and the flag on the client)

[tgoetz]

Ok, I see your point. Are the email addresses unique? You could use those as id then.