Merge pull request #243 from reportportal/develop

Release

CHANGELOG.md

@@ -1,6 +1,8 @@
 # Changelog
 
 ## [Unreleased]
+### Fixed
+- Cookies duplication on requests, by @HardNorth
 
 ## [5.2.10]
 ### Fixed

build.gradle

@@ -80,7 +80,7 @@ dependencies {
         exclude group: 'org.hamcrest'
     }
     testImplementation 'org.apache.commons:commons-io:1.3.2'
-    testImplementation 'com.epam.reportportal:agent-java-test-utils:0.0.3'
+    testImplementation 'com.epam.reportportal:agent-java-test-utils:0.0.6'
 
 }
 

src/main/java/com/epam/reportportal/service/ReportPortal.java

@@ -74,17 +74,18 @@ public class ReportPortal {
 
 	private static final Logger LOGGER = LoggerFactory.getLogger(ReportPortal.class);
 	private static final CookieJar COOKIE_JAR = new CookieJar() {
-		private final Map<String, CopyOnWriteArrayList<Cookie>> STORAGE = new ConcurrentHashMap<>();
+		private final Map<String, Map<String, Cookie>> HOST_STORAGE = new ConcurrentHashMap<>();
 
 		@Override
 		public void saveFromResponse(@Nonnull HttpUrl url, @Nonnull List<Cookie> cookies) {
-			STORAGE.computeIfAbsent(url.url().getHost(), u -> new CopyOnWriteArrayList<>()).addAll(cookies);
+			Map<String, Cookie> storage = HOST_STORAGE.computeIfAbsent(url.url().getHost(), u -> new ConcurrentHashMap<>());
+			cookies.forEach(cookie -> storage.put(cookie.name(), cookie));
 		}
 
 		@Override
 		@Nonnull
 		public List<Cookie> loadForRequest(@Nonnull HttpUrl url) {
-			return STORAGE.computeIfAbsent(url.url().getHost(), u -> new CopyOnWriteArrayList<>());
+			return new ArrayList<>(HOST_STORAGE.computeIfAbsent(url.url().getHost(), u -> new ConcurrentHashMap<>()).values());
 		}
 	};
 

src/test/java/com/epam/reportportal/service/ReportPortalTest.java

@@ -15,6 +15,10 @@
  */
 package com.epam.reportportal.service;
 
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
 import com.epam.reportportal.listeners.ListenerParameters;
 import com.epam.reportportal.test.TestUtils;
 import com.epam.reportportal.util.test.CommonUtils;
@@ -31,6 +35,7 @@
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
+import org.slf4j.LoggerFactory;
 import org.slf4j.bridge.SLF4JBridgeHandler;
 
 import java.net.ServerSocket;
@@ -42,6 +47,7 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 
 import static com.epam.reportportal.test.TestUtils.*;
 import static com.epam.reportportal.util.test.CommonUtils.shutdownExecutorService;
@@ -57,6 +63,7 @@ public class ReportPortalTest {
 	}
 
 	private static final String COOKIE = "AWSALB=P7cqG8g/K70xHAKOUPrWrG0XgmhG8GJNinj8lDnKVyITyubAen2lBr+fSa/e2JAoGksQphtImp49rZxc41qdqUGvAc67SdZHY1BMFIHKzc8kyWc1oQjq6oI+s39U";
+	private static final String OVERRIDING_COOKIE = "AWSALB=4CKP4Er0pg17+DfkR2ItBJbL8bdo2pBpP1SvLyiPwllSai6sawCn2blRepEX9nwdc8Aj4itewNUZdSONcTEgXfJFN2NAWuSl5CMQ3+vKuCO0cZrCxw02hXSXPir5";
 
 	@Mock
 	private ReportPortalClient rpClient;
@@ -159,46 +166,135 @@ public void verify_proxy_credential_works() throws Exception {
 		}
 	}
 
