Add tests and more correct status code handling

Author: bozaro

gitlfs-client/src/main/java/ru/bozaro/gitlfs/client/Client.java

@@ -37,6 +37,10 @@ public class Client {
   private static final int MAX_RETRY_COUNT = 2;
   private static final int MAX_REDIRECT_COUNT = 5;
   @NotNull
+  private static final int[] DEFAULT_HTTP_SUCCESS = new int[]{
+      HttpStatus.SC_OK
+  };
+  @NotNull
   private final ObjectMapper mapper;
   @NotNull
   private final AuthProvider authProvider;
@@ -302,7 +306,17 @@ protected <T extends HttpMethod, R> R doRequest(@Nullable Link link, @NotNull Re
           ++retryCount;
           continue;
       }
-      return task.processResponse(mapper, request);
+      int[] success = task.statusCodes();
+      if (success == null) {
+        success = DEFAULT_HTTP_SUCCESS;
+      }
+      for (int item : success) {
+        if (request.getStatusCode() == item) {
+          return task.processResponse(mapper, request);
+        }
+      }
+      // Unexpected status code.
+      throw new RequestException(request);
     }
   }
 

gitlfs-client/src/main/java/ru/bozaro/gitlfs/client/internal/JsonPost.java

@@ -3,11 +3,10 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.commons.httpclient.HttpMethod;
-import org.apache.commons.httpclient.HttpStatus;
 import org.apache.commons.httpclient.methods.ByteArrayRequestEntity;
 import org.apache.commons.httpclient.methods.PostMethod;
 import org.jetbrains.annotations.NotNull;
-import ru.bozaro.gitlfs.client.exceptions.RequestException;
+import org.jetbrains.annotations.Nullable;
 
 import java.io.IOException;
 
@@ -40,14 +39,15 @@ public HttpMethod createRequest(@NotNull ObjectMapper mapper, @NotNull String ur
     return method;
   }
 
+  @Nullable
+  @Override
+  public int[] statusCodes() {
+    return null;
+  }
+
   @NotNull
   @Override
   public Res processResponse(@NotNull ObjectMapper mapper, @NotNull HttpMethod request) throws IOException {
-    switch (request.getStatusCode()) {
-      case HttpStatus.SC_OK:
-        return mapper.readValue(request.getResponseBodyAsStream(), type);
-      default:
-        throw new RequestException(request);
-    }
+    return mapper.readValue(request.getResponseBodyAsStream(), type);
   }
 }

gitlfs-client/src/main/java/ru/bozaro/gitlfs/client/internal/MetaGet.java

@@ -5,7 +5,7 @@
 import org.apache.commons.httpclient.HttpStatus;
 import org.apache.commons.httpclient.methods.GetMethod;
 import org.jetbrains.annotations.NotNull;
-import ru.bozaro.gitlfs.client.exceptions.RequestException;
+import org.jetbrains.annotations.Nullable;
 import ru.bozaro.gitlfs.common.data.ObjectRes;
 
 import java.io.IOException;
@@ -27,6 +27,15 @@ public HttpMethod createRequest(@NotNull ObjectMapper mapper, @NotNull String ur
     return req;
   }
 
+  @Nullable
+  @Override
+  public int[] statusCodes() {
+    return new int[]{
+        HttpStatus.SC_OK,
+        HttpStatus.SC_NOT_FOUND,
+    };
+  }
+
   @Override
   public ObjectRes processResponse(@NotNull ObjectMapper mapper, @NotNull HttpMethod request) throws IOException {
     switch (request.getStatusCode()) {
@@ -35,7 +44,7 @@ public ObjectRes processResponse(@NotNull ObjectMapper mapper, @NotNull HttpMeth
       case HttpStatus.SC_NOT_FOUND:
         return null;
       default:
-        throw new RequestException(request);
+        throw new IllegalStateException();
     }
   }
 }

gitlfs-client/src/main/java/ru/bozaro/gitlfs/client/internal/MetaPost.java

@@ -7,7 +7,7 @@
 import org.apache.commons.httpclient.methods.ByteArrayRequestEntity;
 import org.apache.commons.httpclient.methods.PostMethod;
 import org.jetbrains.annotations.NotNull;
-import ru.bozaro.gitlfs.client.exceptions.RequestException;
+import org.jetbrains.annotations.Nullable;
 import ru.bozaro.gitlfs.common.data.Meta;
 import ru.bozaro.gitlfs.common.data.ObjectRes;
 
@@ -39,6 +39,15 @@ public HttpMethod createRequest(@NotNull ObjectMapper mapper, @NotNull String ur
     return req;
   }
 
+  @Nullable
+  @Override
+  public int[] statusCodes() {
+    return new int[]{
+        HttpStatus.SC_OK,
+        HttpStatus.SC_ACCEPTED,
+    };
+  }
+
   @Override
   public ObjectRes processResponse(@NotNull ObjectMapper mapper, @NotNull HttpMethod request) throws IOException {
     switch (request.getStatusCode()) {
@@ -47,7 +56,7 @@ public ObjectRes processResponse(@NotNull ObjectMapper mapper, @NotNull HttpMeth
       case HttpStatus.SC_ACCEPTED:
         return mapper.readValue(request.getResponseBodyAsStream(), ObjectRes.class);
       default:
-        throw new RequestException(request);
+        throw new IllegalStateException();
     }
   }
 }

