Improvements based on code review.

11 files changed, 164 insertions, 208 deletions

diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/Result.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/Result.java
index 4488f8603c2..6b3e716c270 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/Result.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/Result.java
@@ -2,9 +2,13 @@
 package com.yahoo.vespa.http.client;
 
 import com.yahoo.vespa.http.client.config.Endpoint;
+import com.yahoo.vespa.http.client.core.Document;
 import com.yahoo.vespa.http.client.core.Exceptions;
 import net.jcip.annotations.Immutable;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -16,55 +20,90 @@ import java.util.List;
  * @since 5.1.20
  */
 // This should be an interface, but in order to be binary compatible during refactoring we made it abstract.
-public abstract class Result {
+public class Result {
+    public enum ResultType {
+        OPERATION_EXECUTED,
+        TRANSITIVE_ERROR,
+        CONDITION_NOT_MET,
+        FATAL_ERROR
+    }
+
+    private final Document document;
+    private final boolean success;
+    private final boolean _transient;
+    private final List<Detail> details;
+    private final String localTrace;
+
+    public Result(Document document, Collection<Detail> values, StringBuilder localTrace) {
+        this.document = document;
+        this.details = Collections.unmodifiableList(new ArrayList<>(values));
+        boolean totalSuccess = true;
+        boolean totalTransient = true;
+        for (Detail d : details) {
+            if (d.getResultType() != ResultType.OPERATION_EXECUTED) {totalSuccess = false; }
+            if (d.getResultType() != ResultType.TRANSITIVE_ERROR) {totalTransient = false; }
+        }
+        this.success = totalSuccess;
+        this._transient = totalTransient;
+        this.localTrace = localTrace == null ? null : localTrace.toString();
+    }
+
+
     /**
      * Returns the document ID that this Result is for.
      *
      * @return the document ID that this Result is for.
      */
-    abstract public String getDocumentId();
+    public String getDocumentId() {
+        return document.getDocumentId();
+    }
 
     /**
      * Returns the document data.
      * @return data as bytebuffer.
      */
-    abstract public CharSequence getDocumentDataAsCharSequence();
+    public CharSequence getDocumentDataAsCharSequence() {
+        return document.getDataAsString();
+    }
 
     /**
      * Returns the context of the object if any.
      * @return context.
      */
-    abstract public Object getContext();
+    public Object getContext() {
+        return document.getContext();
+    }
 
     /**
-     * Returns true if the operation was successful. If at least one {@link Detail}
+     * Returns true if the operation(s) was successful. If at least one {@link Detail}
      * in {@link #getDetails()} is unsuccessful, this will return false.
      *
      * @return true if the operation was successful.
      */
-    abstract public boolean isSuccess();
-
+    public boolean isSuccess() {
+        return success;
+    }
     /**
+     * @deprecated use resultType on items getDetails() to check  operations.
      * Returns true if an error is transient, false if it is permanent. Irrelevant
      * if {@link #isSuccess()} is true (returns true in those cases).
      *
      * @return true if an error is transient (or there is no error), false otherwise.
      */
-    abstract public boolean isTransient();
-
-    /**
-     * Returns true if a error was caused by condition-not-met in the document operation.
-     * @return true if condition is not met.
-     */
-    abstract public boolean isConditionNotMet();
+    @Deprecated
+    public boolean isTransient() {
+        return _transient;
+    }
 
-    abstract public List<Detail> getDetails();
+    public List<Detail> getDetails() { return details; }
 
     /**
      * Checks if operation has been set up with local tracing.
      * @return true if operation has local trace.
      */
-    abstract public boolean hasLocalTrace();
+    public boolean hasLocalTrace() {
+        return localTrace != null;
+    }
 
     /**
      * Information in a Result for a single operation sent to a single endpoint.
@@ -74,28 +113,22 @@ public abstract class Result {
      */
     @Immutable
     public static final class Detail {
+        private final ResultType resultType;
         private final Endpoint endpoint;
-        private final boolean success;
-        private final boolean _transient;
-        private final boolean isConditionNotMet;
         private final Exception exception;
         private final String traceMessage;
         private final long timeStampMillis = System.currentTimeMillis();
 
-        public Detail(Endpoint endpoint, boolean success, boolean _transient, boolean isConditionNotMet, String traceMessage, Exception e) {
+        public Detail(Endpoint endpoint, ResultType resultType, String traceMessage, Exception e) {
             this.endpoint = endpoint;
-            this.success = success;
-            this._transient = _transient;
-            this.isConditionNotMet = isConditionNotMet;
+            this.resultType = resultType;
             this.exception = e;
             this.traceMessage = traceMessage;
         }
 
         public Detail(Endpoint endpoint) {
             this.endpoint = endpoint;
-            this.success = true;
-            this._transient = true;
-            this.isConditionNotMet = false;
+            this.resultType = ResultType.OPERATION_EXECUTED;
             this.exception = null;
             this.traceMessage = null;
         }
@@ -110,30 +143,33 @@ public abstract class Result {
         }
 
         /**
+         * @deprecated use getResultType.
          * Returns true if the operation was successful.
          *
          * @return true if the operation was successful.
          */
+        @Deprecated
         public boolean isSuccess() {
-            return success;
+            return resultType == ResultType.OPERATION_EXECUTED;
         }
 
         /**
+         * @deprecated use getResultType.
          * Returns true if an error is transient, false if it is permanent. Irrelevant
          * if {@link #isSuccess()} is true (returns true in those cases).
          *
          * @return true if an error is transient (or there is no error), false otherwise.
          */
+        @Deprecated
         public boolean isTransient() {
-            return _transient;
+            return resultType == ResultType.TRANSITIVE_ERROR || resultType == ResultType.OPERATION_EXECUTED;
         }
 
         /**
-         * Returns true if a condition in the document operation was not met.
-         * @return if condition not met in operation.
+         * Returns the result of the operation.
          */
-        public boolean isConditionNotMet() {
-            return isConditionNotMet;
+        public ResultType getResultType() {
+            return resultType;
         }
 
         /**
@@ -157,11 +193,7 @@ public abstract class Result {
         public String toString() {
             StringBuilder b = new StringBuilder();
             b.append("Detail ");
-            b.append("success=").append(success).append(" ");
-            if (!success) {
-                b.append("transient=").append(_transient).append(" ");
-                b.append("conditionNotMet=").append(isConditionNotMet).append(" ");
-            }
+            b.append("resultType=").append(resultType);
             if (exception != null) {
                 b.append("exception='").append(Exceptions.toMessageString(exception)).append("' ");
             }
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/OperationStatus.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/OperationStatus.java
index d0355b0e269..cbf690c1f14 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/OperationStatus.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/OperationStatus.java
@@ -8,7 +8,7 @@ import java.util.Iterator;
 
 
 /**
- * Wrapper to represent the result of a single operation fed to Vespa.
+ * Serialization/deserialization class for the result of a single document operation against Vespa.
  *
  * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
  * @since 5.1
@@ -26,6 +26,14 @@ public final class OperationStatus {
     private static final char SEPARATOR = ' ';
     private static final Splitter spaceSep = Splitter.on(SEPARATOR);
 
+    /**
+     * Constructor
+     * @param message some human readable information what happened
+     * @param operationId the doc ID for the operation
+     * @param errorCode if it is success, transitive, or fatal
+     * @param isConditionNotMet if error is due to condition not met
+     * @param traceMessage any tracemessage
+     */
     public OperationStatus(String message, String operationId, ErrorCode errorCode, boolean isConditionNotMet, String traceMessage) {
         this.isConditionNotMet = isConditionNotMet;
         this.message = message;
@@ -35,7 +43,7 @@ public final class OperationStatus {
     }
 
     /**
-     * Parse a single rendered OperationStatus. White space may be padded after
+     * Parse a single rendered OperationStatus string. White space may be padded after
      * and before the given status.
      *
      * @param singleLine
@@ -73,6 +81,9 @@ public final class OperationStatus {
         return new OperationStatus(message, operationId, errorCode, isConditionNotMet, traceMessage);
     }
 
+    /**
+     * @return a string representing the status.
+     */
     public String render() {
         StringBuilder s = new StringBuilder();
         Encoder.encode(operationId, s).append(SEPARATOR);
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/api/ResultImpl.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/api/ResultImpl.java
deleted file mode 100644
index 3e3f6897ba8..00000000000
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/api/ResultImpl.java
+++ /dev/null
@@ -1,94 +0,0 @@
-// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.http.client.core.api;
-
-import com.yahoo.vespa.http.client.Result;
-import com.yahoo.vespa.http.client.core.Document;
-import net.jcip.annotations.Immutable;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-
-/**
- * The result of an operation written to an OutputStream returned by
- * {@link com.yahoo.vespa.http.client.Session#stream(CharSequence)}. A Result refers to a single document,
- * but may contain more than one Result.Detail instances, as these pertains to a
- * single endpoint, and a Result may wrap data for multiple endpoints.
- *
- * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a>
- * @since 5.1.20
- */
-@Immutable
-final public class ResultImpl extends Result {
-    private final Document document;
-    private final boolean success;
-    private final boolean _transient;
-    private final boolean isConditionNotMet;
-    private final List<Detail> details;
-    private final String localTrace;
-
-    public ResultImpl(Document document, Collection<Detail> values, StringBuilder localTrace) {
-        this.document = document;
-        this.details = Collections.unmodifiableList(new ArrayList<>(values));
-        boolean totalSuccess = true;
-        boolean totalTransient = true;
-        boolean isConditionNotMet = true;
-        for (Detail d : details) {
-            if (!d.isSuccess()) {totalSuccess = false; }
-            if (!d.isTransient()) {totalTransient = false; }
-            if (!d.isConditionNotMet()) { isConditionNotMet = false; }
-        }
-        this.success = totalSuccess;
-        this._transient = totalTransient;
-        this.isConditionNotMet = isConditionNotMet;
-        this.localTrace = localTrace == null ? null : localTrace.toString();
-    }
-
-    @Override
-    public String getDocumentId() {
-        return document.getDocumentId();
-    }
-
-    @Override
-    public CharSequence getDocumentDataAsCharSequence() {
-        return document.getDataAsString();
-    }
-
-    @Override
-    public Object getContext() {
-        return document.getContext();
-    }
-
-    @Override
-    public boolean isSuccess() {
-        return success;
-    }
-
-    @Override
-    public boolean isTransient() {
-        return _transient;
-    }
-
-    @Override
-    public boolean isConditionNotMet() { return isConditionNotMet; }
-
-
-    @Override
-    public List<Detail> getDetails() { return details; }
-
-    @Override
-    public boolean hasLocalTrace() {
-        return localTrace != null;
-    }
-
-    @Override
-    public String toString() {
-        StringBuilder b = new StringBuilder();
-        b.append("Result for '").append(document.getDocumentId());
-        if (localTrace != null) {
-            b.append(localTrace);
-        }
-        return b.toString();
-    }
-}
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/EndpointResultQueue.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/EndpointResultQueue.java
index 948d2e7f865..37395f87fd8 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/EndpointResultQueue.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/EndpointResultQueue.java
@@ -104,7 +104,7 @@ class EndpointResultQueue {
     }
 
     private synchronized void failedOperationId(String operationId, Exception exception) {
-        EndpointResult endpointResult = EndPointResultFactory.createError(endpoint, operationId, false, exception);
+        EndpointResult endpointResult = EndPointResultFactory.createError(endpoint, operationId, exception);
         operationProcessor.resultReceived(endpointResult, clusterId);
     }
 
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java
index 9697256fbde..a6e0c9092db 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.http.client.core.communication;
 
 import com.google.common.annotations.Beta;
+import com.yahoo.vespa.http.client.Result;
 import com.yahoo.vespa.http.client.config.Endpoint;
 import com.yahoo.vespa.http.client.core.Document;
 import com.yahoo.vespa.http.client.core.operationProcessor.EndPointResultFactory;
@@ -245,8 +246,7 @@ class IOThread implements Runnable, AutoCloseable {
         statusReceivedCounter.addAndGet(endpointResults.size());
         int transientErrors = 0;
         for (EndpointResult endpointResult : endpointResults) {
-            if (! endpointResult.getDetail().isSuccess() &&
-                    endpointResult.getDetail().isTransient()) {
+            if (endpointResult.getDetail().getResultType() == Result.ResultType.TRANSITIVE_ERROR) {
                 transientErrors++;
             }
             resultQueue.resultReceived(endpointResult, clusterId);
@@ -404,7 +404,7 @@ class IOThread implements Runnable, AutoCloseable {
 
         for (Document document : documentQueue.removeAllDocuments()) {
             EndpointResult endpointResult=
-                    EndPointResultFactory.createError(endpoint, document.getOperationId(), false, exception);
+                    EndPointResultFactory.createError(endpoint, document.getOperationId(), exception);
             resultQueue.failOperation(endpointResult, clusterId);
         }
     }
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/DocumentSendInfo.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/DocumentSendInfo.java
index 89c2ecc8ace..b25678d2b9d 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/DocumentSendInfo.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/DocumentSendInfo.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.http.client.core.operationProcessor;
 
 import com.yahoo.vespa.http.client.Result;
 import com.yahoo.vespa.http.client.core.Document;
-import com.yahoo.vespa.http.client.core.api.ResultImpl;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -46,7 +45,7 @@ class DocumentSendInfo {
     }
 
     public Result createResult() {
-        return new ResultImpl(document, detailByClusterId.values(), localTrace);
+        return new Result(document, detailByClusterId.values(), localTrace);
     }
 
     int incRetries(int clusterId, Result.Detail detail) {
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/EndPointResultFactory.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/EndPointResultFactory.java
index b357efc82d4..8c71af69100 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/EndPointResultFactory.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/EndPointResultFactory.java
@@ -41,13 +41,30 @@ public final class EndPointResultFactory {
     }
 
     public static EndpointResult createError(
-            Endpoint endpoint, String operationId, boolean isConditionNotMetError, Exception exception) {
-        return new EndpointResult(operationId, new Result.Detail(endpoint, false, false, isConditionNotMetError, null, exception));
+            Endpoint endpoint, String operationId, Exception exception) {
+        return new EndpointResult(operationId, new Result.Detail(
+                endpoint,  Result.ResultType.FATAL_ERROR, null, exception));
     }
 
     public static EndpointResult createTransientError(
             Endpoint endpoint, String operationId, Exception exception) {
-        return new EndpointResult(operationId, new Result.Detail(endpoint, false, true, false, null, exception));
+        return new EndpointResult(operationId, new Result.Detail(
+                endpoint, Result.ResultType.TRANSITIVE_ERROR, null, exception));
+    }
+
+    private static Result.ResultType replyToResultType(OperationStatus reply) {
+        final Result.ResultType resultType;
+        // The ordering below is important, e.g. if success, it is never a transient error even if isTransient is true.
+        if (reply.errorCode.isSuccess()) {
+            return Result.ResultType.OPERATION_EXECUTED;
+        }
+        if (reply.isConditionNotMet) {
+            return Result.ResultType.CONDITION_NOT_MET;
+        }
+        if (reply.errorCode.isTransient()) {
+            return Result.ResultType.TRANSITIVE_ERROR;
+        }
+        return Result.ResultType.FATAL_ERROR;
     }
 
     private static EndpointResult parseResult(String line, Endpoint endpoint) {
@@ -69,9 +86,7 @@ public final class EndPointResultFactory {
             return new EndpointResult(
                     reply.operationId,
                     new Result.Detail(endpoint,
-                            reply.errorCode.isSuccess(),
-                            reply.errorCode.isTransient(),
-                            reply.isConditionNotMet,
+                            replyToResultType(reply),
                             reply.traceMessage,
                             exception));
         } catch (Throwable t) {
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/OperationProcessor.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/OperationProcessor.java
index 199c5992e8b..b53476d2b18 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/OperationProcessor.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/OperationProcessor.java
@@ -110,7 +110,7 @@ public class OperationProcessor {
     private boolean retriedThis(EndpointResult endpointResult, DocumentSendInfo documentSendInfo, int clusterId) {
         final Result.Detail detail = endpointResult.getDetail();
         // If success, no retries to do.
-        if (detail.isSuccess()) {
+        if (detail.getResultType() == Result.ResultType.OPERATION_EXECUTED) {
             return false;
         }
 
@@ -125,7 +125,8 @@ public class OperationProcessor {
         }
         // TODO: Return proper error code in structured data in next version of internal API.
         // Error codes from messagebus/src/cpp/messagebus/errorcode.h
-        boolean retryThisOperation = detail.isTransient() ||
+        boolean retryThisOperation =
+                detail.getResultType() == Result.ResultType.TRANSITIVE_ERROR ||
                 exceptionMessage.contains("SEND_QUEUE_CLOSED") ||
                 exceptionMessage.contains("ILLEGAL_ROUTE") ||
                 exceptionMessage.contains("NO_SERVICES_FOR_ROUTE") ||
@@ -247,7 +248,6 @@ public class OperationProcessor {
                     EndPointResultFactory.createError(
                             eio.getEndpoint(),
                             document.getOperationId(),
-                            false,
                             eio),
                     clusterConnection.getClusterId());
         }
diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPIMultiClusterTest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPIMultiClusterTest.java
index e401fe5cfe6..dd12cacf296 100644
--- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPIMultiClusterTest.java
+++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPIMultiClusterTest.java
@@ -19,6 +19,7 @@ import static com.yahoo.vespa.http.client.TestUtils.getResults;
 import static com.yahoo.vespa.http.client.V3HttpAPITest.documents;
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 
 /**
  * Only runs on screwdriver to save time!
@@ -129,11 +130,7 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 assertThat(r, not(nullValue()));
                 assertThat(r.getDetails().toString(), r.isSuccess(), is(false));
                 assertThat(r.getDetails().size(), is(2));
-                // One of the details should be true and one false.
-                assertThat(
-                        r.getDetails().toString(),
-                        r.getDetails().get(0).isSuccess() ^ r.getDetails().get(1).isSuccess(),
-                        is(true));
+                assert(r.getDetails().get(0).getResultType() != r.getDetails().get(1).getResultType());
             }
             assertThat(results.isEmpty(), is(true));
         }
@@ -224,9 +221,9 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 for (Result.Detail detail : r.getDetails()) {
                     details.put(detail.getEndpoint(), detail);
                 }
-                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).isSuccess(), is(true));
-                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).isSuccess(), is(true));
-                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).isSuccess(), is(false));
+                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
+                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
+                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             }
             assertThat(results.isEmpty(), is(true));
         }
@@ -281,9 +278,9 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 for (Result.Detail detail : r.getDetails()) {
                     details.put(detail.getEndpoint(), detail);
                 }
-                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).isSuccess(), is(false));
-                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).isSuccess(), is(false));
-                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).isSuccess(), is(false));
+                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
+                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
+                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             }
             assertThat(results.isEmpty(), is(true));
         }
@@ -339,10 +336,10 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 for (Result.Detail detail : r.getDetails()) {
                     details.put(detail.getEndpoint(), detail);
                 }
-                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).isSuccess(), is(true));
-                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).isSuccess(), is(true));
-                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).isSuccess(), is(false));
-                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).isTransient(), is(true));
+                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
+                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
+                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
+                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             }
             assertThat(results.isEmpty(), is(true));
         }
@@ -398,9 +395,9 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 for (Result.Detail detail : r.getDetails()) {
                     details.put(detail.getEndpoint(), detail);
                 }
-                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).isSuccess(), is(true));
-                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).isSuccess(), is(true));
-                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).isSuccess(), is(false));
+                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
+                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
+                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             }
             assertThat(results.isEmpty(), is(true));
         }
@@ -454,9 +451,9 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 for (Result.Detail detail : r.getDetails()) {
                     details.put(detail.getEndpoint(), detail);
                 }
-                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).isSuccess(), is(false));
-                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).isSuccess(), is(false));
-                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).isSuccess(), is(false));
+                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
+                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
+                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             }
             assertThat(results.isEmpty(), is(true));
         }
@@ -506,11 +503,9 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 details.put(detail.getEndpoint(), detail);
             }
             Result.Detail failed = details.remove(Endpoint.create("localhost", serverC.getPort(), false));
-            assertThat(failed.toString(), failed.isSuccess(), is(false));
-            assertThat(failed.toString(), failed.isTransient(), is(true));
+            assertThat(failed.getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             for (Result.Detail detail : details.values()) {
-                assertThat(detail.toString(), detail.isSuccess(), is(true));
-                assertThat(detail.toString(), detail.isTransient(), is(true));
+                assertThat(detail.getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
             }
         }
     }
@@ -562,9 +557,9 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 for (Result.Detail detail : r.getDetails()) {
                     details.put(detail.getEndpoint(), detail);
                 }
-                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).isSuccess(), is(false));
-                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).isSuccess(), is(false));
-                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).isSuccess(), is(false));
+                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
+                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
+                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             }
             assertThat(results.isEmpty(), is(true));
         }
@@ -617,9 +612,9 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 for (Result.Detail detail : r.getDetails()) {
                     details.put(detail.getEndpoint(), detail);
                 }
-                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).isSuccess(), is(true));
-                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).isSuccess(), is(true));
-                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).isSuccess(), is(false));
+                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
+                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
+                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             }
             assertThat(results.isEmpty(), is(true));
         }
@@ -665,8 +660,7 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
             assertThat(r, not(nullValue()));
             assertThat(r.getDetails().toString(), r.isSuccess(), is(false));
             for (Result.Detail detail : r.getDetails()) {
-                assertThat(detail.toString(), detail.isSuccess(), is(false));
-                assertThat(detail.toString(), detail.isTransient(), is(true));
+                assertThat(detail.getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             }
         }
     }
@@ -715,11 +709,9 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 details.put(detail.getEndpoint(), detail);
             }
             Result.Detail failed = details.remove(Endpoint.create("localhost", serverC.getPort(), false));
-            assertThat(failed.toString(), failed.isSuccess(), is(false));
-            assertThat(failed.toString(), failed.isTransient(), is(true));
+            assertThat(failed.getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             for (Result.Detail detail : details.values()) {
-                assertThat(detail.toString(), detail.isSuccess(), is(true));
-                assertThat(detail.toString(), detail.isTransient(), is(true));
+                assertThat(detail.getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
             }
         }
     }
@@ -764,8 +756,7 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
             assertThat(r, not(nullValue()));
             assertThat(r.getDetails().toString(), r.isSuccess(), is(false));
             for (Result.Detail detail : r.getDetails()) {
-                assertThat(detail.toString(), detail.isSuccess(), is(false));
-                assertThat(detail.toString(), detail.isTransient(), is(true));
+                assertThat(detail.getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             }
         }
     }
@@ -815,9 +806,9 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 for (Result.Detail detail : r.getDetails()) {
                     details.put(detail.getEndpoint(), detail);
                 }
-                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).isSuccess(), is(true));
-                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).isSuccess(), is(true));
-                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).isSuccess(), is(false));
+                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
+                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
+                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             }
             assertThat(results.isEmpty(), is(true));
         }
@@ -868,9 +859,9 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 for (Result.Detail detail : r.getDetails()) {
                     details.put(detail.getEndpoint(), detail);
                 }
-                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).isSuccess(), is(false));
-                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).isSuccess(), is(false));
-                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).isSuccess(), is(false));
+                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
+                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
+                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             }
             assertThat(results.isEmpty(), is(true));
         }
@@ -929,9 +920,9 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 for (Result.Detail detail : r.getDetails()) {
                     details.put(detail.getEndpoint(), detail);
                 }
-                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).isSuccess(), is(true));
-                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).isSuccess(), is(true));
-                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).isSuccess(), is(true));
+                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
+                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
+                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
 
             }
             assertThat(results.isEmpty(), is(true));
@@ -987,9 +978,9 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
                 for (Result.Detail detail : r.getDetails()) {
                     details.put(detail.getEndpoint(), detail);
                 }
-                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).isSuccess(), is(false));
-                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).isSuccess(), is(false));
-                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).isSuccess(), is(false));
+                assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).getResultType(), is(Result.ResultType.FATAL_ERROR));
+                assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).getResultType(), is(Result.ResultType.FATAL_ERROR));
+                assertThat(details.get(Endpoint.create("localhost", serverC.getPort(), false)).getResultType(), is(Result.ResultType.FATAL_ERROR));
             }
             assertThat(results.isEmpty(), is(true));
         }
@@ -1040,8 +1031,8 @@ public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
             for (Result.Detail detail : r.getDetails()) {
                 details.put(detail.getEndpoint(), detail);
             }
-            assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).isSuccess(), is(false));
-            assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).isSuccess(), is(true));
+            assertThat(details.get(Endpoint.create("localhost", serverA.getPort(), false)).getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
+            assertThat(details.get(Endpoint.create("localhost", serverB.getPort(), false)).getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
 
 
             //Set B in bad state => B returns bad request.
diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPITest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPITest.java
index 68e86c86929..6ca4a77dcb9 100644
--- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPITest.java
+++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPITest.java
@@ -84,9 +84,10 @@ public class V3HttpAPITest extends TestOnCiBuildingSystemOnly {
             TestDocument document = documents.get(0);
             Result r = results.remove(document.getDocumentId());
             assertThat(r, not(nullValue()));
-            assertThat(r.isConditionNotMet(), is(conditionNotMet));
+            if (conditionNotMet) {
+                assertThat(r.getDetails().iterator().next().getResultType(), is(Result.ResultType.CONDITION_NOT_MET));
+            }
             assertThat(r.getDetails().toString(), r.isSuccess(), is(false));
-            assertThat(r.getDetails().toString(), r.isConditionNotMet(), is(conditionNotMet));
             assertThat(results.isEmpty(), is(true));
         }
     }
@@ -174,7 +175,7 @@ public class V3HttpAPITest extends TestOnCiBuildingSystemOnly {
                 Result r = results.remove(document.getDocumentId());
                 assertThat(r, not(nullValue()));
                 assertThat(r.getDetails().toString(), r.isSuccess(), is(false));
-                assertThat(r.isTransient(), is(true));
+                assertThat(r.getDetails().iterator().next().getResultType(), is(Result.ResultType.TRANSITIVE_ERROR));
             }
             assertThat(results.isEmpty(), is(true));
         }
diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/communication/IOThreadTest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/communication/IOThreadTest.java
index 61965fa68b1..8c90c1d4f92 100644
--- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/communication/IOThreadTest.java
+++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/communication/IOThreadTest.java
@@ -1,6 +1,7 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.http.client.core.communication;
 
+import com.yahoo.vespa.http.client.Result;
 import com.yahoo.vespa.http.client.V3HttpAPITest;
 import com.yahoo.vespa.http.client.core.Document;
 import com.yahoo.vespa.http.client.core.EndpointResult;
@@ -45,10 +46,11 @@ public class IOThreadTest {
         doAnswer(invocation -> {
             EndpointResult endpointResult = (EndpointResult) invocation.getArguments()[0];
             assertThat(endpointResult.getOperationId(), is(expectedDocIdFail));
-            assertThat(endpointResult.getDetail().isSuccess(), is(false));
             assertThat(endpointResult.getDetail().getException().toString(),
                     containsString(expectedException));
-            assertThat(endpointResult.getDetail().isTransient(), is(isTransient));
+            assertThat(endpointResult.getDetail().getResultType(), is(
+                    isTransient ? Result.ResultType.TRANSITIVE_ERROR : Result.ResultType.FATAL_ERROR));
+
             latch.countDown();
             return null;
         }).when(endpointResultQueue).failOperation(anyObject(), eq(0));
@@ -56,8 +58,7 @@ public class IOThreadTest {
         doAnswer(invocation -> {
             EndpointResult endpointResult = (EndpointResult) invocation.getArguments()[0];
             assertThat(endpointResult.getOperationId(), is(expectedDocIdOk));
-            assertThat(endpointResult.getDetail().isSuccess(), is(true));
-            assertThat(endpointResult.getDetail().isTransient(), is(isTransient));
+            assertThat(endpointResult.getDetail().getResultType(), is(Result.ResultType.OPERATION_EXECUTED));
             latch.countDown();
             return null;
         }).when(endpointResultQueue).resultReceived(anyObject(), eq(0));