Merge pull request #59 from fossa-app/add-status-code-helpers

Add status code helpers and make ProblemDetails status non-nullable

Author: tiksn

src/Bridge/Bridge.fsproj

@@ -13,6 +13,7 @@
     <Compile Include="Models\ApiModels\PayloadModels.fs" />
     <Compile Include="Services\Endpoints.fs" />
     <Compile Include="Services\UrlHelpers.fs" />
+    <Compile Include="Services\StatusCodeHelpers.fs" />
     <Compile Include="Services\IHttpRequestSender.fs" />
     <Compile Include="Services\IJsonSerializer.fs" />
     <Compile Include="Services\JsonSerializer.fs" />

src/Bridge/Models/ApiModels/SharedModels.fs

@@ -6,7 +6,7 @@ open System
 type ProblemDetailsModel =
     { Type: string | null
       Title: string | null
-      Status: Nullable<int>
+      Status: int
       Detail: string | null
       Instance: string | null }
 

src/Bridge/Services/StatusCodeHelpers.fs

@@ -0,0 +1,11 @@
+module Fossa.Bridge.Services.StatusCodeHelpers
+
+open Fossa.Bridge.Models.ApiModels
+
+let isStatusCodeClientError (statusCode: int) : bool = statusCode >= 400 && statusCode <= 499
+
+let isStatusCodeServerError (statusCode: int) : bool = statusCode >= 500 && statusCode <= 599
+
+let isClientProblem (problem: ProblemDetailsModel) : bool = isStatusCodeClientError problem.Status
+
+let isServerProblem (problem: ProblemDetailsModel) : bool = isStatusCodeServerError problem.Status

tests/Bridge.Tests/Bridge.Tests.fsproj

@@ -7,6 +7,7 @@
   <ItemGroup>
     <Compile Include="JsonSerializerTests.fs" />
     <Compile Include="UrlHelpersTests.fs" />
+    <Compile Include="StatusCodeHelpersTests.fs" />
     <Compile Include="Main.fs" />
   </ItemGroup>
   <ItemGroup>

tests/Bridge.Tests/JsonSerializerTests.fs

@@ -83,7 +83,7 @@ let tests =
               let model =
                   { Type = "https://example.com/problems/conflict"
                     Title = "Conflict"
-                    Status = Nullable 409
+                    Status = 409
                     Detail = "The requested change conflicts with current state."
                     Instance = "/companies/42" }
 
@@ -103,7 +103,7 @@ let tests =
               let model =
                   { Type = null
                     Title = null
-                    Status = Nullable 404
+                    Status = 404
                     Detail = null
                     Instance = null }
 
@@ -125,6 +125,17 @@ let tests =
 
               Expect.equal model.Type "https://example.com/problems/not-found" "Type should deserialize"
               Expect.equal model.Title "Not Found" "Title should deserialize"
-              Expect.equal model.Status (Nullable 404) "Status should deserialize"
+              Expect.equal model.Status 404 "Status should deserialize"
               Expect.equal model.Detail "Missing." "Detail should deserialize"
-              Expect.equal model.Instance "/companies/99" "Instance should deserialize" ]
+              Expect.equal model.Instance "/companies/99" "Instance should deserialize"
+
+          testCase "ProblemDetailsModel deserializes missing status as default int"
+          <| fun _ ->
+              let serializer = JsonSerializer() :> IJsonSerializer
+
+              let json =
+                  "{\"type\":\"https://example.com/problems/not-found\",\"title\":\"Not Found\",\"detail\":\"Missing.\",\"instance\":\"/companies/99\"}"
+
+              let model = serializer.Deserialize<ProblemDetailsModel>(json)
+
+              Expect.equal model.Status 0 "Missing status should deserialize to default int value" ]

tests/Bridge.Tests/StatusCodeHelpersTests.fs

@@ -0,0 +1,67 @@
+module StatusCodeHelpersTests
+
+open Expecto
+open Fossa.Bridge.Models.ApiModels
+open Fossa.Bridge.Services.StatusCodeHelpers
+
+let private problemWithStatus status =
+    { Type = null
+      Title = null
+      Status = status
+      Detail = null
+      Instance = null }
+
+[<Tests>]
+let tests =
+    testList
+        "StatusCodeHelpersTests"
+        [ testList
+              "isStatusCodeClientError"
+              [ testCase "returns false below 400"
+                <| fun _ -> Expect.isFalse (isStatusCodeClientError 399) "399 should not be a client error"
+
+                testCase "returns true at 400"
+                <| fun _ -> Expect.isTrue (isStatusCodeClientError 400) "400 should be a client error"
+
+                testCase "returns true at 499"
+                <| fun _ -> Expect.isTrue (isStatusCodeClientError 499) "499 should be a client error"
+
+                testCase "returns false at 500"
+                <| fun _ -> Expect.isFalse (isStatusCodeClientError 500) "500 should not be a client error" ]
+
+          testList
+              "isStatusCodeServerError"
+              [ testCase "returns false at 499"
+                <| fun _ -> Expect.isFalse (isStatusCodeServerError 499) "499 should not be a server error"
+
+                testCase "returns true at 500"
+                <| fun _ -> Expect.isTrue (isStatusCodeServerError 500) "500 should be a server error"
+
+                testCase "returns true at 599"
+                <| fun _ -> Expect.isTrue (isStatusCodeServerError 599) "599 should be a server error"
+
+                testCase "returns false at 600"
+                <| fun _ -> Expect.isFalse (isStatusCodeServerError 600) "600 should not be a server error" ]
+
+          testList
+              "ProblemDetailsModel helpers"
+              [ testCase "client problem returns client true and server false"
+                <| fun _ ->
+                    let problem = problemWithStatus 404
+
+                    Expect.isTrue (isClientProblem problem) "4xx problem should be a client problem"
+                    Expect.isFalse (isServerProblem problem) "4xx problem should not be a server problem"
+
+                testCase "server problem returns server true and client false"
+                <| fun _ ->
+                    let problem = problemWithStatus 500
+
+                    Expect.isTrue (isServerProblem problem) "5xx problem should be a server problem"
+                    Expect.isFalse (isClientProblem problem) "5xx problem should not be a client problem"
+
+                testCase "non-error problem returns false for both helpers"
+                <| fun _ ->
+                    let problem = problemWithStatus 200
+
+                    Expect.isFalse (isClientProblem problem) "Non-error problem should not be a client problem"
+                    Expect.isFalse (isServerProblem problem) "Non-error problem should not be a server problem" ] ]