issue122 PR review

tck/src/main/java/org/eclipse/microprofile/lra/tck/TckContextTests.java

@@ -77,7 +77,7 @@ public void testBasicContextPropagation() {
         String lra = invoke(NEW_LRA_PATH, PUT, null, 200, ContextTckResource.EndPhase.SUCCESS, 200);
         invoke(REQUIRED_LRA_PATH, PUT, lra, 200, ContextTckResource.EndPhase.SUCCESS, 200);
 
-        // verify that the resource was not asked to complete
+        // verify that the resource was asked to complete
         String completions = invoke(METRIC_PATH + "/" + Complete.class.getName(), GET, lra);
 
         assertEquals(testName.getMethodName() + ": Resource was not asked to complete",
@@ -89,7 +89,7 @@ public void testStatus() {
         // call a resource that begins and ends an LRA and coerces the resource to return ACCEPTED when asked to complete
         String lra = invoke(REQUIRED_LRA_PATH, PUT, null, 200, ContextTckResource.EndPhase.ACCEPTED, 202);
 
-        // verfiy that the resource was asked to complete and is in the state Conpleting
+        // verfiy that the resource was asked to complete and is in the state Completing
         String status = invoke(STATUS_PATH, HttpMethod.GET, lra, 202, ContextTckResource.EndPhase.SUCCESS, 200);
         assertEquals(testName.getMethodName() + ": participant is not completing", ParticipantStatus.Completing.name(), status);
 
@@ -106,7 +106,7 @@ public void testStatus() {
 
     @Test
     public void testLeave() {
-        // call a resource that begins but does not ends an LRA
+        // call a resource that begins but does not end an LRA
         String lra = invoke(NEW_LRA_PATH, PUT, null, 200, ContextTckResource.EndPhase.SUCCESS, 200);
         // verfiy that the resource is active
         String status = invoke(STATUS_PATH, HttpMethod.GET, lra, 200, ContextTckResource.EndPhase.SUCCESS, 200);
@@ -124,24 +124,25 @@ public void testLeave() {
 
 
         assertEquals(testName.getMethodName() + ": Resource left but was still asked to complete",
-                Integer.toString(0), completions);
+                "0", completions);
     }
 
     @Test
     public void testForget() {
+        String count;
         // call a resource that begins and ends an LRA and coerces the resource to return FAILED when asked to complete
         String lra = invoke(REQUIRED_LRA_PATH, PUT, null, 200, ContextTckResource.EndPhase.FAILED, 500);
 
         // trigger a replay attempt
         replayEndPhase(TCK_CONTEXT_RESOURCE_PATH);
 
         // the implementation should have called status which will have returned 500
-        String count1 = invoke(METRIC_PATH + "/" + Status.class.getName(), GET, lra);
-        assertEquals(testName.getMethodName() + " resource status should have been called", "1", count1);
+        count = invoke(METRIC_PATH + "/" + Status.class.getName(), GET, lra);
+        assertEquals(testName.getMethodName() + " resource status should have been called", "1", count);
 
         // the implementation should not call forget until it knows the particpant status
-        String count2 = invoke(METRIC_PATH + "/" + Forget.class.getName(), GET, lra);
-        assertEquals(testName.getMethodName() + " resource forget should not have been called", "0", count2);
+        count = invoke(METRIC_PATH + "/" + Forget.class.getName(), GET, lra);
+        assertEquals(testName.getMethodName() + " resource forget should not have been called", "0", count);
 
         // clear the fault
         invoke(STATUS_PATH, PUT, lra, 200, ContextTckResource.EndPhase.FAILED, 200);
@@ -150,11 +151,11 @@ public void testForget() {
         replayEndPhase(TCK_CONTEXT_RESOURCE_PATH);
 
         // the implementation should have called status again which will have returned 200
-        count1 = invoke(METRIC_PATH + "/" + Status.class.getName(), GET, lra);
-        assertEquals(testName.getMethodName() + " resource status should have been called again", "2", count1);
+        count = invoke(METRIC_PATH + "/" + Status.class.getName(), GET, lra);
+        assertEquals(testName.getMethodName() + " resource status should have been called again", "2", count);
         // the implementation should call forget since it knows the particpant status
-        count2 = invoke(METRIC_PATH + "/" + Forget.class.getName(), GET, lra);
-        assertEquals(testName.getMethodName() + " resource forget should have been called", "1", count2);
+        count = invoke(METRIC_PATH + "/" + Forget.class.getName(), GET, lra);
+        assertEquals(testName.getMethodName() + " resource forget should have been called", "1", count);
     }
 
     // invoke a method in an LRA context which performs various outgoing calls checking that the notion of active context

tck/src/main/java/org/eclipse/microprofile/lra/tck/TckTests.java

@@ -332,7 +332,7 @@ public void dependentLRA() throws WebApplicationException {
         Object lraHeader = response.getHeaders().getFirst(LRA_HTTP_CONTEXT_HEADER);
         String lraId = checkStatusReadAndCloseResponse(Response.Status.OK, response, resourcePath);
 
-        // LRAs started programatically should not be available to the caller in the context header
+        // LRAs started within the invoked remote method should not be available to the caller via the context header
         assertNull("JAX-RS response to PUT request should not have returned the header " + LRA_HTTP_CONTEXT_HEADER
                 + ". The test call went to " + resourcePath.getUri(), lraHeader);
 

tck/src/main/java/org/eclipse/microprofile/lra/tck/participant/api/ContextTckResource.java

@@ -46,7 +46,6 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -59,24 +58,35 @@
 @ApplicationScoped
 @Path(ContextTckResource.TCK_CONTEXT_RESOURCE_PATH)
 public class ContextTckResource {
-    private static final Logger LOGGER = Logger.getLogger(ContextTckResource.class.getName());
-
     public static final String TCK_CONTEXT_RESOURCE_PATH = "context-tck-resource";
 
-    public static final String RESET_PATH = "/reset";
     public static final String NEW_LRA_PATH = "/new-lra";
     public static final String REQUIRED_LRA_PATH = "/required-lra";
+    // resource path for testing that context on outbound and inbound calls made from an
+    // method annotated with @LRA are spec compliant
     public static final String CONTEXT_CHECK_LRA_PATH = "/context-check-lra";
     public static final String LEAVE_PATH = "/leave";
+    // resource path for reading and writing the participant status
     public static final String STATUS_PATH = "/status";
+    // resource path for reading stats relating to a participant in the context of a single LRA
     public static final String METRIC_PATH = "/count";
+    // resource path for clearing state
+    public static final String RESET_PATH = "/reset";
 
+    // headers for tests to indicate how the participant should respond to callbacks
+    // from the LRA implementation (complete, compensate, forget and status)
     public static final String LRA_TCK_FAULT_TYPE_HEADER = "Lra-Tck-Fault-Type";
     public static final String LRA_TCK_FAULT_CODE_HEADER = "Lra-Tck-Fault-Status";
+
+    // a header for tests to indicate which LRA stats are being queried/reset
     public static final String LRA_TCK_HTTP_CONTEXT_HEADER = "Lra-Tck-Context";
 
     private static final String REQUIRES_NEW_LRA_PATH = "/requires-new-lra";
 
+    /**
+     * A class to hold all of the metrics gathered in the context of a single LRA.
+     * We need stats per LRA since a misbehaving test may interfere with subsequent tests
+     */
     private static class LRAMetric {
         String lraId;
         Map<String, AtomicInteger> metrics = Stream.of(
@@ -117,33 +127,35 @@ private void incrementMetric(String name, String lraId) {
         metrics.get(lraId).increment(name);
     }
 
-    public enum EndPhase {ACCEPTED, FAILED, SUCCESS, HTTP_STATUS}
-
-    @Context
-    private UriInfo context;
+    /**
+     * An enum which controls the behaviour of participant when the
+     * LRA spec implementation invokes the compensation, completion,
+     * forget and status callbacks. It is used for testing the
+     * implementation is spec compliant.
+     */
+    public enum EndPhase {
+        ACCEPTED, // a participant reports that the complete/compensate is in progress
+        FAILED, // a particant reports that is is unable to complete/compensate
+        SUCCESS // clear the any injected behaviour
+    }
 
     private EndPhase endPhase = EndPhase.SUCCESS;
+    // control which status to return from participant callbacks
     private Response.Status endPhaseStatus = Response.Status.OK;
     private ParticipantStatus status = ParticipantStatus.Active;
 
+    // update the injected behaviour
     private void setEndPhase(String faultType, int faultCode) {
         if (faultType == null) {
             endPhase = EndPhase.SUCCESS;
             endPhaseStatus = Response.Status.OK;
         } else {
-            try {
-                endPhase = EndPhase.valueOf(faultType);
-                endPhaseStatus = Response.Status.fromStatusCode(faultCode);
-            } catch (RuntimeException e) {
-                // invalid or no enum so use the current value
-                LOGGER.warning("Invalid test data: endPhase ("
-                        + endPhase
-                        + ") or endPhaseStatus ("
-                        + endPhaseStatus + ")");
-            }
+            endPhase = EndPhase.valueOf(faultType);
+            endPhaseStatus = Response.Status.fromStatusCode(faultCode);
         }
     }
 
+    // reset any state in preparation for the next test
     @PUT
     @Path(RESET_PATH)
     public Response reset(@HeaderParam(LRA_TCK_HTTP_CONTEXT_HEADER) String lraId) {
@@ -158,17 +170,20 @@ public Response reset(@HeaderParam(LRA_TCK_HTTP_CONTEXT_HEADER) String lraId) {
         return Response.ok().build();
     }
 
+    // start a new LRA which remains active after the method returns
     @LRA(value = LRA.Type.REQUIRES_NEW, end = false)
     @PUT
     @Path(NEW_LRA_PATH)
     public Response newLRA(@HeaderParam(LRA_HTTP_CONTEXT_HEADER) String lraId,
                            @HeaderParam(LRA_TCK_FAULT_TYPE_HEADER) String tckFaultType,
                            @HeaderParam(LRA_TCK_FAULT_CODE_HEADER) int tckFaultCode) {
+        // check for a requests to inject particular behaviour
         setEndPhase(tckFaultType, tckFaultCode);
 
         return Response.ok().entity(lraId).build();
     }
 
+    // end an existing LRA or start and end a new one
     @LRA(value = LRA.Type.REQUIRED)
     @PUT
     @Path(REQUIRED_LRA_PATH)
@@ -187,6 +202,7 @@ public Response requiresNew(@HeaderParam(LRA_HTTP_CONTEXT_HEADER) String lraId)
         return Response.ok().entity(lraId).build();
     }
 
+    // test that outgoing calls do not affect the calling context
     @LRA(value = LRA.Type.REQUIRED)
     @PUT
     @Path(CONTEXT_CHECK_LRA_PATH)
@@ -215,7 +231,12 @@ public Response contextCheck(@HeaderParam(LRA_HTTP_CONTEXT_HEADER) String lraId)
         return Response.ok().entity(lraId).build();
     }
 
-
+    /**
+     * Query the current value of a metric in the context of an LRA
+     * @param lraId the LRA that owns the metric
+     * @param metric the name of the metric
+     * @return the current value of the metric
+     */
     @GET
     @Path(METRIC_PATH + "/{metric}")
     public Response count(@HeaderParam(LRA_TCK_HTTP_CONTEXT_HEADER) String lraId,
@@ -272,6 +293,7 @@ public Response forget(@HeaderParam(LRA_HTTP_CONTEXT_HEADER) String lraId) {
         return Response.status(endPhaseStatus).entity(status.name()).build();
     }
 
+    // clear any injected participant behaviour
     @PUT
     @Path(STATUS_PATH)
     public Response clearStatus(@HeaderParam(LRA_TCK_HTTP_CONTEXT_HEADER) String lraId,
@@ -293,6 +315,7 @@ public Response clearStatus(@HeaderParam(LRA_TCK_HTTP_CONTEXT_HEADER) String lra
         return Response.ok().entity(status.name()).build();
     }
 
+    // modify participant responses based on injected behavour specified by the test
     private Response getEndPhaseResponse(boolean completing) {
         switch (endPhase) {
             case ACCEPTED:
@@ -301,16 +324,20 @@ private Response getEndPhaseResponse(boolean completing) {
             case FAILED:
                 status = completing ? ParticipantStatus.FailedToComplete : ParticipantStatus.FailedToCompensate;
                 return Response.status(endPhaseStatus).entity(status.name()).build();
-            case HTTP_STATUS:
-                status = completing ? ParticipantStatus.Completed : ParticipantStatus.Compensated;
-                return Response.status(endPhaseStatus).entity(status.name()).build();
             case SUCCESS: /* FALLTHRU */
             default:
                 status = completing ? ParticipantStatus.Completed : ParticipantStatus.Compensated;
                 return Response.status(Response.Status.OK).entity(status.name()).build();
         }
     }
 
+    /*
+     * helper methods for testing how outgoing/incoming JAX-RS calls affect the notion of the current context
+     */
+
+    @Context
+    private UriInfo context;
+
     private String getActiveLRA() {
         return restPutInvocation(null, TCK_NON_PARTICIPANT_RESOURCE_PATH, SUPPORTS_PATH, "");
     }