gitlfs-client/src/main/java/ru/bozaro/gitlfs/client/internal/ObjectGet.java

@@ -4,6 +4,7 @@
 import org.apache.commons.httpclient.HttpMethod;
 import org.apache.commons.httpclient.methods.GetMethod;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -20,6 +21,12 @@ public HttpMethod createRequest(@NotNull ObjectMapper mapper, @NotNull String ur
     return new GetMethod(url);
   }
 
+  @Nullable
+  @Override
+  public int[] statusCodes() {
+    return null;
+  }
+
   @Override
   public InputStream processResponse(@NotNull ObjectMapper mapper, @NotNull HttpMethod request) throws IOException {
     return request.getResponseBodyAsStream();

gitlfs-client/src/main/java/ru/bozaro/gitlfs/client/internal/ObjectPut.java

@@ -5,6 +5,7 @@
 import org.apache.commons.httpclient.methods.PutMethod;
 import org.apache.commons.httpclient.methods.RequestEntity;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import ru.bozaro.gitlfs.client.io.StreamProvider;
 
 import java.io.IOException;
@@ -62,6 +63,12 @@ public String getContentType() {
     return req;
   }
 
+  @Nullable
+  @Override
+  public int[] statusCodes() {
+    return null;
+  }
+
   @Override
   public Void processResponse(@NotNull ObjectMapper mapper, @NotNull HttpMethod request) throws IOException {
     return null;

gitlfs-client/src/main/java/ru/bozaro/gitlfs/client/internal/ObjectVerify.java

@@ -2,11 +2,10 @@
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.commons.httpclient.HttpMethod;
-import org.apache.commons.httpclient.HttpStatus;
 import org.apache.commons.httpclient.methods.ByteArrayRequestEntity;
 import org.apache.commons.httpclient.methods.PostMethod;
 import org.jetbrains.annotations.NotNull;
-import ru.bozaro.gitlfs.client.exceptions.RequestException;
+import org.jetbrains.annotations.Nullable;
 import ru.bozaro.gitlfs.common.data.Meta;
 
 import java.io.IOException;
@@ -37,14 +36,14 @@ public HttpMethod createRequest(@NotNull ObjectMapper mapper, @NotNull String ur
     return req;
   }
 
+  @Nullable
+  @Override
+  public int[] statusCodes() {
+    return null;
+  }
+
   @Override
   public Void processResponse(@NotNull ObjectMapper mapper, @NotNull HttpMethod request) throws IOException {
-    switch (request.getStatusCode()) {
-      case HttpStatus.SC_OK:
-      case HttpStatus.SC_ACCEPTED:
-        return null;
-      default:
-        throw new RequestException(request);
-    }
+    return null;
   }
 }

gitlfs-client/src/main/java/ru/bozaro/gitlfs/client/internal/Request.java

@@ -3,6 +3,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.commons.httpclient.HttpMethod;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import java.io.IOException;
 
@@ -16,4 +17,12 @@ public interface Request<R> {
   HttpMethod createRequest(@NotNull ObjectMapper mapper, @NotNull String url) throws IOException;
 
   R processResponse(@NotNull ObjectMapper mapper, @NotNull HttpMethod request) throws IOException;
+
+  /**
+   * Success status codes.
+   *
+   * @return Success status codes.
+   */
+  @Nullable
+  int[] statusCodes();
 }

gitlfs-pointer/src/test/java/ru/bozaro/gitlfs/pointer/PointerTest.java

@@ -36,6 +36,18 @@ public static Object[][] parseValidProvider() {
                 .put("size", "12345")
                 .build()
         },
+        new Object[]{
+            "pointer-valid-03.dat",
+            ImmutableMap.builder()
+                .put("version", "https://git-lfs.github.com/spec/v1")
+                .put("object-name", " Foo")
+                .put("object.id", "F1")
+                .put("object0123456789", ":)")
+                .put("oid", "sha256:4d7a214614ab2935c943f9e0ff69d22eadbb8f32b1258daaa5e2ca24d17e2393")
+                .put("name", "Текст в UTF-8")
+                .put("size", "12345")
+                .build()
+        },
     };
   }
 
@@ -70,6 +82,7 @@ public static Object[][] parseInvalidProvider() {
         new Object[]{"pointer-invalid-07.dat", "Non utf-8"},
         new Object[]{"pointer-invalid-08.dat", "Size is not number"},
         new Object[]{"pointer-invalid-09.dat", "Duplicate line"},
+        new Object[]{"pointer-invalid-10.dat", "Duplicate version"},
     };
   }
 

gitlfs-pointer/src/test/resources/ru/bozaro/gitlfs/pointer/pointer-invalid-10.dat

@@ -0,0 +1,4 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:4d7a214614ab2935c943f9e0ff69d22eadbb8f32b1258daaa5e2ca24d17e2393
+size 12345
+version https://git-lfs.github.com/spec/v1