Improved navitia api error handling and testing

5 files changed, 372 insertions, 34 deletions

diff --git a/pkg/navitia_api_client/departures.go b/pkg/navitia_api_client/departures.go
index 7447cdc..50ebe17 100644
--- a/pkg/navitia_api_client/departures.go
+++ b/pkg/navitia_api_client/departures.go
@@ -57,27 +57,32 @@ func (c *NavitiaClient) GetDepartures(trainStop string) (departures []model.Depa
 	}
 	req, err := http.NewRequest("GET", request, nil)
 	if err != nil {
-		return nil, err
+		return nil, newHttpClientError("http.NewRequest error", err)
 	}
 	resp, err := c.httpClient.Do(req)
 	if err != nil {
-		return nil, err
+		return nil, newHttpClientError("httpClient.Do error", err)
 	}
 	defer resp.Body.Close()
-	var data DeparturesResponse
-	if err = json.NewDecoder(resp.Body).Decode(&data); err != nil {
-		return nil, err
-	}
-	for i := 0; i < len(data.Departures); i++ {
-		t, err := time.Parse("20060102T150405", data.Departures[i].StopDateTime.ArrivalDateTime)
-		if err != nil { // TODO fail better
-			panic(err)
+	if resp.StatusCode == http.StatusOK {
+		var data DeparturesResponse
+		if err = json.NewDecoder(resp.Body).Decode(&data); err != nil {
+			return nil, newJsonDecodeError("GetDepartures "+trainStop, err)
 		}
-		departures = append(departures, model.Departure{data.Departures[i].DisplayInformations.Direction, t.Format("Mon, 02 Jan 2006 15:04:05")})
-	}
-	c.cache[request] = cachedResult{
-		ts:     start,
-		result: departures,
+		// TODO test for no json error
+		for i := 0; i < len(data.Departures); i++ {
+			t, err := time.Parse("20060102T150405", data.Departures[i].StopDateTime.ArrivalDateTime)
+			if err != nil {
+				return nil, newDateParsingError(data.Departures[i].StopDateTime.ArrivalDateTime, err)
+			}
+			departures = append(departures, model.Departure{data.Departures[i].DisplayInformations.Direction, t.Format("Mon, 02 Jan 2006 15:04:05")})
+		}
+		c.cache[request] = cachedResult{
+			ts:     start,
+			result: departures,
+		}
+	} else {
+		err = newApiError(resp.StatusCode, "GetDepartures "+trainStop)
 	}
 	return
 }
diff --git a/pkg/navitia_api_client/departures_test.go b/pkg/navitia_api_client/departures_test.go
index 9b2f52c..3cf9eab 100644
--- a/pkg/navitia_api_client/departures_test.go
+++ b/pkg/navitia_api_client/departures_test.go
@@ -3,35 +3,72 @@ package navitia_api_client
 import (
 	"net/http"
 	"net/http/httptest"
+	"reflect"
 	"testing"
+
+	"git.adyxax.org/adyxax/trains/pkg/model"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestGetDepartures(t *testing.T) {
-	// invalid characters in token
-	client := NewClient("}")
-	_, err := client.GetDepartures("test")
-	if err == nil {
-		t.Fatalf("invalid characters in token should raise an error because the url is invalid")
+	// Simple Test cases
+	testCases := []struct {
+		name               string
+		inputNewCLient     string
+		inputGetDepartures string
+		expected           []model.Departure
+		expectedError      interface{}
+	}{
+		{"invalid characters in token should fail", "}", "test", nil, &HttpClientError{}},
+		{"unreachable server should fail", "https://", "test", nil, &HttpClientError{}},
 	}
-	// unreachable server
-	client = NewClient("https://")
-	_, err = client.GetDepartures("test")
-	if err == nil {
-		t.Fatalf("unreachable server should raise an error")
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			client := NewClient(tc.inputNewCLient)
+			valid, err := client.GetDepartures(tc.inputGetDepartures)
+			if tc.expectedError != nil {
+				require.Error(t, err)
+				assert.Equalf(t, reflect.TypeOf(err), reflect.TypeOf(tc.expectedError), "Invalid error type. Got %s but expected %s", reflect.TypeOf(err), reflect.TypeOf(tc.expectedError))
+				assert.Equal(t, tc.expected, valid)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, tc.expected, valid)
+			}
+		})
 	}
-	// invalid json
-	client, ts := newTestClientFromFilename(t, "test_data/invalid.json")
-	defer ts.Close()
-	_, err = client.GetDepartures("test")
-	if err == nil {
-		t.Fatalf("invalid json should raise an error")
+	// Test cases with a filename
+	testCasesFilename := []struct {
+		name               string
+		inputFilename      string
+		inputGetDepartures string
+		expected           []model.Departure
+		expectedError      interface{}
+	}{
+		{"invalid json should fail", "test_data/invalid.json", "test", nil, &JsonDecodeError{}},
+		{"invalid date should fail", "test_data/invalid_date.json", "test", nil, &DateParsingError{}},
+	}
+	for _, tc := range testCasesFilename {
+		t.Run(tc.name, func(t *testing.T) {
+			client, ts := newTestClientFromFilename(t, tc.inputFilename)
+			defer ts.Close()
+			valid, err := client.GetDepartures(tc.inputGetDepartures)
+			if tc.expectedError != nil {
+				require.Error(t, err)
+				assert.Equalf(t, reflect.TypeOf(err), reflect.TypeOf(tc.expectedError), "Invalid error type. Got %s but expected %s", reflect.TypeOf(err), reflect.TypeOf(tc.expectedError))
+				assert.Equal(t, tc.expected, valid)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, tc.expected, valid)
+			}
+		})
 	}
 	// http error
-	ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusNotFound)
 	}))
