From 48484be608cc1fdf56ca9b2807706a604fbfdaab Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Mon, 17 Jul 2017 20:40:17 +0300 Subject: [PATCH 1/5] Ensure app exists before listing its routes --- api/server/routes_list.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/server/routes_list.go b/api/server/routes_list.go index 6501280a9..a854a1522 100644 --- a/api/server/routes_list.go +++ b/api/server/routes_list.go @@ -23,6 +23,9 @@ func (s *Server) handleRouteList(c *gin.Context) { name, ok := appName.(string) if exists && ok && name != "" { routes, err = s.Datastore.GetRoutesByApp(ctx, name, filter) + if len(routes) == 0 { + _, err = s.Datastore.GetApp(ctx, name) + } } else { routes, err = s.Datastore.GetRoutes(ctx, filter) } From 4f5197a1c288e53e6ce8744cf83e48dab81e5886 Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Mon, 17 Jul 2017 20:54:42 +0300 Subject: [PATCH 2/5] Fixing tests and addressing comments --- api/server/routes_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/server/routes_test.go b/api/server/routes_test.go index b772f628a..7fa7c81b8 100644 --- a/api/server/routes_test.go +++ b/api/server/routes_test.go @@ -115,6 +115,9 @@ func TestRoutePut(t *testing.T) { func TestRouteDelete(t *testing.T) { buf := setLogBuffer() + routes := models.Routes{{AppName: "a", Path: "/myroute",}} + apps := []*models.App{{Name: "a", Routes: routes, Config: nil}} + for i, test := range []struct { ds models.Datastore logDB models.FnLog @@ -124,10 +127,7 @@ func TestRouteDelete(t *testing.T) { expectedError error }{ {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/missing", "", http.StatusNotFound, models.ErrRoutesNotFound}, - {datastore.NewMockInit(nil, - []*models.Route{ - {Path: "/myroute", AppName: "a"}, - }, nil, nil, + {datastore.NewMockInit(apps, routes, nil, nil, ), logs.NewMock(), "/v1/apps/a/routes/myroute", "", http.StatusOK, nil}, } { rnr, cancel := testRunner(t) From b3ba42e3b9969f97ac942e04fe0fd19ef376b30e Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Mon, 17 Jul 2017 20:57:18 +0300 Subject: [PATCH 3/5] Fixing fmt --- api/server/routes_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/api/server/routes_test.go b/api/server/routes_test.go index 7fa7c81b8..229ceff2a 100644 --- a/api/server/routes_test.go +++ b/api/server/routes_test.go @@ -115,7 +115,7 @@ func TestRoutePut(t *testing.T) { func TestRouteDelete(t *testing.T) { buf := setLogBuffer() - routes := models.Routes{{AppName: "a", Path: "/myroute",}} + routes := models.Routes{{AppName: "a", Path: "/myroute"}} apps := []*models.App{{Name: "a", Routes: routes, Config: nil}} for i, test := range []struct { @@ -126,9 +126,8 @@ func TestRouteDelete(t *testing.T) { expectedCode int expectedError error }{ - {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/missing", "", http.StatusNotFound, models.ErrRoutesNotFound}, - {datastore.NewMockInit(apps, routes, nil, nil, - ), logs.NewMock(), "/v1/apps/a/routes/myroute", "", http.StatusOK, nil}, + {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/missing", "", http.StatusNotFound, models.ErrAppsNotFound}, + {datastore.NewMockInit(apps, routes, nil, nil), logs.NewMock(), "/v1/apps/a/routes/myroute", "", http.StatusOK, nil}, } { rnr, cancel := testRunner(t) srv := testServer(test.ds, &mqs.Mock{}, test.logDB, rnr) From bdffa7576245c841a588071b8c8009bcb5c7ae16 Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Mon, 17 Jul 2017 21:17:00 +0300 Subject: [PATCH 4/5] Fixing tests --- api/server/routes_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/server/routes_test.go b/api/server/routes_test.go index 229ceff2a..532298a8a 100644 --- a/api/server/routes_test.go +++ b/api/server/routes_test.go @@ -116,7 +116,7 @@ func TestRouteDelete(t *testing.T) { buf := setLogBuffer() routes := models.Routes{{AppName: "a", Path: "/myroute"}} - apps := []*models.App{{Name: "a", Routes: routes, Config: nil}} + apps := models.Apps{{Name: "a", Routes: routes, Config: nil}} for i, test := range []struct { ds models.Datastore @@ -126,7 +126,7 @@ func TestRouteDelete(t *testing.T) { expectedCode int expectedError error }{ - {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/missing", "", http.StatusNotFound, models.ErrAppsNotFound}, + {datastore.NewMock(), logs.NewMock(), "/v1/apps/a/routes/missing", "", http.StatusNotFound, models.ErrRoutesNotFound}, {datastore.NewMockInit(apps, routes, nil, nil), logs.NewMock(), "/v1/apps/a/routes/myroute", "", http.StatusOK, nil}, } { rnr, cancel := testRunner(t) @@ -169,7 +169,7 @@ func TestRouteList(t *testing.T) { expectedCode int expectedError error }{ - {"/v1/apps/a/routes", "", http.StatusOK, nil}, + {"/v1/apps/a/routes", "", http.StatusNotFound, models.ErrAppsNotFound}, } { _, rec := routerRequest(t, srv.Router, "GET", test.path, nil) From 5ed3e79b6336d9d14eb6b8fa48b491e5ba915f97 Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Tue, 25 Jul 2017 20:09:05 +0300 Subject: [PATCH 5/5] Adding comment with the reason for doing this --- api/server/routes_list.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api/server/routes_list.go b/api/server/routes_list.go index a854a1522..7f8a091f5 100644 --- a/api/server/routes_list.go +++ b/api/server/routes_list.go @@ -23,6 +23,7 @@ func (s *Server) handleRouteList(c *gin.Context) { name, ok := appName.(string) if exists && ok && name != "" { routes, err = s.Datastore.GetRoutesByApp(ctx, name, filter) + // if there are no routes for the app, check if the app exists to return 404 if it does not if len(routes) == 0 { _, err = s.Datastore.GetApp(ctx, name) }