I've created a jira and fixed the issue: https://issues.apache.org/jira/browse/WICKET-5916
Martijn On Fri, May 29, 2015 at 6:27 PM, Martijn Dashorst <martijn.dasho...@gmail.com> wrote: > Apparently this change breaks our application after all: a > StackOverflowError trying to load a ResourceModel from a DDC's > "appendOptionHtml" method. > > JaNeeCombobox.getResourcePostfix() line: 106 > CobraLocalizer.getCacheKey(String, Component, Locale, String, String) line: 24 > CobraLocalizer(Localizer).getStringIgnoreSettings(String, Component, > IModel<?>, Locale, String, String) line: 369 > CobraLocalizer(Localizer).getString(String, Component, IModel<?>, > Locale, String, IModel<String>) line: 232 > ResourceModel$AssignmentWrapper.load() line: 132 > ResourceModel$AssignmentWrapper.load() line: 1 > ResourceModel$AssignmentWrapper(LoadableDetachableModel<T>).getObject() > line: 120 > JaNeeCombobox.getResourcePostfix() line: 114 > CobraLocalizer.getCacheKey(String, Component, Locale, String, String) line: 24 > CobraLocalizer(Localizer).getStringIgnoreSettings(String, Component, > IModel<?>, Locale, String, String) line: 369 > CobraLocalizer(Localizer).getString(String, Component, IModel<?>, > Locale, String, IModel<String>) line: 232 > CobraLocalizer(Localizer).getString(String, Component, IModel<?>, > Locale, String, String) line: 201 > CobraLocalizer(Localizer).getString(String, Component, String) line: 150 > JaNeeCombobox$JaNeeRenderer.getDisplayValue(Boolean) line: 84 > JaNeeCombobox$JaNeeRenderer.getDisplayValue(Object) line: 1 > JaNeeCombobox(AbstractChoice<T,E>).appendOptionHtml(AppendingStringBuffer, > E, int, String) line: 409 > > This worked until this change came in. It appears that the LDM should > be extended with an additional flag "attaching" to break load cycles. > While there's probably an issue in our code base, this likely will > cause issues for others upgrading as well. > > Martijn > > > > On Mon, Nov 24, 2014 at 11:39 PM, Martijn Dashorst > <martijn.dasho...@gmail.com> wrote: >> It appears innocent enough this change, but we can't be sure this >> won't break applications: isAttached() is a public method and it is >> valid to call inside load(). If someone depends on isAttached() during >> load() we will change this behavior. >> >> Granted this new semantics are much more to my liking, and it fixes a >> bug. But does this justify the risk of breaking applications in other >> ways? >> >> I checked some of our projects for such mishaps but couldn't find any >> misuses of isAttached inside load(). >> >> Martijn >> >> >> On Mon, Nov 24, 2014 at 9:11 PM, Sven Meier (JIRA) <j...@apache.org> wrote: >>> >>> [ >>> https://issues.apache.org/jira/browse/WICKET-5772?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel >>> ] >>> >>> Sven Meier resolved WICKET-5772. >>> -------------------------------- >>> Resolution: Fixed >>> Fix Version/s: 6.19.0 >>> 7.0.0-M5 >>> >>> #attached is now set after calling #load(), as suggested. >>> >>> Thanks Martin! >>> >>>> LoadableDetachableModel caches null value if load() fails, bug in >>>> getObject() {attached = true;} >>>> ------------------------------------------------------------------------------------------------ >>>> >>>> Key: WICKET-5772 >>>> URL: https://issues.apache.org/jira/browse/WICKET-5772 >>>> Project: Wicket >>>> Issue Type: Bug >>>> Components: wicket >>>> Affects Versions: 1.4.23, 6.18.0, 7.0.0-M4 >>>> Reporter: Martin Makundi >>>> Assignee: Sven Meier >>>> Labels: easyfix, easytest, patch >>>> Fix For: 7.0.0-M5, 6.19.0 >>>> >>>> Original Estimate: 10m >>>> Remaining Estimate: 10m >>>> >>>> If you have a LoadableDetachableModel whose load() operation fails at an >>>> instance, the LoadableDetachableModel will cache null value because >>>> getObject() method sets attached = true; before load() invocation has >>>> returned. >>>> This results in methods trusting LoadableDetachableModel using the null >>>> value for their operations which is logically incorrect as null might not >>>> be a legal value at all. Such behavior would result in unexpected >>>> difficult-to-debug behavior in depending components. >>>> Easy fix: >>>> Move: >>>> attached = true; >>>> after line: >>>> transientModelObject = load(); >>>> Test case: >>>> /** >>>> * LoadableDetachableModel must not return null as an attached value if >>>> load() >>>> * fails. >>>> */ >>>> public void testWhenLoadFails() { >>>> final LoadableDetachableModel<Void> loadableDetachableModel = new >>>> LoadableDetachableModel<Void>() { >>>> /** >>>> * @see org.apache.wicket.model.LoadableDetachableModel#load() >>>> */ >>>> @Override >>>> protected Void load() { >>>> throw new UnsupportedOperationException("Let's assume this fails >>>> for some reason."); >>>> } >>>> }; >>>> try { >>>> loadableDetachableModel.getObject(); >>>> fail(UnsupportedOperationException.class + " expected."); >>>> } catch (final UnsupportedOperationException e) { >>>> >>>> LoggerFactory.getLogger(LoadableDetachableModel.class).debug("Expected >>>> behavior due to " + UnsupportedOperationException.class); >>>> } >>>> try { >>>> assertNotSame(LoadableDetachableModel.class + " should not return >>>> null if it's load() has failed\n", null, >>>> loadableDetachableModel.getObject()); >>>> } catch (final UnsupportedOperationException e) { >>>> >>>> LoggerFactory.getLogger(LoadableDetachableModel.class).debug("Expected >>>> behavior due to " + UnsupportedOperationException.class); >>>> } >>>> } >>> >>> >>> >>> -- >>> This message was sent by Atlassian JIRA >>> (v6.3.4#6332) >> >> >> >> -- >> Become a Wicket expert, learn from the best: http://wicketinaction.com > > > > -- > Become a Wicket expert, learn from the best: http://wicketinaction.com -- Become a Wicket expert, learn from the best: http://wicketinaction.com
