you are right. That is what the javadocs says. Besides what the javadocs says, silently doing nothing happened with no indication that nothing happened, is poor design. You'll end up with if(ART.get() != null) updateTree(). We should return a boolean saying we did nothing and add a parameter throwException (if false return true. If true throw an exception).
On Thu, Jun 23, 2011 at 4:48 PM, Martin Grigorov <[email protected]> wrote: > I'm also not acquaint with Tree classes but this part of > #updateTree()'s javadoc makes me this this was the initial design: > "However, it is also safe to call this method outside Ajax response." > > Now it is not safe. > I think the problem in this ticket is that until today a wrong way to > get the ART was used. Now it is better. > Otherwise the method's javadoc explains it: if this is not Ajax > request it is no-op. > > On Thu, Jun 23, 2011 at 5:37 PM, Juergen Donnerstag > <[email protected]> wrote: >> but the previous behavior didn't make any sense. Silently do nothing >> if no or a wrong request target is provide?!?! updateTree() should be >> quick for updateTree(ART.get()). But ART.get() may return false if no >> ART is available. And that obviously is an error that should be >> reported. >> >> Juergen >> >> On Thu, Jun 23, 2011 at 3:43 PM, Martin Grigorov <[email protected]> >> wrote: >>> Hm. This changed previous behavior. >>> I think we should preserve the old behavior or remove updateTree() at >>> all. The user can use updateTree(ART.get()) which will do nothing if >>> ART is null. >>> >>> On Thu, Jun 23, 2011 at 4:36 PM, <[email protected]> wrote: >>>> Author: jdonnerstag >>>> Date: Thu Jun 23 13:36:05 2011 >>>> New Revision: 1138873 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=1138873&view=rev >>>> Log: >>>> fixed: AbstractTree.updateTree() method not works >>>> Issue: WICKET-3818 >>>> >>>> Modified: >>>> >>>> wicket/trunk/wicket-core/src/main/java/org/apache/wicket/markup/html/tree/AbstractTree.java >>>> >>>> Modified: >>>> wicket/trunk/wicket-core/src/main/java/org/apache/wicket/markup/html/tree/AbstractTree.java >>>> URL: >>>> http://svn.apache.org/viewvc/wicket/trunk/wicket-core/src/main/java/org/apache/wicket/markup/html/tree/AbstractTree.java?rev=1138873&r1=1138872&r2=1138873&view=diff >>>> ============================================================================== >>>> --- >>>> wicket/trunk/wicket-core/src/main/java/org/apache/wicket/markup/html/tree/AbstractTree.java >>>> (original) >>>> +++ >>>> wicket/trunk/wicket-core/src/main/java/org/apache/wicket/markup/html/tree/AbstractTree.java >>>> Thu Jun 23 13:36:05 2011 >>>> @@ -33,6 +33,7 @@ import javax.swing.tree.TreeNode; >>>> >>>> import org.apache.wicket.Component; >>>> import org.apache.wicket.MarkupContainer; >>>> +import org.apache.wicket.WicketRuntimeException; >>>> import org.apache.wicket.ajax.AjaxRequestTarget; >>>> import org.apache.wicket.behavior.Behavior; >>>> import org.apache.wicket.markup.ComponentTag; >>>> @@ -45,10 +46,10 @@ import org.apache.wicket.markup.html.pan >>>> import org.apache.wicket.model.IDetachable; >>>> import org.apache.wicket.model.IModel; >>>> import org.apache.wicket.model.Model; >>>> -import org.apache.wicket.request.IRequestHandler; >>>> import org.apache.wicket.request.Response; >>>> import org.apache.wicket.request.resource.JavaScriptResourceReference; >>>> import org.apache.wicket.request.resource.ResourceReference; >>>> +import org.apache.wicket.util.lang.Args; >>>> import org.apache.wicket.util.string.AppendingStringBuffer; >>>> import org.apache.wicket.util.visit.IVisit; >>>> import org.apache.wicket.util.visit.IVisitor; >>>> @@ -67,10 +68,6 @@ public abstract class AbstractTree exten >>>> TreeModelListener, >>>> AjaxRequestTarget.ITargetRespondListener >>>> { >>>> - >>>> - /** >>>> - * >>>> - */ >>>> private static final long serialVersionUID = 1L; >>>> >>>> /** >>>> @@ -1001,21 +998,6 @@ public abstract class AbstractTree exten >>>> } >>>> >>>> /** >>>> - * Convenience method that updates changed portions on tree. You >>>> can call this method during >>>> - * Ajax response, where calling {@link >>>> #updateTree(AjaxRequestTarget)} would be appropriate, but >>>> - * you don't have the AjaxRequestTarget instance. However, it is >>>> also safe to call this method >>>> - * outside Ajax response. >>>> - */ >>>> - public final void updateTree() >>>> - { >>>> - IRequestHandler handler = >>>> getRequestCycle().getActiveRequestHandler(); >>>> - if (handler instanceof AjaxRequestTarget) >>>> - { >>>> - updateTree((AjaxRequestTarget)handler); >>>> - } >>>> - } >>>> - >>>> - /** >>>> * Allows to intercept adding dirty components to AjaxRequestTarget. >>>> * >>>> * @param target >>>> @@ -1124,6 +1106,24 @@ public abstract class AbstractTree exten >>>> } >>>> >>>> /** >>>> + * Convenience method that updates changed portions on tree. You >>>> can call this method during >>>> + * Ajax response, where calling {@link >>>> #updateTree(AjaxRequestTarget)} would be appropriate, but >>>> + * you don't have the AjaxRequestTarget instance. However, it is >>>> also safe to call this method >>>> + * outside Ajax response. >>> This comment is no more valid. >>>> + */ >>>> + public final void updateTree() >>>> + { >>>> + AjaxRequestTarget handler = AjaxRequestTarget.get(); >>>> + if (handler == null) >>>> + { >>>> + throw new WicketRuntimeException( >>>> + "No AjaxRequestTarget available to execute >>>> updateTree(ART target)"); >>>> + } >>>> + >>>> + updateTree(handler); >>>> + } >>>> + >>>> + /** >>>> * Updates the changed portions of the tree using given >>>> AjaxRequestTarget. Call this method if >>>> * you modified the tree model during an ajax request target and >>>> you want to partially update >>>> * the component on page. Make sure that the tree model has fired >>>> the proper listener functions. >>>> @@ -1135,11 +1135,7 @@ public abstract class AbstractTree exten >>>> */ >>>> public final void updateTree(final AjaxRequestTarget target) >>>> { >>>> - if (target == null) >>>> - { >>>> - return; >>>> - } >>>> - >>>> + Args.notNull(target, "target"); >>>> target.registerRespondListener(this); >>>> } >>>> >>>> >>>> >>>> >>> >>> >>> >>> -- >>> Martin Grigorov >>> jWeekend >>> Training, Consulting, Development >>> http://jWeekend.com >>> >> > > > > -- > Martin Grigorov > jWeekend > Training, Consulting, Development > http://jWeekend.com >