+	private static Map<String, Object> createCookieModel() {
+		Map<String, Object> model = new HashMap<>();
+		model.put("cookie", COOKIE);
+		SimpleDateFormat sdf = new SimpleDateFormat(SocketUtils.WEB_DATE_FORMAT, Locale.US);
+		sdf.setTimeZone(TimeZone.getTimeZone("GMT"));
+		Calendar cal = Calendar.getInstance();
+		model.put("date", sdf.format(cal.getTime()));
+		cal.add(Calendar.MINUTE, 2);
+		model.put("expire", sdf.format(cal.getTime()));
+		return model;
+	}
+
+	private static <T> Pair<List<String>, T> executeWithClosingOnException(ExecutorService clientExecutor, ServerSocket ss,
+			Callable<Pair<List<String>, T>> actions) throws Exception {
+		Exception error;
+		try {
+			return actions.call();
+		} catch (Exception e) {
+			error = e;
+		}
+		ss.close();
+		shutdownExecutorService(clientExecutor);
+		throw error;
+	}
+
+	private static <T> Pair<List<String>, T> executeWithClosingOnException(ExecutorService clientExecutor, ServerSocket ss,
+			SocketUtils.ServerCallable serverCallable, Callable<T> clientCallable) throws Exception {
+		return executeWithClosingOnException(clientExecutor, ss, () -> SocketUtils.executeServerCallable(serverCallable, clientCallable));
+	}
+
+	private static Pair<List<String>, StartLaunchRS> executeWithClosing(ExecutorService clientExecutor, ServerSocket ss,
+			SocketUtils.ServerCallable serverCallable, Callable<StartLaunchRS> clientCallable) throws Exception {
+		Pair<List<String>, StartLaunchRS> result = executeWithClosingOnException(clientExecutor,
+				ss,
+				() -> SocketUtils.executeServerCallable(serverCallable, clientCallable)
+		);
+		ss.close();
+		shutdownExecutorService(clientExecutor);
+		return result;
+	}
+
 	@Test
 	public void verify_rp_client_saves_and_bypasses_cookies() throws Exception {
 		ServerSocket ss = SocketUtils.getServerSocketOnFreePort();
 		ListenerParameters parameters = standardParameters();
-		parameters.setHttpLogging(true);
 		parameters.setBaseUrl("http://localhost:" + ss.getLocalPort());
 		ExecutorService clientExecutor = Executors.newSingleThreadExecutor();
 		ReportPortalClient rpClient = ReportPortal.builder().buildClient(ReportPortalClient.class, parameters, clientExecutor);
-		Exception error = null;
-		try {
-			Map<String, Object> model = new HashMap<>();
-			model.put("cookie", COOKIE);
-			SimpleDateFormat sdf = new SimpleDateFormat(SocketUtils.WEB_DATE_FORMAT, Locale.US);
-			sdf.setTimeZone(TimeZone.getTimeZone("GMT"));
-			Calendar cal = Calendar.getInstance();
-			model.put("date", sdf.format(cal.getTime()));
-			cal.add(Calendar.MINUTE, 2);
-			model.put("expire", sdf.format(cal.getTime()));
-
-			SocketUtils.ServerCallable serverCallable = new SocketUtils.ServerCallable(ss, model, "files/socket_response.txt");
-			Callable<StartLaunchRS> clientCallable = () -> rpClient.startLaunch(new StartLaunchRQ())
-					.timeout(5, TimeUnit.SECONDS)
-					.blockingGet();
-			Pair<List<String>, StartLaunchRS> result = SocketUtils.executeServerCallable(serverCallable, clientCallable);
-
-			assertThat(result.getValue(), notNullValue());
-			assertThat("First request should not contain cookie value", result.getKey().get(0), not(containsString(COOKIE)));
-
-			result = SocketUtils.executeServerCallable(serverCallable, clientCallable);
-
-			assertThat(result.getValue(), notNullValue());
-			assertThat("Second request should contain cookie value", result.getKey().get(0), containsString(COOKIE));
-		} catch (Exception e) {
-			error = e;
-		}
-		ss.close();
-		shutdownExecutorService(clientExecutor);
-		if (error != null) {
-			throw error;
-		}
+		SocketUtils.ServerCallable serverCallable = new SocketUtils.ServerCallable(ss, createCookieModel(), "files/socket_response.txt");
+		Callable<StartLaunchRS> clientCallable = () -> rpClient.startLaunch(new StartLaunchRQ()).timeout(5, TimeUnit.SECONDS).blockingGet();
+		Pair<List<String>, StartLaunchRS> result = executeWithClosingOnException(clientExecutor, ss, serverCallable, clientCallable);
+
+		assertThat(result.getValue(), notNullValue());
+		assertThat("First request should not contain cookie value", result.getKey().get(0), not(containsString(COOKIE)));
+
+		result = executeWithClosing(clientExecutor, ss, serverCallable, clientCallable);
+
+		assertThat(result.getValue(), notNullValue());
+		assertThat("Second request should contain cookie value", result.getKey().get(0), containsString(COOKIE));
+	}
+
+	@Test
+	public void verify_rp_client_does_not_duplicate_cookies() throws Exception {
+		ServerSocket ss = SocketUtils.getServerSocketOnFreePort();
+		ListenerParameters parameters = standardParameters();
+		parameters.setBaseUrl("http://localhost:" + ss.getLocalPort());
+		ExecutorService clientExecutor = Executors.newSingleThreadExecutor();
+		ReportPortalClient rpClient = ReportPortal.builder().buildClient(ReportPortalClient.class, parameters, clientExecutor);
+		Map<String, Object> model = createCookieModel();
+		SocketUtils.ServerCallable serverCallable = new SocketUtils.ServerCallable(ss, model, "files/socket_response.txt");
+		Callable<StartLaunchRS> clientCallable = () -> rpClient.startLaunch(new StartLaunchRQ()).timeout(5, TimeUnit.SECONDS).blockingGet();
+		executeWithClosingOnException(clientExecutor, ss, serverCallable, clientCallable);
+		model.put("cookie", OVERRIDING_COOKIE);
+		executeWithClosingOnException(clientExecutor, ss, serverCallable, clientCallable);
+		Pair<List<String>, StartLaunchRS> result = executeWithClosing(clientExecutor, ss, serverCallable, clientCallable);
+
+		assertThat(result.getValue(), notNullValue());
+		assertThat("Second request should contain new cookie value", result.getKey().get(0), containsString(OVERRIDING_COOKIE));
+		assertThat("Second request should not contain old cookie value", result.getKey().get(0), not(containsString(COOKIE)));
+	}
+
+	@Test
+	public void verify_rp_client_http_logging() throws Exception {
+		ServerSocket ss = SocketUtils.getServerSocketOnFreePort();
+		ListenerParameters parameters = standardParameters();
+		parameters.setHttpLogging(true);
+		String host = "localhost:" + ss.getLocalPort();
+		@SuppressWarnings("HttpUrlsUsage")
+		String baseUrl = "http://" + host;
+		parameters.setBaseUrl(baseUrl);
+		ExecutorService clientExecutor = Executors.newSingleThreadExecutor();
+		ReportPortalClient rpClient = ReportPortal.builder().buildClient(ReportPortalClient.class, parameters, clientExecutor);
+
+		SocketUtils.ServerCallable serverCallable = new SocketUtils.ServerCallable(ss,
+				Collections.emptyMap(),
+				Collections.singletonList("files/simple_response.txt")
+		);
+		// trigger http logging to init loggers
+		executeWithClosingOnException(clientExecutor, ss, serverCallable, () -> rpClient.getProjectSettings().blockingGet());
+
+		LoggerContext loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory();
+		List<Logger> loggerList = loggerContext.getLoggerList()
+				.stream()
+				.filter(l -> l.getName() != null && l.getName().endsWith("OkHttpClient"))
+				.collect(Collectors.toList());
+		assertThat(loggerList, hasSize(1));
+
+		ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
+		listAppender.setContext(loggerContext);
+		listAppender.start();
+		loggerList.get(0).addAppender(listAppender);
+
+		serverCallable = new SocketUtils.ServerCallable(ss, createCookieModel(), Collections.singletonList("files/socket_response.txt"));
+		Callable<StartLaunchRS> clientCallable = () -> rpClient.startLaunch(new StartLaunchRQ()).timeout(5, TimeUnit.SECONDS).blockingGet();
+		executeWithClosingOnException(clientExecutor, ss, serverCallable, clientCallable);
+
+		assertThat(listAppender.list, hasSize(greaterThan(10)));
+		String requestTarget = "--> POST " + baseUrl + "/api/v1/unit-test/launch http/1.1";
+		assertThat(listAppender.list.get(0).getMessage(), equalTo(requestTarget));
+		List<String> messages = listAppender.list.stream().map(ILoggingEvent::getMessage).collect(Collectors.toList());
+		assertThat(messages, hasItem("Host: " + host));
+		listAppender.list.clear();
+
+		executeWithClosing(clientExecutor, ss, serverCallable, clientCallable);
+
+		assertThat(listAppender.list, hasSize(greaterThan(10)));
+		assertThat(listAppender.list.get(0).getMessage(), equalTo(requestTarget));
+		messages = listAppender.list.stream().map(ILoggingEvent::getMessage).collect(Collectors.toList());
+		assertThat(messages, hasItem("Cookie: " + COOKIE));
 	}
 
 	@Test

src/test/resources/files/simple_response.txt

@@ -6,5 +6,6 @@ Pragma: no-cache
 X-Content-Type-Options: nosniff
 X-Frame-Options: DENY
 X-Xss-Protection: 1; mode=block
-Content-Length: 0
+Content-Length: 2
 
+{}