You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by bitstorm <gi...@git.apache.org> on 2016/05/10 13:08:16 UTC
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
GitHub user bitstorm opened a pull request:
https://github.com/apache/wicket/pull/170
Port of AJAX stateless artifacts from wicketstuff stateless module
Port of AJAX stateless artifacts from wicketstuff stateless module
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bitstorm/wicket ajax-stateless
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/wicket/pull/170.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #170
----
commit 0ca1c0cce91501e7626e4e37d7ae408c856348dc
Author: Andrea Del Bene <an...@innoteam.it>
Date: 2016-04-05T16:43:38Z
Porting of stateless AJAX components to the official project
commit 587440b51d71c71ee64444021bfe8c34249dbcc5
Author: Andrea Del Bene <an...@innoteam.it>
Date: 2016-04-07T09:47:27Z
Integrated examples and unit tests
commit 35f11550c96ebd03c2c7eb7b000b76c68b7be968
Author: Andrea Del Bene <an...@innoteam.it>
Date: 2016-04-07T15:12:28Z
Added javadoc
commit 7364d9effdba03da129629c89e1a0b76067b06d6
Author: Andrea Del Bene <an...@innoteam.it>
Date: 2016-04-08T12:27:28Z
added chapter for stateless AJAX
commit 8950580644d6fb932cea926d2c498cea1daff211
Author: Andrea Del Bene <an...@innoteam.it>
Date: 2016-04-08T16:31:48Z
Minor fix
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on the pull request:
https://github.com/apache/wicket/pull/170#issuecomment-218571214
Overall it is OK, but I don't like that there is so much duplicated code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on the pull request:
https://github.com/apache/wicket/pull/170#issuecomment-221927027
I agree!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/170#discussion_r62914350
--- Diff: wicket-core/src/main/java/org/apache/wicket/ajax/form/StatelessAjaxFormComponentUpdatingBehavior.java ---
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.ajax.form;
+
+import org.apache.wicket.Component;
+import org.apache.wicket.request.Url;
+import org.apache.wicket.request.UrlUtils;
+import org.apache.wicket.request.mapper.parameter.PageParameters;
+
+/**
+ * Stateless version of {@link AjaxFormComponentUpdatingBehavior}.
+ *
+ * @author jfk
+ *
+ */
+public abstract class StatelessAjaxFormComponentUpdatingBehavior
+ extends AjaxFormComponentUpdatingBehavior
+{
+
+ private static final long serialVersionUID = -286307141298283926L;
+
+ /**
+ * @param event
+ */
+ public StatelessAjaxFormComponentUpdatingBehavior(final String event)
+ {
+ super(event);
+ }
+
+ @Override
+ protected void onBind()
+ {
+ super.onBind();
+
+ getComponent().getBehaviorId(this);
+ }
+
+ @Override
+ public CharSequence getCallbackUrl()
+ {
+ final Url url = Url.parse(super.getCallbackUrl().toString());
+ final PageParameters params = getPageParameters();
+
+ return UrlUtils.mergeParameters(url, params).toString();
+ }
--- End diff --
`#onBind()` and `#getCallbackUrl()` are the same as in StatelessAjaxEventBehavior.
Maybe we could introduce an interface with default methods that is mixed into those classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/170#discussion_r62914168
--- Diff: wicket-core/src/main/java/org/apache/wicket/ajax/form/StatelessAjaxFormComponentUpdatingBehavior.java ---
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.ajax.form;
+
+import org.apache.wicket.Component;
+import org.apache.wicket.request.Url;
+import org.apache.wicket.request.UrlUtils;
+import org.apache.wicket.request.mapper.parameter.PageParameters;
+
+/**
+ * Stateless version of {@link AjaxFormComponentUpdatingBehavior}.
+ *
+ * @author jfk
+ *
+ */
+public abstract class StatelessAjaxFormComponentUpdatingBehavior
+ extends AjaxFormComponentUpdatingBehavior
+{
+
+ private static final long serialVersionUID = -286307141298283926L;
+
+ /**
+ * @param event
+ */
+ public StatelessAjaxFormComponentUpdatingBehavior(final String event)
+ {
+ super(event);
+ }
+
+ @Override
+ protected void onBind()
+ {
+ super.onBind();
+
+ getComponent().getBehaviorId(this);
+ }
+
+ @Override
+ public CharSequence getCallbackUrl()
+ {
+ final Url url = Url.parse(super.getCallbackUrl().toString());
+ final PageParameters params = getPageParameters();
+
+ return UrlUtils.mergeParameters(url, params).toString();
+ }
+
+ protected PageParameters getPageParameters()
+ {
+ return null;
--- End diff --
Why not `getComponent().getPageParameters()` ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on the pull request:
https://github.com/apache/wicket/pull/170#issuecomment-218979582
Oh. I didn't pay attention to the destination branch. It is 'wicket-7.x', I thought it is 'master'.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by bitstorm <gi...@git.apache.org>.
Github user bitstorm commented on a diff in the pull request:
https://github.com/apache/wicket/pull/170#discussion_r63567121
--- Diff: wicket-core/src/main/java/org/apache/wicket/ajax/AbstractDefaultAjaxBehavior.java ---
@@ -81,7 +82,27 @@
@Override
protected void onBind()
{
- getComponent().setOutputMarkupId(true);
+ final Component component = getComponent();
+
+ component.setOutputMarkupId(true);
+
+ if (getStatelessHint(component))
+ {
+ //generate behavior id
+ component.getBehaviorId(this);
+ }
+ }
+
+ @Override
+ public CharSequence getCallbackUrl()
+ {
+ if (getStatelessHint(getComponent()))
+ {
+ final Url url = Url.parse(super.getCallbackUrl().toString());
+ return UrlUtils.mergeParameters(url).toString();
--- End diff --
@svenmeier yes you are just right. We should pass page parameters here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by bitstorm <gi...@git.apache.org>.
Github user bitstorm commented on a diff in the pull request:
https://github.com/apache/wicket/pull/170#discussion_r63556050
--- Diff: wicket-core/src/main/java/org/apache/wicket/ajax/AbstractDefaultAjaxBehavior.java ---
@@ -81,7 +82,27 @@
@Override
protected void onBind()
{
- getComponent().setOutputMarkupId(true);
+ final Component component = getComponent();
+
+ component.setOutputMarkupId(true);
+
+ if (getStatelessHint(component))
+ {
+ //generate behavior id
+ component.getBehaviorId(this);
+ }
+ }
+
+ @Override
+ public CharSequence getCallbackUrl()
+ {
+ if (getStatelessHint(getComponent()))
+ {
+ final Url url = Url.parse(super.getCallbackUrl().toString());
+ return UrlUtils.mergeParameters(url).toString();
--- End diff --
yes you are right, I haven't finished yet the refactoring. We can simplify even more moving away getCallbackUrl from AbstractDefaultAjaxBehavior and using it with page parameters only in StatelessAjaxEventBehavior.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by bitstorm <gi...@git.apache.org>.
Github user bitstorm commented on the pull request:
https://github.com/apache/wicket/pull/170#issuecomment-218976648
Hi @martin-g and thank you for your reviw,
as you noted the code contains some classes where code is repeated. I've tried to reduce code duplication but without using Java 8 is nearly impossible. As you mentioned above default methods might help a lot. I'm considering to post-pone this integration to Wicket 8 where I'm free to use Java 8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by svenmeier <gi...@git.apache.org>.
Github user svenmeier commented on a diff in the pull request:
https://github.com/apache/wicket/pull/170#discussion_r63531449
--- Diff: wicket-core/src/main/java/org/apache/wicket/ajax/AbstractDefaultAjaxBehavior.java ---
@@ -81,7 +82,27 @@
@Override
protected void onBind()
{
- getComponent().setOutputMarkupId(true);
+ final Component component = getComponent();
+
+ component.setOutputMarkupId(true);
+
+ if (getStatelessHint(component))
+ {
+ //generate behavior id
+ component.getBehaviorId(this);
+ }
+ }
+
+ @Override
+ public CharSequence getCallbackUrl()
+ {
+ if (getStatelessHint(getComponent()))
+ {
+ final Url url = Url.parse(super.getCallbackUrl().toString());
+ return UrlUtils.mergeParameters(url).toString();
--- End diff --
#mergeParameters() does nothing when no parameters are passed - are the page parameters missing here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by svenmeier <gi...@git.apache.org>.
Github user svenmeier commented on the pull request:
https://github.com/apache/wicket/pull/170#issuecomment-219817138
Actually I'd fine with adding the stateless-handling into AbstractDefaultAjaxBehavior as proposed here.
But I'd rather not add all these Stateless* classes - IMHO it's fine to let users override #isStateless() when they want to support stateless requests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/170#discussion_r62913968
--- Diff: wicket-core/src/main/java/org/apache/wicket/ajax/StatelessOnChangeAjaxBehavior.java ---
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.ajax;
+
+import org.apache.wicket.Component;
+import org.apache.wicket.ajax.attributes.AjaxRequestAttributes;
+import org.apache.wicket.ajax.form.OnChangeAjaxBehavior;
+import org.apache.wicket.ajax.form.StatelessAjaxFormComponentUpdatingBehavior;
+import org.apache.wicket.markup.html.form.TextArea;
+import org.apache.wicket.markup.html.form.TextField;
+
+/**
+ * Stateless version of {@link OnChangeAjaxBehavior}.
+ *
+ * <p>
+ * This behavior uses best available method to track changes on different types of form components.
+ * To accomplish this for text input form components it uses a custom event, named 'inputchange',
+ * that is handled in the best way for the specific browser. For other form component types the
+ * 'change' event is used.
+ * </p>
+ *
+ * @author Andrea Del Bene
+ *
+ */
+public class StatelessOnChangeAjaxBehavior extends StatelessAjaxFormComponentUpdatingBehavior
--- End diff --
Except the parent class this class is an exact copy of `OnChangeAjaxBehavior`.
Can't we extract the common code somewhere and reuse it ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by bitstorm <gi...@git.apache.org>.
Github user bitstorm commented on the pull request:
https://github.com/apache/wicket/pull/170#issuecomment-219660664
@martin-g using inheritance instead of a separate stateless hierarchy reduced the code a lot :-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by bitstorm <gi...@git.apache.org>.
Github user bitstorm closed the pull request at:
https://github.com/apache/wicket/pull/170
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/170#discussion_r62914465
--- Diff: wicket-core/src/main/java/org/apache/wicket/ajax/form/StatelessAjaxFormSubmitBehavior.java ---
@@ -0,0 +1,211 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.ajax.form;
+
+import org.apache.wicket.Component;
+import org.apache.wicket.ajax.AjaxRequestTarget;
+import org.apache.wicket.ajax.StatelessAjaxEventBehavior;
+import org.apache.wicket.ajax.attributes.AjaxRequestAttributes;
+import org.apache.wicket.ajax.attributes.AjaxRequestAttributes.Method;
+import org.apache.wicket.markup.html.form.Form;
+import org.apache.wicket.markup.html.form.IFormSubmitter;
+import org.apache.wicket.request.mapper.parameter.PageParameters;
+
+/**
+ * Stateless version of {@link AjaxFormSubmitBehavior}.
+ */
+public class StatelessAjaxFormSubmitBehavior extends StatelessAjaxEventBehavior
+{
+
+ /**
+ *
+ */
+ private static final long serialVersionUID = -4052276703516669553L;
--- End diff --
Useless javadoc.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by bitstorm <gi...@git.apache.org>.
Github user bitstorm commented on the pull request:
https://github.com/apache/wicket/pull/170#issuecomment-219976785
BTW I'm starting to wonder if we really need a custom AbstractDefaultAjaxBehavior#getCallbackUrl() to add page parameters to the callback url. When we generate an URL for stateless pages we end up using AbstractBookmarkableMapper or MountedMapper and both uses page.getPageParameters() to build the URL (line 182 for MountedMapper and 476 for AbstractBookmarkableMapper )
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: Port of AJAX stateless artifacts from wickets...
Posted by bitstorm <gi...@git.apache.org>.
Github user bitstorm commented on the pull request:
https://github.com/apache/wicket/pull/170#issuecomment-220080928
@svenmeier I also agree that the best way to proceed now would be to commit just the code to make stateless AJAX possible, i.e. to commit just the code involving the existing AJAX components/behaviors.
If everybody agrees I will close this PR without merging anything and I will open an issue on JIRA to commit just the code we need.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---