aboutsummaryrefslogtreecommitdiff
path: root/pkg/navitia_api_client
diff options
context:
space:
mode:
authorJulien Dessaux2021-05-04 16:19:42 +0200
committerJulien Dessaux2021-05-04 16:47:07 +0200
commitd2658b5c0c7bd8c6214a8e34582cdba23da8ce0d (patch)
tree5f55025c86f7ebbb0932bc94e2e1415cbfe8f040 /pkg/navitia_api_client
parentReworked the navitia_api_client to be mockable (diff)
downloadtrains-d2658b5c0c7bd8c6214a8e34582cdba23da8ce0d.tar.gz
trains-d2658b5c0c7bd8c6214a8e34582cdba23da8ce0d.tar.bz2
trains-d2658b5c0c7bd8c6214a8e34582cdba23da8ce0d.zip
Improved navitia api error handling and testing
Diffstat (limited to '')
-rw-r--r--pkg/navitia_api_client/departures.go35
-rw-r--r--pkg/navitia_api_client/departures_test.go75
-rw-r--r--pkg/navitia_api_client/error.go70
-rw-r--r--pkg/navitia_api_client/error_test.go17
-rw-r--r--pkg/navitia_api_client/test_data/invalid_date.json209
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"
+ }
+ }
+ ]
+}