Dropdowns in JSF: Validating the selected value

Pete, WDYM it isn't a great a idea?

It /is/ a great idea to override equals() and hashCode() on all entities, to compare business identity. In fact this is the recommended best practice...

It's definitely the second choice if you can have a persistence context that can do the work of identifying things for you instead. One less thing that can go wrong - and writing a correct equals() and hashCode() method is still a challenge for many Java devs. You would be surprised how many people I see in Hibernate trainings who never had to touch these methods.

Gavin, this was largely based on discussions with Christian (who reasons I won't repeat). If overriding equals() is best practice then we don't have an issue here...

IMO, it is still best practice.

I don't think Java developers are too stupid to be able to write a correct equals() method holds much water here. Correct equals() and hashCode() methods are a basic requirement of the Java collections framework, and it's important that people understand the importance of them. My experience is that once you explain the concept of a /business key/, most folks find it pretty intuitive.

On average, I'd say that half of the room in a typical Hibernate training does not know the default implementation of java.lang.Object#equals(). They are much happier with a persistence context identity guarantee.

Anyway, the problem I had with the validation that is enforced by the JSF spec is not entity equality. The problem is that I have some kind of DTO that I display in a selection list and I use my own converter to convert it to a string. And yes, I can convert it back to an object from that string but I really need to create a new instance at that time. I don't see why I should not be allowed to do that.

It is great to read about this topic here, since it is a very disputed thing at my company. Generally I think it makes sense to have a business key - if it is that easy to identify!

But what happens if you have an entity with lots of relationships and FKs? It is very hard to identify the business key relevant fields at such a place. /(Does the fifth Hashset relationship of this entity really identify it? Can it be left out? But is it a real equals() method if it does not compare everything?)/

I think you should either go for it and implement equals() and hashcode() for all of the entities, or dont do it at all and look if the PContext-magic under the hood does the trick for you (with those specific entities).

Any good strategies that others successfully applied?

1) Overriding equals() and hashCode() is a good idea only if you have business keys that never change, yet uniquely identify the entity. In practice I found very few entities satisfy this requirement. See a longer discussion here: http://hibernate.org/109.html

2) Ultimately a Hibernate session resp. JPA persistence context defines object identity which is another reason not to override those methods.

... http://hibernate.org/109.html

2) Ultimately the Hibernate session / JPA persistence context defines object identity (for DB related operations), so that 's another reason not to override equals() and hashCode().

A workaround:

/**
 * Enhances the Seam entity converter and avoids "Value is not valid" validation errors by
 * not only decoding a requested entity identifier and loading that entity, but also returning
 * the matching (configurable) original instance from the select list. This solves the following
 * problem if you are not using a long-running CONVERSATION:
 * <p/>
 * <a href="http://in.relation.to/2708.lace">Dropdowns in JSF: Validating the selected value</a>
 *
 * @author Christian Bauer
 */
@Name("matchingEntityConverter")
@Scope(ScopeType.CONVERSATION)
@Install(precedence = Install.APPLICATION)
@Converter
@BypassInterceptors
public class MatchingEntityConverter implements javax.faces.convert.Converter, Serializable {

    private AbstractEntityLoader entityLoader;

    public AbstractEntityLoader getEntityLoader() {
        if (entityLoader == null) {
            return AbstractEntityLoader.instance();
        } else {
            return entityLoader;
        }
    }

    public void setEntityLoader(AbstractEntityLoader entityLoader) {
        this.entityLoader = entityLoader;
    }

    @SuppressWarnings("unchecked")
    @Transactional
    public String getAsString(FacesContext facesContext, UIComponent cmp, Object value) throws ConverterException {
        if (value == null) {
            return null;
        }
        if (value instanceof String) {
            return (String) value;
        }
        return getEntityLoader().put(value);
    }


    @Transactional
    public Object getAsObject(FacesContext facesContext, UIComponent cmp, String value) throws ConverterException {
        if (value == null) {
            return null;
        }
        Object loadedInstance = getEntityLoader().get(value);

        // THIS IS THE EXTENSION THAT MAKES THIS SPECIAL:

        // We will try to return the _original_value from the _original_ list, instead of the instance that
        // we just loaded from the database. This makes validation work, because equals() will be called on the
        // returned instance and it will be compared to all items in the list. If the instance we just loaded
        // is therefore not equal() to an item of the list, we'd get a "value is not valid" error. Returning one
        // of the instances from the list avoids that. We implement our own matching routine here, configurable.

        Object equalListInstance = null;
        try {

            Object loadedPropertyValue = null;
            if (loadedInstance != null) {
                loadedPropertyValue = readMatchingProperty(loadedInstance);
            }

            if (loadedPropertyValue != null) {

                Set originalList = getOriginalSelectItems(cmp);
                for (Object listInstance : originalList) {

                    Object listItemPropertyValue = readMatchingProperty(listInstance);
                    if (listItemPropertyValue != null && loadedPropertyValue.equals(listItemPropertyValue)) {
                        equalListInstance = listInstance;
                        break;
                    }
                }
            }
        } catch (Exception ex) {
            throw new ConverterException(ex);
        }

        // Fall back to loadedInstance.equals(<for each listInstance>) which is what the JSF RI implementors
        // do in their JPA-unfriendly validation routine that will run after this...
        return equalListInstance != null ? equalListInstance : loadedInstance;
    }

    protected Object readMatchingProperty(Object o) throws Exception {
        PropertyDescriptor[] props = Introspector.getBeanInfo(o.getClass()).getPropertyDescriptors();
        for (PropertyDescriptor descriptor : props) {
            if (descriptor.getName().equals(getMatch())) {
                return descriptor.getReadMethod().invoke(o);
            }
        }
        return null;
    }

    protected Set getOriginalSelectItems(UIComponent cmp) {
        Set items = new HashSet();
        // This is really ugly, but what can you do...
        for (UIComponent child : cmp.getChildren()) {
            if (child instanceof javax.faces.component.UISelectItems) {
                javax.faces.component.UISelectItems selectItems = (javax.faces.component.UISelectItems) child;
                for (Object selectItemsValue : (Collection) selectItems.getValue()) {
                    javax.faces.model.SelectItem selectItem = (javax.faces.model.SelectItem) selectItemsValue;
                    if (selectItem.getValue() == null) continue;
                    items.add(selectItem.getValue());
                }
                break;
            }
        }
        return items;
    }

    private String match;

    public String getMatch() {
        if (match == null) {
            return "id"; // Most entities would have that property
        }
        return match;
    }

    public void setMatch(String match) {
        this.match = match;
    }
}

Put this in your components.xml:

<component name="identifierMatchingEntityConverter"
           class="common.web.MatchingEntityConverter">
    <property name="match">id</property> <!-- That's actually the default property we'd use for comparison-->
</component>

Use it like this in your templates:

<h:selectOneMenu value="#{browseProducts.selectedProduct}"
                 converter="#{identifierMatchingEntityConverter}"> <!-- The magic that makes it work! -->
    <s:selectItems value="#{browseProducts.list}"
                   var="p"
                   label="#{skeleton:truncateString(p.name, 15, '...')}"
                   noSelectionLabel="NO PRODUCT SELECTED">
    </s:selectItems>
    <a:support event="onchange" reRender="productSelectOutputDecorate" status="globalStatus"/>
</h:selectOneMenu>

Just so this is clear: The browseProduct backing bean is in PAGE scope. This new converter makes it work without a long-running conversation scope and with two different persistence contexts before and after the submit.