You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by sv...@apache.org on 2021/07/26 20:03:19 UTC

[wicket] 01/02: WICKET-6908 wrap exceptions for log data

This is an automated email from the ASF dual-hosted git repository.

svenmeier pushed a commit to branch WICKET-6908-detach-failure
in repository https://gitbox.apache.org/repos/asf/wicket.git

commit 6dceb0cd86eee796203e335b8d23f667758aca41
Author: Sven Meier <sv...@apache.org>
AuthorDate: Mon Jul 26 21:37:01 2021 +0200

    WICKET-6908 wrap exceptions for log data
---
 .../request/handler/logger/ListenerLogData.java    | 79 +++++--------------
 .../core/request/handler/logger/PageLogData.java   | 27 ++++---
 .../handler/logger/ListenerLogDataTest.java        | 92 ++++++++++++++++++++++
 3 files changed, 125 insertions(+), 73 deletions(-)

diff --git a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/logger/ListenerLogData.java b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/logger/ListenerLogData.java
index c0f4165..da4887b 100644
--- a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/logger/ListenerLogData.java
+++ b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/logger/ListenerLogData.java
@@ -35,9 +35,9 @@ public class ListenerLogData extends PageLogData
 	private final Class<? extends IRequestableComponent> componentClass;
 	private final String componentPath;
 	private final Integer behaviorIndex;
-	private Class<? extends Behavior> behaviorClass;
-	private Class<? extends IRequestableComponent> submittingComponentClass;
-	private String submittingComponentPath;
+	private final Class<? extends Behavior> behaviorClass;
+	private final Class<? extends IRequestableComponent> submittingComponentClass;
+	private final String submittingComponentPath;
 
 	/**
 	 * Construct.
@@ -48,68 +48,24 @@ public class ListenerLogData extends PageLogData
 	public ListenerLogData(IPageAndComponentProvider pageAndComponentProvider, Integer behaviorIndex)
 	{
 		super(pageAndComponentProvider);
-		componentClass = tryToGetComponentClass(pageAndComponentProvider);
-		componentPath = tryToGetComponentPath(pageAndComponentProvider);
+
 		this.behaviorIndex = behaviorIndex;
-		if (behaviorIndex != null && componentClass != null)
+
+		componentClass = optional(() -> pageAndComponentProvider.getComponent().getClass());
+		componentPath = optional(() -> pageAndComponentProvider.getComponentPath());
+
+		if (behaviorIndex != null)
 		{
-			try
-			{
-				behaviorClass = pageAndComponentProvider.getComponent()
+			behaviorClass = optional(() -> pageAndComponentProvider.getComponent()
 					.getBehaviorById(behaviorIndex)
-					.getClass();
-			}
-			catch (Exception ignore)
-			{
-				behaviorClass = null;
-			}
+					.getClass());
 		}
 		else
 		{
 			behaviorClass = null;
 		}
 		
-		final Component formSubmitter = tryToGetFormSubmittingComponent(pageAndComponentProvider);
-		if (formSubmitter != null)
-		{
-			submittingComponentClass = formSubmitter.getClass();
-			submittingComponentPath = formSubmitter.getPageRelativePath();
-		}
-	}
-
-	private static Class<? extends IRequestableComponent> tryToGetComponentClass(
-		IPageAndComponentProvider pageAndComponentProvider)
-	{
-		try
-		{
-			return pageAndComponentProvider.getComponent().getClass();
-		}
-		catch (Exception e)
-		{
-			// getComponent might fail if the page does not exist (ie session timeout)
-			return null;
-		}
-	}
-
-
-	private static String tryToGetComponentPath(IPageAndComponentProvider pageAndComponentProvider)
-	{
-		try
-		{
-			return pageAndComponentProvider.getComponentPath();
-		}
-		catch (Exception e)
-		{
-			// getComponentPath might fail if the page does not exist (ie session timeout)
-			return null;
-		}
-	}
-
-	private static Component tryToGetFormSubmittingComponent(
-		IPageAndComponentProvider pageAndComponentProvider)
-	{
-		try
-		{
+		final Component formSubmitter = optional(() -> {
 			final IRequestableComponent component = pageAndComponentProvider.getComponent();
 			if (component instanceof Form)
 			{
@@ -117,11 +73,14 @@ public class ListenerLogData extends PageLogData
 				return submitter instanceof Component ? (Component)submitter : null;
 			}
 			return null;
-		}
-		catch (Exception e)
+		});
+		if (formSubmitter != null)
 		{
-			// getComponent might fail if the page does not exist (ie session timeout)
-			return null;
+			submittingComponentClass = formSubmitter.getClass();
+			submittingComponentPath = formSubmitter.getPageRelativePath();
+		} else {
+			submittingComponentClass = null;
+			submittingComponentPath = null;
 		}
 	}
 
diff --git a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/logger/PageLogData.java b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/logger/PageLogData.java
index fd3b53f..e27d1da 100644
--- a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/logger/PageLogData.java
+++ b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/logger/PageLogData.java
@@ -16,6 +16,8 @@
  */
 package org.apache.wicket.core.request.handler.logger;
 