-	client = newTestClient(ts)
-	_, err = client.GetDepartures("test")
+	client := newTestClient(ts)
+	_, err := client.GetDepartures("test")
 	if err == nil {
 		t.Fatalf("404 should raise an error")
 	}
diff --git a/pkg/navitia_api_client/error.go b/pkg/navitia_api_client/error.go
new file mode 100644
index 0000000..31080de
--- /dev/null
+++ b/pkg/navitia_api_client/error.go
@@ -0,0 +1,70 @@
+package navitia_api_client
+
+import "fmt"
+
+// navitia api query error
+type ApiError struct {
+	code    int
+	request string
+}
+
+func (e *ApiError) Error() string {
+	return fmt.Sprintf("Navitia Api error return code %d - %s", e.code, e.request)
+}
+
+func newApiError(code int, request string) error {
+	return &ApiError{
+		code:    code,
+		request: request,
+	}
+}
+
+// http client error
+type HttpClientError struct {
+	msg string
+	err error
+}
+
+func (e *HttpClientError) Error() string { return fmt.Sprintf("Navitia HttpClient error %s", e.msg) }
+func (e *HttpClientError) Unwrap() error { return e.err }
+
+func newHttpClientError(msg string, err error) error {
+	return &HttpClientError{
+		msg: msg,
+		err: err,
+	}
+}
+
+// json decoding error
+type JsonDecodeError struct {
+	msg string
+	err error
+}
+
+func (e *JsonDecodeError) Error() string { return fmt.Sprintf("Navitia JsonDecode error %s", e.msg) }
+func (e *JsonDecodeError) Unwrap() error { return e.err }
+
+func newJsonDecodeError(msg string, err error) error {
+	return &JsonDecodeError{
+		msg: msg,
+		err: err,
+	}
+}
+
+// date parsing error
+type DateParsingError struct {
+	date string
+	err  error
+}
+
+func (e *DateParsingError) Error() string {
+	return fmt.Sprintf("Navitia date parsing error %s", e.date)
+}
+func (e *DateParsingError) Unwrap() error { return e.err }
+
+func newDateParsingError(date string, err error) error {
+	return &DateParsingError{
+		date: date,
+		err:  err,
+	}
+}
diff --git a/pkg/navitia_api_client/error_test.go b/pkg/navitia_api_client/error_test.go
new file mode 100644
index 0000000..5ecf8c2
--- /dev/null
+++ b/pkg/navitia_api_client/error_test.go
@@ -0,0 +1,17 @@
+package navitia_api_client
+
+import "testing"
+
+func TestErrorsCoverage(t *testing.T) {
+	apiErr := ApiError{}
+	_ = apiErr.Error()
+	httpClientErr := HttpClientError{}
+	_ = httpClientErr.Error()
+	_ = httpClientErr.Unwrap()
+	jsonDecodeErr := JsonDecodeError{}
+	_ = jsonDecodeErr.Error()
+	_ = jsonDecodeErr.Unwrap()
+	dateParsingErr := DateParsingError{}
+	_ = dateParsingErr.Error()
+	_ = dateParsingErr.Unwrap()
+}
diff --git a/pkg/navitia_api_client/test_data/invalid_date.json b/pkg/navitia_api_client/test_data/invalid_date.json
new file mode 100644
index 0000000..6b9e20d
--- /dev/null
+++ b/pkg/navitia_api_client/test_data/invalid_date.json
@@ -0,0 +1,209 @@
+{
+  "departures": [
+    {
+      "display_informations": {
+        "direction": "Ambérieu-en-Bugey (Ambérieu-en-Bugey)",
+        "code": "",
+        "network": "SNCF",
+        "links": [],
+        "color": "000000",
+        "name": "St-Etienne - Lyon - Ambérieu",
+        "physical_mode": "Train régional / TER",
+        "headsign": "886823",
+        "label": "St-Etienne - Lyon - Ambérieu",
+        "equipments": [],
+        "text_color": "FFFFFF",
+        "trip_short_name": "886823",
+        "commercial_mode": "TER",
+        "description": ""
+      },
+      "stop_point": {
+        "commercial_modes": [
+          {
+            "id": "commercial_mode:ter",
+            "name": "TER"
+          }
+        ],
+        "name": "Crépieux-la-Pape",
+        "links": [],
+        "physical_modes": [
+          {
+            "id": "physical_mode:LocalTrain",
+            "name": "Train régional / TER"
+          }
+        ],
+        "coord": {
+          "lat": "45.803921",
+          "lon": "4.892737"
+        },
+        "label": "Crépieux-la-Pape (Rillieux-la-Pape)",
+        "equipments": [],
+        "administrative_regions": [
+          {
+            "insee": "69286",
+            "name": "Rillieux-la-Pape",
+            "level": 8,
+            "coord": {
+              "lat": "45.823514",
+              "lon": "4.8994366"
+            },
+            "label": "Rillieux-la-Pape (69140)",
+            "id": "admin:fr:69286",
+            "zip_code": "69140"
+          }
+        ],
+        "fare_zone": {
+          "name": "0"
+        },
+        "id": "stop_point:OCE:SP:TrainTER-87723502",
+        "stop_area": {
+          "codes": [
+            {
+              "type": "CR-CI-CH",
+              "value": "0087-723502-00"
+            },
+            {
+              "type": "UIC8",
+              "value": "87723502"
+            },
+            {
+              "type": "external_code",
+              "value": "OCE87723502"
+            }
+          ],
+          "name": "Crépieux-la-Pape",
+          "links": [],
+          "coord": {
+            "lat": "45.803921",
+            "lon": "4.892737"
+          },
+          "label": "Crépieux-la-Pape (Rillieux-la-Pape)",
+          "administrative_regions": [
+            {
+              "insee": "69286",
+              "name": "Rillieux-la-Pape",
+              "level": 8,
+              "coord": {
+                "lat": "45.823514",
+                "lon": "4.8994366"
+              },
+              "label": "Rillieux-la-Pape (69140)",
+              "id": "admin:fr:69286",
+              "zip_code": "69140"
+            }
+          ],
+          "timezone": "Europe/Paris",
+          "id": "stop_area:OCE:SA:87723502"
+        }
+      },
+      "route": {
+        "direction": {
+          "embedded_type": "stop_area",
+          "stop_area": {
+            "codes": [
+              {
+                "type": "CR-CI-CH",
+                "value": "0087-743716-BV"
+              },
+              {
+                "type": "UIC8",
+                "value": "87743716"
+              },
+              {
+                "type": "external_code",
+                "value": "OCE87743716"
+              }
+            ],
+            "name": "Ambérieu-en-Bugey",
+            "links": [],
+            "coord": {
+              "lat": "45.954008",
+              "lon": "5.342313"
+            },
+            "label": "Ambérieu-en-Bugey (Ambérieu-en-Bugey)",
+            "timezone": "Europe/Paris",
+            "id": "stop_area:OCE:SA:87743716"
+          },
+          "quality": 0,
+          "name": "Ambérieu-en-Bugey (Ambérieu-en-Bugey)",
+          "id": "stop_area:OCE:SA:87743716"
+        },
+        "name": "St-Etienne-Châteaucreux vers Ambérieu-en-Bugey (Train TER)",
+        "links": [],
+        "physical_modes": [
+          {
+            "id": "physical_mode:LocalTrain",
+            "name": "Train régional / TER"
+          }
+        ],
+        "is_frequence": "False",
+        "geojson": {
+          "type": "MultiLineString",
+          "coordinates": []
+        },
+        "direction_type": "forward",
+        "line": {
+          "code": "",
+          "name": "St-Etienne - Lyon - Ambérieu",
+          "links": [],
+          "color": "000000",
+          "geojson": {
+            "type": "MultiLineString",
+            "coordinates": []
+          },
+          "text_color": "FFFFFF",
+          "physical_modes": [
+            {
+              "id": "physical_mode:LocalTrain",
+              "name": "Train régional / TER"
+            }
+          ],
+          "codes": [],
+          "closing_time": "221200",
+          "opening_time": "053500",
+          "commercial_mode": {
+            "id": "commercial_mode:ter",
+            "name": "TER"
+          },
+          "id": "line:OCE:199"
+        },
+        "id": "route:OCE:199-TrainTER-87726000-87743716"
+      },
+      "links": [
+        {
+          "type": "line",
+          "id": "line:OCE:199"
+        },
+        {
+          "type": "vehicle_journey",
+          "id": "vehicle_journey:OCE:SN886823F29029_dst_1"
+        },
+        {
+          "type": "route",
+          "id": "route:OCE:199-TrainTER-87726000-87743716"
+        },
+        {
+          "type": "commercial_mode",
+          "id": "commercial_mode:ter"
+        },
+        {
+          "type": "physical_mode",
+          "id": "physical_mode:LocalTrain"
+        },
+        {
+          "type": "network",
+          "id": "network:sncf"
+        }
+      ],
+      "stop_date_time": {
+        "links": [],
+        "arrival_date_time": "XXX",
+        "additional_informations": [],
+        "departure_date_time": "20210218T131800",
+        "base_arrival_date_time": "20210218T131800",
+        "base_departure_date_time": "20210218T131800",
+        "data_freshness": "base_schedule"
+      }
+    }
+  ]
+}