From 39ea45ec4a8e5f00e51efa6a229a493143f9d3f8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 8 Nov 2016 18:13:34 +0100 Subject: [PATCH] SONAR-8332 make RequestUidGeneratorImpl support any number of HTTP request up to LONG.MAX_VALUE thanks to renewal of internal UuidGenerator.WithFixedBase instance every 4_194_304 requests --- .../web/requestid/HttpRequestUidModule.java | 3 +- .../web/requestid/RequestIdConfiguration.java | 35 +++++++++++ .../requestid/RequestUidGeneratorBase.java | 10 +-- .../RequestUidGeneratorBaseImpl.java | 5 +- .../requestid/RequestUidGeneratorImpl.java | 62 +++++++++++++++++-- .../requestid/HttpRequestUidModuleTest.java | 2 +- .../requestid/RequestIdConfigurationTest.java | 33 ++++++++++ .../RequestUidGeneratorImplTest.java | 62 ++++++++++++++++++- .../requestid/RequestUidMDCStorageTest.java | 19 ++++++ 9 files changed, 212 insertions(+), 19 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestIdConfiguration.java create mode 100644 server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/RequestIdConfigurationTest.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/HttpRequestUidModule.java b/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/HttpRequestUidModule.java index 745c49ef85a..3a2e5d531e9 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/HttpRequestUidModule.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/HttpRequestUidModule.java @@ -24,7 +24,8 @@ import org.sonar.core.platform.Module; public class HttpRequestUidModule extends Module { @Override protected void configureModule() { - add(RequestUidGeneratorBaseImpl.class, + add(new RequestIdConfiguration(RequestUidGeneratorImpl.UUID_GENERATOR_RENEWAL_COUNT), + RequestUidGeneratorBaseImpl.class, RequestUidGeneratorImpl.class); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestIdConfiguration.java b/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestIdConfiguration.java new file mode 100644 index 00000000000..750aeec03c9 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestIdConfiguration.java @@ -0,0 +1,35 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.platform.web.requestid; + +public class RequestIdConfiguration { + /** + * @see RequestUidGeneratorImpl#mustRenewUuidGenerator(long) + */ + private final long uuidGeneratorRenewalCount; + + public RequestIdConfiguration(long uuidGeneratorRenewalCount) { + this.uuidGeneratorRenewalCount = uuidGeneratorRenewalCount; + } + + public long getUidGeneratorRenewalCount() { + return uuidGeneratorRenewalCount; + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorBase.java b/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorBase.java index 4d93d8476c0..65e0cb76ff8 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorBase.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorBase.java @@ -21,9 +21,9 @@ package org.sonar.server.platform.web.requestid; import org.sonar.core.util.UuidGenerator; -/** - * Marker interface to be able to add a {@link org.sonar.core.util.UuidGenerator.WithFixedBase} to pico without risk - * of use in another domain. - */ -public interface RequestUidGeneratorBase extends UuidGenerator.WithFixedBase { +public interface RequestUidGeneratorBase { + /** + * Provides a new instance of {@link UuidGenerator.WithFixedBase} to be used by {@link RequestUidGeneratorImpl}. + */ + UuidGenerator.WithFixedBase createNew(); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorBaseImpl.java b/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorBaseImpl.java index e6930b96401..059f5f6efbe 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorBaseImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorBaseImpl.java @@ -23,10 +23,9 @@ import org.sonar.core.util.UuidGenerator; import org.sonar.core.util.UuidGeneratorImpl; public class RequestUidGeneratorBaseImpl implements RequestUidGeneratorBase { - private final UuidGenerator.WithFixedBase delegate = new UuidGeneratorImpl().withFixedBase(); @Override - public byte[] generate(int increment) { - return delegate.generate(increment); + public UuidGenerator.WithFixedBase createNew() { + return new UuidGeneratorImpl().withFixedBase(); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorImpl.java b/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorImpl.java index 85d2a30ae5e..961f568d201 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorImpl.java @@ -20,27 +20,77 @@ package org.sonar.server.platform.web.requestid; import java.util.Base64; -import java.util.concurrent.atomic.AtomicInteger; -import org.sonar.core.util.UuidGeneratorImpl; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; +import org.sonar.core.util.UuidGenerator; /** * This implementation of {@link RequestUidGenerator} creates unique identifiers for HTTP requests leveraging - * {@link UuidGeneratorImpl#withFixedBase()}. + * {@link UuidGenerator.WithFixedBase#generate(int)} and a counter of HTTP requests. + *

+ * To work around the limit of unique values produced by {@link UuidGenerator.WithFixedBase#generate(int)}, the + * {@link UuidGenerator.WithFixedBase} instance will be renewed every + * {@link RequestIdConfiguration#getUidGeneratorRenewalCount() RequestIdConfiguration#uidGeneratorRenewalCount} + * HTTP requests. + *

*

* This implementation is Thread safe. *

*/ public class RequestUidGeneratorImpl implements RequestUidGenerator { - private final AtomicInteger counter = new AtomicInteger(); + /** + * The value to which the HTTP request count will be compared to (using a modulo operator, + * see {@link #mustRenewUuidGenerator(long)}). + * + *

+ * This value can't be the last value before {@link UuidGenerator.WithFixedBase#generate(int)} returns a non unique + * value, ie. 2^23-1 because there is no guarantee the renewal will happen before any other thread calls + * {@link UuidGenerator.WithFixedBase#generate(int)} method of the deplated {@link UuidGenerator.WithFixedBase} instance. + *

+ * + *

+ * To keep a comfortable margin of error, 2^22 will be used. + *

+ */ + public static final long UUID_GENERATOR_RENEWAL_COUNT = 4_194_304; + + private final AtomicLong counter = new AtomicLong(); private final RequestUidGeneratorBase requestUidGeneratorBase; + private final RequestIdConfiguration requestIdConfiguration; + private final AtomicReference uuidGenerator; - public RequestUidGeneratorImpl(RequestUidGeneratorBase requestUidGeneratorBase) { + public RequestUidGeneratorImpl(RequestUidGeneratorBase requestUidGeneratorBase, RequestIdConfiguration requestIdConfiguration) { this.requestUidGeneratorBase = requestUidGeneratorBase; + this.uuidGenerator = new AtomicReference<>(requestUidGeneratorBase.createNew()); + this.requestIdConfiguration = requestIdConfiguration; } @Override public String generate() { - return Base64.getEncoder().encodeToString(requestUidGeneratorBase.generate(counter.incrementAndGet())); + UuidGenerator.WithFixedBase currentUuidGenerator = this.uuidGenerator.get(); + long counterValue = counter.getAndIncrement(); + if (counterValue != 0 && mustRenewUuidGenerator(counterValue)) { + UuidGenerator.WithFixedBase newUuidGenerator = requestUidGeneratorBase.createNew(); + uuidGenerator.set(newUuidGenerator); + return generate(newUuidGenerator, counterValue); + } + return generate(currentUuidGenerator, counterValue); + } + + /** + * Since renewal of {@link UuidGenerator.WithFixedBase} instance is based on the HTTP request counter, only a single + * thread can get the right value which will make this method return true. So, this is thread-safe by design, therefor + * this method doesn't need external synchronization. + *

+ * The value to which the counter is compared should however be chosen with caution: see {@link #UUID_GENERATOR_RENEWAL_COUNT}. + *

+ */ + private boolean mustRenewUuidGenerator(long counter) { + return counter % requestIdConfiguration.getUidGeneratorRenewalCount() == 0; + } + + private static String generate(UuidGenerator.WithFixedBase uuidGenerator, long increment) { + return Base64.getEncoder().encodeToString(uuidGenerator.generate((int) increment)); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/HttpRequestUidModuleTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/HttpRequestUidModuleTest.java index 377df7dc18a..e8b4c173d57 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/HttpRequestUidModuleTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/HttpRequestUidModuleTest.java @@ -35,6 +35,6 @@ public class HttpRequestUidModuleTest { underTest.configure(container); assertThat(container.getPicoContainer().getComponentAdapters()) - .hasSize(COMPONENTS_HARDCODED_IN_CONTAINER + 2); + .hasSize(COMPONENTS_HARDCODED_IN_CONTAINER + 3); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/RequestIdConfigurationTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/RequestIdConfigurationTest.java new file mode 100644 index 00000000000..e0f09ab7e7c --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/RequestIdConfigurationTest.java @@ -0,0 +1,33 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.platform.web.requestid; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class RequestIdConfigurationTest { + private RequestIdConfiguration underTest = new RequestIdConfiguration(50); + + @Test + public void getUidGeneratorRenewalCount_returns_value_provided_from_constructor() { + assertThat(underTest.getUidGeneratorRenewalCount()).isEqualTo(50); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorImplTest.java index 755d1d25b12..f49c4568d0d 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/RequestUidGeneratorImplTest.java @@ -19,15 +19,71 @@ */ package org.sonar.server.platform.web.requestid; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.core.util.UuidGenerator; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class RequestUidGeneratorImplTest { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + private UuidGenerator.WithFixedBase generator1 = increment -> new byte[] {124, 22, 66, 96, 55, 88, 2, 9}; + private UuidGenerator.WithFixedBase generator2 = increment -> new byte[] {0, 5, 88, 81, 8, 6, 44, 19}; + private UuidGenerator.WithFixedBase generator3 = increment -> new byte[] {126, 9, 35, 76, 2, 1, 2}; + private RequestUidGeneratorBase uidGeneratorBase = mock(RequestUidGeneratorBase.class); + private IllegalStateException expected = new IllegalStateException("Unexpected third call to createNew"); + + @Test + public void generate_renews_inner_UuidGenerator_instance_every_number_of_calls_to_generate_specified_in_RequestIdConfiguration_supports_2() { + when(uidGeneratorBase.createNew()) + .thenReturn(generator1) + .thenReturn(generator2) + .thenReturn(generator3) + .thenThrow(expected); + + RequestUidGeneratorImpl underTest = new RequestUidGeneratorImpl(uidGeneratorBase, new RequestIdConfiguration(2)); + + assertThat(underTest.generate()).isEqualTo("fBZCYDdYAgk="); // using generator1 + assertThat(underTest.generate()).isEqualTo("fBZCYDdYAgk="); // still using generator1 + assertThat(underTest.generate()).isEqualTo("AAVYUQgGLBM="); // renewing generator and using generator2 + assertThat(underTest.generate()).isEqualTo("AAVYUQgGLBM="); // still using generator2 + assertThat(underTest.generate()).isEqualTo("fgkjTAIBAg=="); // renewing generator and using generator3 + assertThat(underTest.generate()).isEqualTo("fgkjTAIBAg=="); // using generator3 + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage(expected.getMessage()); + + underTest.generate(); // renewing generator and failing + } + @Test - public void generate_returns_value_if_RequestUidGeneratorBase_generate_encoded_in_base64() { - RequestUidGeneratorImpl underTest = new RequestUidGeneratorImpl(increment -> new byte[] {124, 22, 66, 96, 55, 88, 2, 9}); + public void generate_renews_inner_UuidGenerator_instance_every_number_of_calls_to_generate_specified_in_RequestIdConfiguration_supports_3() { + when(uidGeneratorBase.createNew()) + .thenReturn(generator1) + .thenReturn(generator2) + .thenReturn(generator3) + .thenThrow(expected); + + RequestUidGeneratorImpl underTest = new RequestUidGeneratorImpl(uidGeneratorBase, new RequestIdConfiguration(3)); + + assertThat(underTest.generate()).isEqualTo("fBZCYDdYAgk="); // using generator1 + assertThat(underTest.generate()).isEqualTo("fBZCYDdYAgk="); // still using generator1 + assertThat(underTest.generate()).isEqualTo("fBZCYDdYAgk="); // still using generator1 + assertThat(underTest.generate()).isEqualTo("AAVYUQgGLBM="); // renewing generator and using it + assertThat(underTest.generate()).isEqualTo("AAVYUQgGLBM="); // still using generator2 + assertThat(underTest.generate()).isEqualTo("AAVYUQgGLBM="); // still using generator2 + assertThat(underTest.generate()).isEqualTo("fgkjTAIBAg=="); // renewing generator and using it + assertThat(underTest.generate()).isEqualTo("fgkjTAIBAg=="); // using generator3 + assertThat(underTest.generate()).isEqualTo("fgkjTAIBAg=="); // using generator3 + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage(expected.getMessage()); - assertThat(underTest.generate()).isEqualTo("fBZCYDdYAgk="); + underTest.generate(); // renewing generator and failing } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/RequestUidMDCStorageTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/RequestUidMDCStorageTest.java index 81d991f3813..53595b2db69 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/RequestUidMDCStorageTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/platform/web/requestid/RequestUidMDCStorageTest.java @@ -1,3 +1,22 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ package org.sonar.server.platform.web.requestid; import org.apache.log4j.MDC; -- 2.39.5