Fix api responses (#173)

* dev: disable vcs from development enviroment -buildvcs Whether to stamp binaries with version control information. By default, version control information is stamped into a binary if the main package and the main module containing it are in the repository containing the current directory (if there is a repository). Use -buildvcs=false to omit version control information. Signed-off-by: Kairo Araujo <[email protected]> * docs: swagger methods and parameter for download Improve the swagger documentation including - Fix correct method for /api/v1/download as GET instead POST - Include the parameter which allows users to try the endpoint directly from swagger documentation Signed-off-by: Kairo Araujo <[email protected]> * fix: Archivista Download API responses Archivista Download API returned 500 if a GitOID is available while the user calls the `/v1/download/<gitoid>` (`/download/<gitoid>`) endpoint. It is not an Internal Server Error (500) but a not-found for the GitOID endpoint. This PR fixes the HTTP status code, giving the users a more precise response. This commit also includes a better list of return codes to the Swagger documentation. It includes the UT Signed-off-by: Kairo Araujo <[email protected]> --------- Signed-off-by: Kairo Araujo <[email protected]>
in-toto · Jan 30, 2024 · d160aaf · d160aaf
1 parent e7f13b2
commit d160aaf
Show file tree

Hide file tree

Showing 6 changed files with 238 additions and 66 deletions.
diff --git a/docs/docs.go b/docs/docs.go
@@ -21,20 +21,44 @@ const docTemplate = `{
     "host": "{{.Host}}",
     "basePath": "{{.BasePath}}",
     "paths": {
-        "/donwload/{gitoid}": {
-            "post": {
+        "/download/{gitoid}": {
+            "get": {
                 "description": "download an attestation",
                 "produces": [
                     "application/json"
                 ],
                 "summary": "Download",
                 "deprecated": true,
+                "parameters": [
+                    {
+                        "type": "string",
+                        "description": "gitoid",
+                        "name": "gitoid",
+                        "in": "path",
+                        "required": true
+                    }
+                ],
                 "responses": {
                     "200": {
                         "description": "OK",
                         "schema": {
                             "$ref": "#/definitions/dsse.Envelope"
                         }
+                    },
+                    "400": {
+                        "description": "Bad Request",
+                        "schema": {
+                            "type": "string"
+                        }
+                    },
+                    "404": {
+                        "description": "Not Found"
+                    },
+                    "500": {
+                        "description": "Internal Server Error",
+                        "schema": {
+                            "type": "string"
+                        }
                     }
                 }
             }
@@ -45,7 +69,7 @@ const docTemplate = `{
                 "produces": [
                     "application/json"
                 ],
-                "summary": "Store",
+                "summary": "Upload",
                 "deprecated": true,
                 "responses": {
                     "200": {
@@ -57,8 +81,8 @@ const docTemplate = `{
                 }
             }
         },
-        "/v1/donwload/{gitoid}": {
-            "post": {
+        "/v1/download/{gitoid}": {
+            "get": {
                 "description": "download an attestation",
                 "produces": [
                     "application/json"
@@ -67,12 +91,36 @@ const docTemplate = `{
                     "attestation"
                 ],
                 "summary": "Download",
+                "parameters": [
+                    {
+                        "type": "string",
+                        "description": "gitoid",
+                        "name": "gitoid",
+                        "in": "path",
+                        "required": true
+                    }
+                ],
                 "responses": {
                     "200": {
                         "description": "OK",
                         "schema": {
                             "$ref": "#/definitions/dsse.Envelope"
                         }
+                    },
+                    "400": {
+                        "description": "Bad Request",
+                        "schema": {
+                            "type": "string"
+                        }
+                    },
+                    "404": {
+                        "description": "Not Found"
+                    },
+                    "500": {
+                        "description": "Internal Server Error",
+                        "schema": {
+                            "type": "string"
+                        }
                     }
                 }
             }
@@ -106,7 +154,7 @@ const docTemplate = `{
                 "tags": [
                     "attestation"
                 ],
-                "summary": "Store",
+                "summary": "Upload",
                 "responses": {
                     "200": {
                         "description": "OK",

diff --git a/docs/swagger.json b/docs/swagger.json
@@ -13,20 +13,44 @@
         "version": "v1"
     },
     "paths": {
-        "/donwload/{gitoid}": {
-            "post": {
+        "/download/{gitoid}": {
+            "get": {
                 "description": "download an attestation",
                 "produces": [
                     "application/json"
                 ],
                 "summary": "Download",
                 "deprecated": true,
+                "parameters": [
+                    {
+                        "type": "string",
+                        "description": "gitoid",
+                        "name": "gitoid",
+                        "in": "path",
+                        "required": true
+                    }
+                ],
                 "responses": {
                     "200": {
                         "description": "OK",
                         "schema": {
                             "$ref": "#/definitions/dsse.Envelope"
                         }
+                    },
+                    "400": {
+                        "description": "Bad Request",
+                        "schema": {
+                            "type": "string"
+                        }
+                    },
+                    "404": {
+                        "description": "Not Found"
+                    },
+                    "500": {
+                        "description": "Internal Server Error",
+                        "schema": {
+                            "type": "string"
+                        }
                     }
                 }
             }
@@ -37,7 +61,7 @@
                 "produces": [
                     "application/json"
                 ],
-                "summary": "Store",
+                "summary": "Upload",
                 "deprecated": true,
                 "responses": {
                     "200": {
@@ -49,8 +73,8 @@
                 }
             }
         },
-        "/v1/donwload/{gitoid}": {
-            "post": {
+        "/v1/download/{gitoid}": {
+            "get": {
                 "description": "download an attestation",
                 "produces": [
                     "application/json"
@@ -59,12 +83,36 @@
                     "attestation"
                 ],
                 "summary": "Download",
+                "parameters": [
+                    {
+                        "type": "string",
+                        "description": "gitoid",
+                        "name": "gitoid",
+                        "in": "path",
+                        "required": true
+                    }
+                ],
                 "responses": {
                     "200": {
                         "description": "OK",
                         "schema": {
                             "$ref": "#/definitions/dsse.Envelope"
                         }
+                    },
+                    "400": {
+                        "description": "Bad Request",
+                        "schema": {
+                            "type": "string"
+                        }
+                    },
+                    "404": {
+                        "description": "Not Found"
+                    },
+                    "500": {
+                        "description": "Internal Server Error",
+                        "schema": {
+                            "type": "string"
+                        }
                     }
                 }
             }
@@ -98,7 +146,7 @@
                 "tags": [
                     "attestation"
                 ],
-                "summary": "Store",
+                "summary": "Upload",
                 "responses": {
                     "200": {
                         "description": "OK",
@@ -201,4 +249,4 @@
             ]
         }
     }
-}
+}
diff --git a/docs/swagger.yaml b/docs/swagger.yaml
@@ -67,17 +67,33 @@ info:
   title: Archivista API
   version: v1
 paths:
-  /donwload/{gitoid}:
-    post:
+  /download/{gitoid}:
+    get:
       deprecated: true
       description: download an attestation
+      parameters:
+      - description: gitoid
+        in: path
+        name: gitoid
+        required: true
+        type: string
       produces:
       - application/json
       responses:
         "200":
           description: OK
           schema:
             $ref: '#/definitions/dsse.Envelope'
+        "400":
+          description: Bad Request
+          schema:
+            type: string
+        "404":
+          description: Not Found
+        "500":
+          description: Internal Server Error
+          schema:
+            type: string
       summary: Download
   /upload:
     post:
@@ -90,17 +106,33 @@ paths:
           description: OK
           schema:
             $ref: '#/definitions/api.StoreResponse'
-      summary: Store
-  /v1/donwload/{gitoid}:
-    post:
+      summary: Upload
+  /v1/download/{gitoid}:
+    get:
       description: download an attestation
+      parameters:
+      - description: gitoid
+        in: path
+        name: gitoid
+        required: true
+        type: string
       produces:
       - application/json
       responses:
         "200":
           description: OK
           schema:
             $ref: '#/definitions/dsse.Envelope'
+        "400":
+          description: Bad Request
+          schema:
+            type: string
+        "404":
+          description: Not Found
+        "500":
+          description: Internal Server Error
+          schema:
+            type: string
       summary: Download
       tags:
       - attestation
@@ -127,7 +159,7 @@ paths:
           description: OK
           schema:
             $ref: '#/definitions/api.StoreResponse'
-      summary: Store
+      summary: Upload
       tags:
       - attestation
 swagger: "2.0"
diff --git a/entrypoint-dev.sh b/entrypoint-dev.sh
@@ -44,4 +44,4 @@ if [[ $atlas_rc -ne 0 ]]; then
     exit 1
 fi
 
-CompileDaemon -log-prefix=false -build="go build -o /out/archivista ./cmd/archivista" -command="/out/archivista"
+CompileDaemon -log-prefix=false -build="go build -buildvcs=false -o /out/archivista ./cmd/archivista" -command="/out/archivista"
diff --git a/internal/server/server.go b/internal/server/server.go
@@ -160,14 +160,14 @@ func (s *Server) UploadHandler(w http.ResponseWriter, r *http.Request) {
 // @Summary Download
 // @Description download an attestation
 // @Produce  json
+// @Param gitoid path string true "gitoid"
 // @Success 200 {object} dsse.Envelope
+// @Failure 500 {object} string
+// @Failure 404 {object} nil
+// @Failure 400 {object} string
 // @Tags attestation
-// @Router /v1/download/{gitoid} [post]
+// @Router /v1/download/{gitoid} [get]
 func (s *Server) Download(ctx context.Context, gitoid string) (io.ReadCloser, error) {
-	if len(strings.TrimSpace(gitoid)) == 0 {
-		return nil, errors.New("gitoid parameter is required")
-	}
-
 	if s.objectStore == nil {
 		return nil, errors.New("object store unavailable")
 	}
@@ -183,9 +183,13 @@ func (s *Server) Download(ctx context.Context, gitoid string) (io.ReadCloser, er
 // @Summary Download
 // @Description download an attestation
 // @Produce  json
+// @Param gitoid path string true "gitoid"
 // @Success 200 {object} dsse.Envelope
+// @Failure 500 {object} string
+// @Failure 404 {object} nil
+// @Failure 400 {object} string
 // @Deprecated
-// @Router /download/{gitoid} [post]
+// @Router /download/{gitoid} [get]
 func (s *Server) DownloadHandler(w http.ResponseWriter, r *http.Request) {
 	if r.Method != http.MethodGet {
 		http.Error(w, fmt.Sprintf("%s is an unsupported method", r.Method), http.StatusBadRequest)
@@ -197,6 +201,10 @@ func (s *Server) DownloadHandler(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "gitoid parameter is required", http.StatusBadRequest)
 		return
 	}
+	if len(strings.TrimSpace(vars["gitoid"])) == 0 {
+		http.Error(w, "gitoid parameter is required", http.StatusBadRequest)
+		return
+	}
 
 	attestationReader, err := s.Download(r.Context(), vars["gitoid"])
 	if err != nil {
@@ -207,7 +215,7 @@ func (s *Server) DownloadHandler(w http.ResponseWriter, r *http.Request) {
 	defer attestationReader.Close()
 	if _, err := io.Copy(w, attestationReader); err != nil {
 		logrus.Errorf("failed to copy attestation to response: %+v", err)
-		w.WriteHeader(http.StatusInternalServerError)
+		w.WriteHeader(http.StatusNotFound)
 		return
 	}
-Original file line number
+Diff line change
@@ Expand Up / @@ -44,4 +44,4 @@ if [[ $atlas_rc -ne 0 ]]; then @@
         exit 1
     fi
-    CompileDaemon -log-prefix=false -build="go build -o /out/archivista ./cmd/archivista" -command="/out/archivista"
+    CompileDaemon -log-prefix=false -build="go build -buildvcs=false -o /out/archivista ./cmd/archivista" -command="/out/archivista"