WICKET-6461 don't consume segments for optional parameters if required parameters are following
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/2283dc50 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/2283dc50 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/2283dc50 Branch: refs/heads/WICKET-6105-java.time Commit: 2283dc502fca56f9e912823a4f94a629188aa63f Parents: c28b72d Author: Sven Meier <[email protected]> Authored: Mon Sep 25 21:14:02 2017 +0200 Committer: Sven Meier <[email protected]> Committed: Mon Sep 25 21:18:27 2017 +0200 ---------------------------------------------------------------------- .../mapper/AbstractBookmarkableMapper.java | 34 ++++++++----- .../core/request/mapper/MountedMapperTest.java | 53 ++++++++++++++++++-- 2 files changed, 72 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/2283dc50/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java index a6bab53..9a39c10 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java @@ -622,46 +622,56 @@ public abstract class AbstractBookmarkableMapper extends AbstractComponentMapper protected PageParameters extractPageParameters(Request request, Url url) { int[] matchedParameters = getMatchedSegmentSizes(url); + int total = 0; for (int curMatchSize : matchedParameters) + { total += curMatchSize; + } PageParameters pageParameters = extractPageParameters(request, total, pageParametersEncoder); if (pageParameters != null) { pageParameters.setLocale(resolveLocale()); } - int skippedParameters = 0; + int segmentIndex = 0; for (int pathSegmentIndex = 0; pathSegmentIndex < pathSegments.size(); pathSegmentIndex++) { - MountPathSegment curPathSegment = pathSegments.get(pathSegmentIndex); - int matchSize = matchedParameters[pathSegmentIndex] - curPathSegment.getFixedPartSize(); - int optionalParameterMatch = matchSize - curPathSegment.getMinParameters(); - for (int matchSegment = 0; matchSegment < matchSize; matchSegment++) + MountPathSegment pathSegment = pathSegments.get(pathSegmentIndex); + + int totalAdded = 0; + int requiredAdded = 0; + for (int segmentParameterIndex = 0; segmentParameterIndex < pathSegment.getMaxParameters() && totalAdded < matchedParameters[pathSegmentIndex]; segmentParameterIndex++) { if (pageParameters == null) { pageParameters = newPageParameters(); } - int curSegmentIndex = matchSegment + curPathSegment.getSegmentIndex(); - String curSegment = mountSegments[curSegmentIndex]; + String curSegment = mountSegments[pathSegment.getSegmentIndex() + segmentParameterIndex]; + String placeholder = getPlaceholder(curSegment); String optionalPlaceholder = getOptionalPlaceholder(curSegment); // extract the parameter from URL if (placeholder != null) { pageParameters.add(placeholder, - url.getSegments().get(curSegmentIndex - skippedParameters), INamedParameters.Type.PATH); + url.getSegments().get(segmentIndex), INamedParameters.Type.PATH); + segmentIndex++; + totalAdded++; + requiredAdded++; } - else if (optionalPlaceholder != null && optionalParameterMatch > 0) + else if (optionalPlaceholder != null && + matchedParameters[pathSegmentIndex] - segmentParameterIndex > pathSegment.getMinParameters() + pathSegment.getFixedPartSize() - requiredAdded) { pageParameters.add(optionalPlaceholder, - url.getSegments().get(curSegmentIndex - skippedParameters), INamedParameters.Type.PATH); - optionalParameterMatch--; + url.getSegments().get(segmentIndex), INamedParameters.Type.PATH); + segmentIndex++; + totalAdded++; } } - skippedParameters += curPathSegment.getMaxParameters() - matchSize; + + segmentIndex += pathSegment.getFixedPartSize(); } return pageParameters; } http://git-wip-us.apache.org/repos/asf/wicket/blob/2283dc50/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/MountedMapperTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/MountedMapperTest.java b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/MountedMapperTest.java index fcd8afc..5b809a7 100644 --- a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/MountedMapperTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/MountedMapperTest.java @@ -739,7 +739,7 @@ public class MountedMapperTest extends AbstractMapperTest IRequestablePage page = ((RenderPageRequestHandler)handler).getPage(); assertEquals(0, page.getPageParameters().getIndexedCount()); - assertTrue(page.getPageParameters().getNamedKeys().size() == 3); + assertEquals(3, page.getPageParameters().getNamedKeys().size()); assertEquals("p1", page.getPageParameters().get("param1").toString()); assertEquals("p2", page.getPageParameters().get("param2").toString()); assertEquals("p3", page.getPageParameters().get("param3").toString()); @@ -811,7 +811,7 @@ public class MountedMapperTest extends AbstractMapperTest IRequestablePage page = ((RenderPageRequestHandler)handler).getPage(); assertEquals(1, page.getPageParameters().getIndexedCount()); - assertTrue(page.getPageParameters().getNamedKeys().size() == 2); + assertEquals(2, page.getPageParameters().getNamedKeys().size()); assertFalse("param1 should not be set", page.getPageParameters().getNamedKeys().contains("param1")); assertEquals("p2", page.getPageParameters().get("param2").toString()); @@ -859,7 +859,54 @@ public class MountedMapperTest extends AbstractMapperTest assertEquals("some/path/p2/p3/i1/i2?a=b&b=c", url.toString()); } - /* WICKET-5056 * */ + /** + * WICKET-6461 + */ + @Test + public void optionalPlaceholdersBeforeRequiredPlaceholder() throws Exception + { + final MountedMapper mapper = new MountedMapper("/params/#{optional1}/#{optional2}/${required}", MockPage.class) { + @Override + protected IMapperContext getContext() + { + return context; + } + + @Override + boolean getRecreateMountedPagesAfterExpiry() + { + return true; + } + }; + + IRequestHandler handler = mapper.mapRequest(getRequest(Url.parse("params/required"))); + assertThat(handler, instanceOf(RenderPageRequestHandler.class)); + IRequestablePage page = ((RenderPageRequestHandler)handler).getPage(); + PageParameters p = page.getPageParameters(); + assertEquals(1, p.getNamedKeys().size()); + assertEquals("required", p.get("required").toString()); + + handler = mapper.mapRequest(getRequest(Url.parse("params/optional1/required"))); + assertThat(handler, instanceOf(RenderPageRequestHandler.class)); + page = ((RenderPageRequestHandler)handler).getPage(); + p = page.getPageParameters(); + assertEquals(2, p.getNamedKeys().size()); + assertEquals("required", p.get("required").toString()); + assertEquals("optional1", p.get("optional1").toString()); + + handler = mapper.mapRequest(getRequest(Url.parse("params/optional1/optional2/required"))); + assertThat(handler, instanceOf(RenderPageRequestHandler.class)); + page = ((RenderPageRequestHandler)handler).getPage(); + p = page.getPageParameters(); + assertEquals(3, p.getNamedKeys().size()); + assertEquals("required", p.get("required").toString()); + assertEquals("optional1", p.get("optional1").toString()); + assertEquals("optional2", p.get("optional2").toString()); + } + + /** + * WICKET-5056 + */ @Test public void optionalParameterGetsLowerScore_ThanExactOne() throws Exception {
