From 6b9661259d0483d00d532aebc6ee5ddf7fc2b7a6 Mon Sep 17 00:00:00 2001 From: Santiago Pericasgeertsen Date: Mon, 8 Feb 2021 14:30:48 -0500 Subject: [PATCH] Do not attempt to access the request context in Fallback callback. If used together with Retry, it is possible for the fallback to be called in a fresh thread for which there is no current request scope. Instead just use the original value obtained in this class' constructor. Updated functional test (with some class renaming) to cover this use case. Signed-off-by: Santiago Pericasgeertsen --- .../faulttolerance/MethodInvoker.java | 20 ++------- .../{SomeService.java => Bean1.java} | 6 +-- .../{SomeService2.java => Bean2.java} | 9 ++-- .../{SomeService3.java => Bean3.java} | 7 ++-- .../tests/functional/requestscope/Bean4.java | 39 +++++++++++++++++ ...{MultiTenantService.java => Service1.java} | 8 ++-- ...MultiTenantService2.java => Service2.java} | 8 ++-- .../{MyResource.java => Service3.java} | 9 ++-- .../functional/requestscope/Service4.java | 42 +++++++++++++++++++ .../functional/requestscope/TenantTest.java | 14 ++++++- 10 files changed, 122 insertions(+), 40 deletions(-) rename tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/{SomeService.java => Bean1.java} (87%) rename tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/{SomeService2.java => Bean2.java} (92%) rename tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/{SomeService3.java => Bean3.java} (94%) create mode 100644 tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean4.java rename tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/{MultiTenantService.java => Service1.java} (89%) rename tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/{MultiTenantService2.java => Service2.java} (88%) rename tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/{MyResource.java => Service3.java} (88%) create mode 100644 tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service4.java diff --git a/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/MethodInvoker.java b/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/MethodInvoker.java index 568de0c9db2..57380e09afe 100644 --- a/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/MethodInvoker.java +++ b/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/MethodInvoker.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 Oracle and/or its affiliates. + * Copyright (c) 2021 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -587,21 +587,9 @@ private FtHandlerTyped createMethodHandler(MethodState methodState) { if (introspector.hasFallback()) { Fallback fallback = Fallback.builder() .fallback(throwable -> { - try { - // Reference request context if request scope is active - if (requestScope != null) { - requestContext = requestScope.referenceCurrent(); - } - - // Execute callback logic - CommandFallback cfb = new CommandFallback(context, introspector, throwable); - return toCompletionStageSupplier(cfb::execute).get(); - } finally { - // Release request context if referenced - if (requestContext != null) { - requestContext.release(); - } - } + // Execute callback logic + CommandFallback cfb = new CommandFallback(context, introspector, throwable); + return toCompletionStageSupplier(cfb::execute).get(); }) .applyOn(mapTypes(introspector.getFallback().applyOn())) .skipOn(mapTypes(introspector.getFallback().skipOn())) diff --git a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/SomeService.java b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean1.java similarity index 87% rename from tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/SomeService.java rename to tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean1.java index 89df5451fca..463d42e3b6e 100644 --- a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/SomeService.java +++ b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean1.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 Oracle and/or its affiliates. + * Copyright (c) 2021 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,11 +21,11 @@ import org.eclipse.microprofile.faulttolerance.Retry; @ApplicationScoped -public class SomeService { +public class Bean1 { @Inject @TestQualifier - RequestTestQualifier requestTestQualifier; + private RequestTestQualifier requestTestQualifier; @Retry public String test() throws Exception { diff --git a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/SomeService2.java b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean2.java similarity index 92% rename from tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/SomeService2.java rename to tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean2.java index e700056c4df..436f57f29a4 100644 --- a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/SomeService2.java +++ b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean2.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 Oracle and/or its affiliates. + * Copyright (c) 2021 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,20 +15,21 @@ */ package io.helidon.tests.functional.requestscope; +import java.util.concurrent.atomic.AtomicLong; + import javax.enterprise.context.ApplicationScoped; import javax.inject.Inject; -import java.util.concurrent.atomic.AtomicLong; import org.eclipse.microprofile.faulttolerance.CircuitBreaker; import org.eclipse.microprofile.faulttolerance.Fallback; @ApplicationScoped -public class SomeService2 { +public class Bean2 { private AtomicLong counter = new AtomicLong(0); @Inject - TenantContext tenantContext; + private TenantContext tenantContext; @CircuitBreaker(successThreshold = 2, requestVolumeThreshold = 4) @Fallback(fallbackMethod = "testFallback") diff --git a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/SomeService3.java b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean3.java similarity index 94% rename from tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/SomeService3.java rename to tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean3.java index 4836ae2b410..f0d57df3653 100644 --- a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/SomeService3.java +++ b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean3.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 Oracle and/or its affiliates. + * Copyright (c) 2021 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,13 +15,14 @@ */ package io.helidon.tests.functional.requestscope; -import javax.enterprise.context.ApplicationScoped; import java.util.concurrent.atomic.AtomicLong; +import javax.enterprise.context.ApplicationScoped; + import org.eclipse.microprofile.faulttolerance.Fallback; @ApplicationScoped -public class SomeService3 { +public class Bean3 { private AtomicLong counter = new AtomicLong(0); diff --git a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean4.java b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean4.java new file mode 100644 index 00000000000..576345518be --- /dev/null +++ b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Bean4.java @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2021 Oracle and/or its affiliates. + * + * Licensed 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 io.helidon.tests.functional.requestscope; + +import java.util.concurrent.atomic.AtomicLong; + +import javax.enterprise.context.ApplicationScoped; + +import org.eclipse.microprofile.faulttolerance.Fallback; +import org.eclipse.microprofile.faulttolerance.Retry; + +@ApplicationScoped +public class Bean4 { + + private AtomicLong counter = new AtomicLong(0); + + @Retry(jitter = 1000L, delay = 3) + @Fallback(fallbackMethod = "testFallback") + public String test(String testParam) throws Exception { + throw new RuntimeException("called failed"); + } + + public String testFallback(String testParam) throws Exception { + return testParam; + } +} diff --git a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/MultiTenantService.java b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service1.java similarity index 89% rename from tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/MultiTenantService.java rename to tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service1.java index b407250af2c..71e3977619e 100644 --- a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/MultiTenantService.java +++ b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service1.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 Oracle and/or its affiliates. + * Copyright (c) 2021 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,15 +24,15 @@ @RequestScoped @Path("/test") -public class MultiTenantService { +public class Service1 { @Inject - SomeService someService; + private Bean1 bean1; @GET public String getTenantResource() { try { - return someService.test(); + return bean1.test(); } catch (IllegalTenantException e) { return "Expected"; } catch (Exception e) { diff --git a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/MultiTenantService2.java b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service2.java similarity index 88% rename from tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/MultiTenantService2.java rename to tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service2.java index 4200678ffb6..3e9447d4a59 100644 --- a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/MultiTenantService2.java +++ b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service2.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 Oracle and/or its affiliates. + * Copyright (c) 2021 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,15 +24,15 @@ @RequestScoped @Path("/test2") -public class MultiTenantService2 { +public class Service2 { @Inject - SomeService2 someService2; + private Bean2 bean2; @GET public String getTenantResource() { try { - String s = someService2.test(); + String s = bean2.test(); return s == null ? "" : s; } catch (Exception e) { // This path implies a CDI exception related to request scope diff --git a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/MyResource.java b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service3.java similarity index 88% rename from tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/MyResource.java rename to tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service3.java index 4bc83195d8c..42c4d935872 100644 --- a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/MyResource.java +++ b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service3.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 Oracle and/or its affiliates. + * Copyright (c) 2021 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,19 +25,18 @@ @RequestScoped @Path("/test3") -public class MyResource { +public class Service3 { @Inject - SomeService3 someService3; + private Bean3 bean3; @GET public String getTestResource(@QueryParam("param1") String param1) { try { - return someService3.test(param1); + return bean3.test(param1); } catch (Exception e) { System.out.println(e.getMessage()); throw new WebApplicationException(e, Response.Status.INTERNAL_SERVER_ERROR); } } - } diff --git a/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service4.java b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service4.java new file mode 100644 index 00000000000..5c52d886873 --- /dev/null +++ b/tests/functional/request-scope/src/main/java/io/helidon/tests/functional/requestscope/Service4.java @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2021 Oracle and/or its affiliates. + * + * Licensed 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 io.helidon.tests.functional.requestscope; + +import javax.enterprise.context.RequestScoped; +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.QueryParam; +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.Response; + +@RequestScoped +@Path("/test4") +public class Service4 { + + @Inject + private Bean4 bean4; + + @GET + public String getTestResource(@QueryParam("param1") String param1) { + try { + return bean4.test(param1); + } catch (Exception e) { + System.out.println(e.getMessage()); + throw new WebApplicationException(e, Response.Status.INTERNAL_SERVER_ERROR); + } + } +} diff --git a/tests/functional/request-scope/src/test/java/io/helidon/tests/functional/requestscope/TenantTest.java b/tests/functional/request-scope/src/test/java/io/helidon/tests/functional/requestscope/TenantTest.java index bb5d6262d58..c4b7326689d 100644 --- a/tests/functional/request-scope/src/test/java/io/helidon/tests/functional/requestscope/TenantTest.java +++ b/tests/functional/request-scope/src/test/java/io/helidon/tests/functional/requestscope/TenantTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 Oracle and/or its affiliates. + * Copyright (c) 2021 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -66,4 +66,16 @@ public void test3() { assertThat(entityValue, is(paramValue)); } } + + @Test + public void test4() { + Response r; + r = baseTarget.path("test4") + .queryParam("param1", "1") + .request() + .get(); + assertThat(r.getStatus(), is(HttpResponseStatus.OK.code())); + String entityValue = r.readEntity(String.class); + assertThat(entityValue, is("1")); + } }