Repository: eclipse/microprofile-lra (java)

Issue:
Clarify usage of 412 and 410 in the participant enlistment cases
We have to distinguish two cases:

* participant returning values from Compensate, Complete, Status
  * 410 means participant has no knowledge of received LRA (basically same as Compensated/Completed) -- [spec#302]( https://github.com/eclipse/microprofile-lra/blame/master/spec/src/main/asciidoc/microprofile-lra-spec.adoc#L302)
  * 412 is not allowed

* values returned from user invocations of LRA methods with included LRA context 
  * this needs clarification as per https://github.com/eclipse/microprofile-lra/pull/281


The options discussed for "values returned from user invocations of LRA methods with included LRA context":

1. use both 412 and 410
* 412 LRA context is present when it shouldn't or not present when it should or invalid context (see MANDATORY or NEVER types) and invalid received URI
* 410 expired context (valid URI but not found in the implementation resources)
  * basically closed or canceled LRA already removed from the implementation resources

2. use only 412 as it is a different use case then participant returning values
* 412 means that the context is present when it shouldn't or not present when it should **or** that the received context is expired (so no distinction) or invalid URI

Can we agree on one of the options and unify the text and test expectations across the specification and the Javadoc?



Pull request description:
Document the possible return HTTP status codes from the LRA method in…
…vocations

resolves #290 

Patch:
diff --git a/api/src/main/java/org/eclipse/microprofile/lra/annotation/ws/rs/LRA.java b/api/src/main/java/org/eclipse/microprofile/lra/annotation/ws/rs/LRA.java
index dbb4770..07b7052 100644
--- a/api/src/main/java/org/eclipse/microprofile/lra/annotation/ws/rs/LRA.java
+++ b/api/src/main/java/org/eclipse/microprofile/lra/annotation/ws/rs/LRA.java
@@ -61,6 +61,13 @@ import java.time.temporal.ChronoUnit;
  *   <li>to cancel the LRA context when the method returns particular HTTP
  *       status codes</li>
  * </ul>
+ * 
+ * Some of the above defined LRA operations are performed before the execution
+ * proceeds to the LRA annotated business method. If any of these LRA operations
+ * cannot complete the implementation MUST return one of the defined HTTP status
+ * codes which means that the business method is not invoked. For the definition of
+ * the allowed status codes and condition in which they are returned please
+ * see the specification document.
  *
  * <p>
  * Newly created LRAs are uniquely identified and the id is referred to as the
diff --git a/spec/src/main/asciidoc/microprofile-lra-spec.adoc b/spec/src/main/asciidoc/microprofile-lra-spec.adoc
index 1cf9236..029dbc9 100644
--- a/spec/src/main/asciidoc/microprofile-lra-spec.adoc
+++ b/spec/src/main/asciidoc/microprofile-lra-spec.adoc
@@ -387,6 +387,13 @@ will run in the context of an LRA and controls whether or not:
 - to throw an exception if the method returns particular HTTP status codes.
 - to enlist the class as a participant of the LRA.
 
+If the incoming context has already ended or does not exist the method will 
+immediately return 410 Gone HTTP status code.
+
+Similarly, if any of the preconditions listed in the Type element
+(see <<source-LRA,LRA.Type>>) of the LRA annotation are not met then the method
+will immediately return 412 Precondition Failed HTTP status code.
+
 When the @LRA annotation is defined multiple times, this is the order of precedence
 
 - On the JAX-RS method itself.
diff --git a/tck/src/main/java/org/eclipse/microprofile/lra/tck/participant/api/RecoveryResource.java b/tck/src/main/java/org/eclipse/microprofile/lra/tck/participant/api/RecoveryResource.java
index c3f2614..8f31d06 100644
--- a/tck/src/main/java/org/eclipse/microprofile/lra/tck/participant/api/RecoveryResource.java
+++ b/tck/src/main/java/org/eclipse/microprofile/lra/tck/participant/api/RecoveryResource.java
@@ -23,7 +23,6 @@ import org.eclipse.microprofile.lra.annotation.AfterLRA;
 import org.eclipse.microprofile.lra.annotation.Compensate;
 import org.eclipse.microprofile.lra.annotation.LRAStatus;
 import org.eclipse.microprofile.lra.annotation.ws.rs.LRA;
-import org.eclipse.microprofile.lra.tck.TckRecoveryTests;
 import org.eclipse.microprofile.lra.tck.service.LRAMetricService;
 import org.eclipse.microprofile.lra.tck.service.LRAMetricType;
 import org.eclipse.microprofile.lra.tck.service.LRATestService;
@@ -39,7 +38,6 @@ import javax.ws.rs.QueryParam;
 import javax.ws.rs.client.Entity;
 import javax.ws.rs.core.Response;
 import java.net.URI;
-import java.net.URL;
 import java.time.temporal.ChronoUnit;
 
 import static org.eclipse.microprofile.lra.annotation.ws.rs.LRA.LRA_HTTP_CONTEXT_HEADER;
@@ -71,16 +69,11 @@ public class RecoveryResource {
      */
     @GET
     @Path(PHASE_1)
-    public Response phase1(@QueryParam("timeout") @DefaultValue("false") boolean timeout,
-                           @HeaderParam(TckRecoveryTests.LRA_TCK_DEPLOYMENT_URL) URL deploymentURL) {
-        
-        lraTestService.start(deploymentURL);
-        
+    public Response phase1(@QueryParam("timeout") @DefaultValue("false") boolean timeout) {
         // start a new LRA and join it with this resource
         URI lra;
         Response response;
         
-
         response = lraTestService.getTCKSuiteTarget().path(RecoveryResource.RECOVERY_RESOURCE_PATH)
                 .path(timeout ? RecoveryResource.REQUIRED_TIMEOUT_PATH : RecoveryResource.REQUIRED_PATH)
                 .request().put(Entity.text(""));
@@ -100,9 +93,7 @@ public class RecoveryResource {
      */
     @GET
     @Path(PHASE_2)
-    public Response phase2(@HeaderParam(LRA_HEADER) URI lraId,
-                           @HeaderParam(TckRecoveryTests.LRA_TCK_DEPLOYMENT_URL) URL deploymentURL) {
-        lraTestService.start(deploymentURL);
+    public Response phase2(@HeaderParam(LRA_HEADER) URI lraId) {
         lraTestService.getLRAClient().cancelLRA(lraId);
         lraTestService.waitForCallbacks(lraId);
 
@@ -127,9 +118,7 @@ public class RecoveryResource {
 
     @GET
     @Path(TRIGGER_RECOVERY)
-    public Response triggerRecovery(@HeaderParam(LRA_HEADER) URI lraId,
-                                    @HeaderParam(TckRecoveryTests.LRA_TCK_DEPLOYMENT_URL) URL deploymentURL) {
-        lraTestService.start(deploymentURL);
+    public Response triggerRecovery(@HeaderParam(LRA_HEADER) URI lraId) {
         lraTestService.waitForRecovery(lraId);
         return Response.ok().build();
     } 


Test patch:
diff --git a/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckRecoveryTests.java b/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckRecoveryTests.java
index f0f43ff..d909a48 100644
--- a/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckRecoveryTests.java
+++ b/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckRecoveryTests.java
@@ -56,8 +56,6 @@ import java.util.logging.Logger;
  */
 @RunWith(Arquillian.class)
 public class TckRecoveryTests {
-    
-    public static final String LRA_TCK_DEPLOYMENT_URL = "LRA-TCK-Deployment-URL";
 
     private static final String DEPLOYMENT_NAME = "tck-recovery";
     private static final Logger LOGGER = Logger.getLogger(TckRecoveryTests.class.getName());
@@ -113,12 +111,8 @@ public class TckRecoveryTests {
         deployer.deploy(DEPLOYMENT_NAME);
         
         // invoke phase 1 inside the container (start new LRA and enlist resource)
-        Response response1 = deploymentTarget
-            .path(RecoveryResource.RECOVERY_RESOURCE_PATH)
-            .path(RecoveryResource.PHASE_1)
-            .request()
-            .header(LRA_TCK_DEPLOYMENT_URL, deploymentURL.toExternalForm())
-            .get();
+        Response response1 = deploymentTarget.path(RecoveryResource.RECOVERY_RESOURCE_PATH)
+            .path(RecoveryResource.PHASE_1).request().get();
         
         Assert.assertEquals(200, response1.getStatus());
         URI lra = URI.create(response1.readEntity(String.class));
@@ -135,7 +129,6 @@ public class TckRecoveryTests {
             .path(RecoveryResource.PHASE_2)
             .request()
                 .header(RecoveryResource.LRA_HEADER, lra)
-                .header(LRA_TCK_DEPLOYMENT_URL, deploymentURL.toExternalForm())
                 .get();
         
         Assert.assertEquals(response2.readEntity(String.class), 200, response2.getStatus());
@@ -167,9 +160,7 @@ public class TckRecoveryTests {
         Response response1 = deploymentTarget.path(RecoveryResource.RECOVERY_RESOURCE_PATH)
             .path(RecoveryResource.PHASE_1)
             .queryParam("timeout", true)
-            .request()
-            .header(LRA_TCK_DEPLOYMENT_URL, deploymentURL.toExternalForm())
-            .get();
+            .request().get();
 
         Assert.assertEquals(200, response1.getStatus());
         URI lra = URI.create(response1.readEntity(String.class));
@@ -201,7 +192,6 @@ public class TckRecoveryTests {
             .path(RecoveryResource.TRIGGER_RECOVERY)
             .request()
                 .header(RecoveryResource.LRA_HEADER, lra)
-                .header(LRA_TCK_DEPLOYMENT_URL, deploymentURL.toExternalForm())
                 .get();
 
         // invoke phase 2 inside the container (execute checks that verify that callbacks have been called)
@@ -209,7 +199,6 @@ public class TckRecoveryTests {
             .path(RecoveryResource.PHASE_2)
             .request()
                 .header(RecoveryResource.LRA_HEADER, lra)
-                .header(LRA_TCK_DEPLOYMENT_URL, deploymentURL.toExternalForm())
                 .get();
 
         Assert.assertEquals(response2.readEntity(String.class), 200, response2.getStatus());
diff --git a/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckTestBase.java b/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckTestBase.java
index b3da654..5fbf6f2 100644
--- a/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckTestBase.java
+++ b/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckTestBase.java
@@ -52,9 +52,6 @@ public class TckTestBase {
     
     @Inject
     private LRATestService lraTestService;
-
-    @Inject
-    private LRAMetricService lraMetricService;
     
     @ArquillianResource
     private URL deploymentURL;
@@ -84,7 +81,6 @@ public class TckTestBase {
         LOGGER.info("Running test: " + testName.getMethodName());
         
         lraTestService.start(deploymentURL);
-        lraMetricService.clear();
         this.lraClient = lraTestService.getLRAClient();
         this.tckSuiteTarget = lraTestService.getTCKSuiteTarget();
     }
diff --git a/tck/src/main/java/org/eclipse/microprofile/lra/tck/service/LRATestService.java b/tck/src/main/java/org/eclipse/microprofile/lra/tck/service/LRATestService.java
index 9e5b6ea..d6ac11b 100644
--- a/tck/src/main/java/org/eclipse/microprofile/lra/tck/service/LRATestService.java
+++ b/tck/src/main/java/org/eclipse/microprofile/lra/tck/service/LRATestService.java
@@ -38,6 +38,9 @@ import java.util.ServiceLoader;
 @ApplicationScoped
 public class LRATestService {
 
+    @Inject
+    LRAMetricService lraMetricService;
+    
     @Inject
     LraTckConfigBean config;
 
@@ -51,6 +54,7 @@ public class LRATestService {
 
     public void start(URL deploymentURL) {
         tckSuiteClient = ClientBuilder.newClient();
+        lraMetricService.clear();
         tckSuiteTarget = tckSuiteClient.target(URI.create(deploymentURL.toExternalForm()));
         lraClient = new LRAClientOps(tckSuiteTarget);
     }