+import java.util.function.Supplier;
+
 import org.apache.wicket.Page;
 import org.apache.wicket.core.request.handler.IPageClassRequestHandler;
 import org.apache.wicket.core.request.handler.IPageProvider;
@@ -46,21 +48,20 @@ public class PageLogData implements ILogData
 	 */
 	public PageLogData(IPageProvider pageProvider)
 	{
-		pageClass = tryToGetPageClass(pageProvider);
-		pageId = pageProvider.getPageId();
-		pageParameters = pageProvider.getPageParameters();
-		renderCount = pageProvider.getRenderCount();
+		pageId = optional(pageProvider::getPageId);
+		renderCount = optional(pageProvider::getRenderCount);
+		pageClass = optional(pageProvider::getPageClass);
+		pageParameters = optional(pageProvider::getPageParameters);
 	}
 
-	private static Class<? extends IRequestablePage> tryToGetPageClass(IPageProvider pageProvider)
-	{
-		try
-		{
-			return pageProvider.getPageClass();
-		}
-		catch (Exception e)
-		{
-			// getPageClass might fail if the page does not exist (ie session timeout)
+	/**
+	 * Wrapper for optional values that might fail if the page does not exist
+	 * (i.e. session timeout).
+	 */
+	protected <T> T optional(Supplier<T> function) {
+		try {
+			return function.get();
+		} catch (Exception ex) {
 			return null;
 		}
 	}
diff --git a/wicket-core/src/test/java/org/apache/wicket/core/request/handler/logger/ListenerLogDataTest.java b/wicket-core/src/test/java/org/apache/wicket/core/request/handler/logger/ListenerLogDataTest.java
new file mode 100644
index 0000000..88c4976
--- /dev/null
+++ b/wicket-core/src/test/java/org/apache/wicket/core/request/handler/logger/ListenerLogDataTest.java
@@ -0,0 +1,92 @@
+/*
+ * 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.core.request.handler.logger;
+
+import org.apache.wicket.core.request.handler.IPageAndComponentProvider;
+import org.apache.wicket.protocol.http.PageExpiredException;
+import org.apache.wicket.request.component.IRequestableComponent;
+import org.apache.wicket.request.component.IRequestablePage;
+import org.apache.wicket.request.mapper.parameter.PageParameters;
+import org.junit.jupiter.api.Test;
+
+class ListenerLogDataTest {
+
+	/**
+	 * Test for WICKET-6908.
+	 */
+	@Test
+	void neverFails() {
+		IPageAndComponentProvider provider = new IPageAndComponentProvider() {
+			
+			@Override
+			public boolean wasExpired() {
+				throw new IllegalStateException();
+			}
+			
+			@Override
+			public boolean hasPageInstance() {
+				throw new IllegalStateException();
+			}
+			
+			@Override
+			public Integer getRenderCount() {
+				throw new IllegalStateException();
+			}
+			
+			@Override
+			public PageParameters getPageParameters() throws PageExpiredException {
+				throw new IllegalStateException();
+			}
+			
+			@Override
+			public IRequestablePage getPageInstance() throws PageExpiredException {
+				throw new IllegalStateException();
+			}
+			
+			@Override
+			public Integer getPageId() {
+				throw new IllegalStateException();
+			}
+			
+			@Override
+			public Class<? extends IRequestablePage> getPageClass() throws PageExpiredException {
+				throw new IllegalStateException();
+			}
+			
+			@Override
+			public boolean doesProvideNewPage() {
+				throw new IllegalStateException();
+			}
+			
+			@Override
+			public void detach() {
+			}
+			
+			@Override
+			public String getComponentPath() {
+				throw new IllegalStateException();
+			}
+			
+			@Override
+			public IRequestableComponent getComponent() {
+				throw new IllegalStateException();
+			}
+		};
+		
+		new ListenerLogData(provider, 1);
+	}
+}