From cdba451c8e5a3d846a6ed2bc4d1d19009d32d676 Mon Sep 17 00:00:00 2001 From: Thomas Osterbind Date: Mon, 6 Nov 2017 17:12:44 -0500 Subject: [PATCH 1/5] fix: updated pagination to rely on core struct - first pass complete - Limit and Offset ListParams obtained by api/handlers' List functions/methods using `ListParamsFromRequest()` - `params_test.go` added, handles a few cases of invalid limits and offsets - partially complete - added support for accepting an 'orderBy' request query param just now but have not yet updated core/params_test and api/handlers functions to use this value yet --- api/handlers/datasets.go | 21 ++--- api/handlers/peers.go | 26 +++--- api/handlers/queries.go | 9 ++- core/c.out | 171 --------------------------------------- core/coverage.out | 171 --------------------------------------- core/datasets_test.go | 6 +- core/params.go | 41 ++++++++++ core/params_test.go | 71 ++++++++++++++++ 8 files changed, 148 insertions(+), 368 deletions(-) delete mode 100644 core/c.out delete mode 100644 core/coverage.out create mode 100644 core/params_test.go diff --git a/api/handlers/datasets.go b/api/handlers/datasets.go index 2fc308c53..8a4d11268 100644 --- a/api/handlers/datasets.go +++ b/api/handlers/datasets.go @@ -102,20 +102,22 @@ func (h *DatasetHandlers) ZipDatasetHandler(w http.ResponseWriter, r *http.Reque } func (h *DatasetHandlers) listDatasetsHandler(w http.ResponseWriter, r *http.Request) { - p := util.PageFromRequest(r) + lp := core.ListParamsFromRequest(r) res := []*repo.DatasetRef{} args := &core.ListParams{ - Limit: p.Limit(), - Offset: p.Offset(), - OrderBy: "created", + Limit: lp.Limit, + Offset: lp.Offset, + OrderBy: "created", //TODO: should there be a global default orderby? } if err := h.List(args, &res); err != nil { h.log.Infof("error listing datasets: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return } - - if err := util.WritePageResponse(w, res, r, p); err != nil { + // TODO: need to update util.WritePageResponse to take a + // core.ListParams rather than a util.Page struct + // for time being I added an empty util.Page struct + if err := util.WritePageResponse(w, res, r, util.Page{}); err != nil { h.log.Infof("error list datasests response: %s", err.Error()) } } @@ -225,7 +227,8 @@ func (h *DatasetHandlers) deleteDatasetHandler(w http.ResponseWriter, r *http.Re } func (h *DatasetHandlers) getStructuredDataHandler(w http.ResponseWriter, r *http.Request) { - page := util.PageFromRequest(r) + //page := util.PageFromRequest(r) + listParams := core.ListParamsFromRequest(r) all, err := util.ReqParamBool("all", r) if err != nil { @@ -241,8 +244,8 @@ func (h *DatasetHandlers) getStructuredDataHandler(w http.ResponseWriter, r *htt Format: dataset.JsonDataFormat, Path: datastore.NewKey(r.URL.Path[len("/data"):]), Objects: objectRows, - Limit: page.Limit(), - Offset: page.Offset(), + Limit: listParams.Limit, + Offset: listParams.Offset, All: all, } data := &core.StructuredData{} diff --git a/api/handlers/peers.go b/api/handlers/peers.go index 02b938557..99165d942 100644 --- a/api/handlers/peers.go +++ b/api/handlers/peers.go @@ -81,25 +81,30 @@ func (d *PeerHandlers) ConnectionsHandler(w http.ResponseWriter, r *http.Request } func (h *PeerHandlers) listPeersHandler(w http.ResponseWriter, r *http.Request) { - p := util.PageFromRequest(r) + //p := util.PageFromRequest(r) + lp := core.ListParamsFromRequest(r) res := []*profile.Profile{} args := &core.ListParams{ - Limit: p.Limit(), - Offset: p.Offset(), - OrderBy: "created", + Limit: lp.Limit, + Offset: lp.Offset, + OrderBy: "created", //TODO: should there be a global default for OrderBy? } if err := h.List(args, &res); err != nil { h.log.Infof("list peers: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return } - util.WritePageResponse(w, res, r, p) + // TODO: need to update util.WritePageResponse to take a + // core.ListParams rather than a util.Page struct + // for time being I added an empty util.Page struct + util.WritePageResponse(w, res, r, util.Page{}) } func (h *PeerHandlers) listConnectionsHandler(w http.ResponseWriter, r *http.Request) { - limit := 0 + //limit := 0 + listParams := core.ListParamsFromRequest(r) peers := []string{} - if err := h.ConnectedPeers(&limit, &peers); err != nil { + if err := h.ConnectedPeers(&listParams.Limit, &peers); err != nil { h.log.Infof("error showing connected peers: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return @@ -142,11 +147,12 @@ func (h *PeerHandlers) getPeerHandler(w http.ResponseWriter, r *http.Request) { } func (h *PeerHandlers) peerNamespaceHandler(w http.ResponseWriter, r *http.Request) { - page := util.PageFromRequest(r) + //page := util.PageFromRequest(r) + listParams := core.ListParamsFromRequest(r) args := &core.NamespaceParams{ PeerId: r.URL.Path[len("/peernamespace/"):], - Limit: page.Limit(), - Offset: page.Offset(), + Limit: listParams.Limit, + Offset: listParams.Offset, } res := []*repo.DatasetRef{} if err := h.GetNamespace(args, &res); err != nil { diff --git a/api/handlers/queries.go b/api/handlers/queries.go index 8ec004ae9..79c9689f6 100644 --- a/api/handlers/queries.go +++ b/api/handlers/queries.go @@ -60,12 +60,13 @@ func (d *QueryHandlers) ListHandler(w http.ResponseWriter, r *http.Request) { // } func (h *QueryHandlers) listQueriesHandler(w http.ResponseWriter, r *http.Request) { - p := util.PageFromRequest(r) + //p := util.PageFromRequest(r) + lp := core.ListParamsFromRequest(r) res := []*repo.DatasetRef{} args := &core.ListParams{ - Limit: p.Limit(), - Offset: p.Offset(), - OrderBy: "created", + Limit: lp.Limit, + Offset: lp.Offset, + OrderBy: "created", //TODO: should there be a global default for OrderBy? } err := h.List(args, &res) if err != nil { diff --git a/core/c.out b/core/c.out deleted file mode 100644 index 31fd0d5c6..000000000 --- a/core/c.out +++ /dev/null @@ -1,171 +0,0 @@ -mode: set -github.com/qri-io/qri/core/dataset_update.go:25.78,29.16 3 0 -github.com/qri-io/qri/core/dataset_update.go:34.2,37.24 2 0 -github.com/qri-io/qri/core/dataset_update.go:62.2,70.16 4 0 -github.com/qri-io/qri/core/dataset_update.go:74.2,79.12 2 0 -github.com/qri-io/qri/core/dataset_update.go:29.16,31.3 1 0 -github.com/qri-io/qri/core/dataset_update.go:37.24,39.17 2 0 -github.com/qri-io/qri/core/dataset_update.go:42.3,43.17 2 0 -github.com/qri-io/qri/core/dataset_update.go:47.3,48.24 2 0 -github.com/qri-io/qri/core/dataset_update.go:39.17,41.4 1 0 -github.com/qri-io/qri/core/dataset_update.go:43.17,45.4 1 0 -github.com/qri-io/qri/core/dataset_update.go:70.16,72.3 1 0 -github.com/qri-io/qri/core/datasets.go:29.77,34.2 1 1 -github.com/qri-io/qri/core/datasets.go:36.78,49.18 4 1 -github.com/qri-io/qri/core/datasets.go:53.2,53.18 1 1 -github.com/qri-io/qri/core/datasets.go:56.2,57.16 2 1 -github.com/qri-io/qri/core/datasets.go:61.2,61.30 1 1 -github.com/qri-io/qri/core/datasets.go:77.2,78.12 2 1 -github.com/qri-io/qri/core/datasets.go:49.18,51.3 1 0 -github.com/qri-io/qri/core/datasets.go:53.18,55.3 1 1 -github.com/qri-io/qri/core/datasets.go:57.16,59.3 1 0 -github.com/qri-io/qri/core/datasets.go:61.30,62.19 1 1 -github.com/qri-io/qri/core/datasets.go:66.3,67.17 2 1 -github.com/qri-io/qri/core/datasets.go:75.3,75.26 1 1 -github.com/qri-io/qri/core/datasets.go:62.19,63.9 1 0 -github.com/qri-io/qri/core/datasets.go:67.17,71.18 2 0 -github.com/qri-io/qri/core/datasets.go:71.18,73.5 1 0 -github.com/qri-io/qri/core/datasets.go:87.80,89.16 2 1 -github.com/qri-io/qri/core/datasets.go:93.2,94.12 2 1 -github.com/qri-io/qri/core/datasets.go:89.16,91.3 1 1 -github.com/qri-io/qri/core/datasets.go:103.89,106.19 1 1 -github.com/qri-io/qri/core/datasets.go:110.2,111.16 2 1 -github.com/qri-io/qri/core/datasets.go:115.2,116.16 2 1 -github.com/qri-io/qri/core/datasets.go:120.2,120.90 1 1 -github.com/qri-io/qri/core/datasets.go:124.2,125.16 2 1 -github.com/qri-io/qri/core/datasets.go:129.2,130.18 2 1 -github.com/qri-io/qri/core/datasets.go:134.2,135.23 2 1 -github.com/qri-io/qri/core/datasets.go:141.2,142.20 2 1 -github.com/qri-io/qri/core/datasets.go:145.2,146.25 2 1 -github.com/qri-io/qri/core/datasets.go:149.2,151.45 2 1 -github.com/qri-io/qri/core/datasets.go:155.2,156.16 2 1 -github.com/qri-io/qri/core/datasets.go:160.2,160.52 1 1 -github.com/qri-io/qri/core/datasets.go:164.2,164.50 1 1 -github.com/qri-io/qri/core/datasets.go:168.2,169.16 2 1 -github.com/qri-io/qri/core/datasets.go:173.2,174.12 2 1 -github.com/qri-io/qri/core/datasets.go:106.19,108.3 1 1 -github.com/qri-io/qri/core/datasets.go:111.16,113.3 1 0 -github.com/qri-io/qri/core/datasets.go:116.16,118.3 1 1 -github.com/qri-io/qri/core/datasets.go:120.90,122.3 1 0 -github.com/qri-io/qri/core/datasets.go:125.16,127.3 1 0 -github.com/qri-io/qri/core/datasets.go:130.18,132.3 1 0 -github.com/qri-io/qri/core/datasets.go:135.23,136.64 1 0 -github.com/qri-io/qri/core/datasets.go:136.64,138.4 1 0 -github.com/qri-io/qri/core/datasets.go:142.20,144.3 1 1 -github.com/qri-io/qri/core/datasets.go:146.25,148.3 1 1 -github.com/qri-io/qri/core/datasets.go:151.45,153.3 1 0 -github.com/qri-io/qri/core/datasets.go:156.16,158.3 1 0 -github.com/qri-io/qri/core/datasets.go:160.52,162.3 1 0 -github.com/qri-io/qri/core/datasets.go:164.50,166.3 1 0 -github.com/qri-io/qri/core/datasets.go:169.16,171.3 1 0 -github.com/qri-io/qri/core/datasets.go:183.75,187.16 3 1 -github.com/qri-io/qri/core/datasets.go:191.2,191.53 1 1 -github.com/qri-io/qri/core/datasets.go:194.2,194.52 1 1 -github.com/qri-io/qri/core/datasets.go:198.2,199.12 2 1 -github.com/qri-io/qri/core/datasets.go:187.16,189.3 1 1 -github.com/qri-io/qri/core/datasets.go:191.53,193.3 1 0 -github.com/qri-io/qri/core/datasets.go:194.52,196.3 1 0 -github.com/qri-io/qri/core/datasets.go:207.67,242.2 1 1 -github.com/qri-io/qri/core/datasets.go:257.101,263.16 3 1 -github.com/qri-io/qri/core/datasets.go:267.2,267.11 1 1 -github.com/qri-io/qri/core/datasets.go:274.2,274.16 1 1 -github.com/qri-io/qri/core/datasets.go:278.2,288.71 5 1 -github.com/qri-io/qri/core/datasets.go:297.2,297.36 1 1 -github.com/qri-io/qri/core/datasets.go:301.2,305.12 2 1 -github.com/qri-io/qri/core/datasets.go:263.16,265.3 1 1 -github.com/qri-io/qri/core/datasets.go:267.11,269.3 1 1 -github.com/qri-io/qri/core/datasets.go:269.3,272.3 2 1 -github.com/qri-io/qri/core/datasets.go:274.16,276.3 1 0 -github.com/qri-io/qri/core/datasets.go:288.71,289.17 1 1 -github.com/qri-io/qri/core/datasets.go:292.3,292.27 1 1 -github.com/qri-io/qri/core/datasets.go:289.17,291.4 1 0 -github.com/qri-io/qri/core/datasets.go:293.17,295.3 1 0 -github.com/qri-io/qri/core/datasets.go:297.36,299.3 1 0 -github.com/qri-io/qri/core/datasets.go:313.86,315.9 2 1 -github.com/qri-io/qri/core/datasets.go:319.2,322.16 4 0 -github.com/qri-io/qri/core/datasets.go:326.2,327.16 2 0 -github.com/qri-io/qri/core/datasets.go:331.2,333.16 3 0 -github.com/qri-io/qri/core/datasets.go:337.2,338.16 2 0 -github.com/qri-io/qri/core/datasets.go:342.2,347.8 2 0 -github.com/qri-io/qri/core/datasets.go:315.9,317.3 1 1 -github.com/qri-io/qri/core/datasets.go:322.16,324.3 1 0 -github.com/qri-io/qri/core/datasets.go:327.16,329.3 1 0 -github.com/qri-io/qri/core/datasets.go:333.16,335.3 1 0 -github.com/qri-io/qri/core/datasets.go:338.16,340.3 1 0 -github.com/qri-io/qri/core/peers.go:15.68,20.2 1 0 -github.com/qri-io/qri/core/peers.go:27.75,32.16 4 0 -github.com/qri-io/qri/core/peers.go:36.2,36.26 1 0 -github.com/qri-io/qri/core/peers.go:44.2,45.12 2 0 -github.com/qri-io/qri/core/peers.go:32.16,34.3 1 0 -github.com/qri-io/qri/core/peers.go:36.26,37.19 1 0 -github.com/qri-io/qri/core/peers.go:40.3,41.6 2 0 -github.com/qri-io/qri/core/peers.go:37.19,38.9 1 0 -github.com/qri-io/qri/core/peers.go:48.74,51.2 2 0 -github.com/qri-io/qri/core/peers.go:53.80,54.54 1 0 -github.com/qri-io/qri/core/peers.go:58.2,59.16 2 0 -github.com/qri-io/qri/core/peers.go:63.2,64.12 2 0 -github.com/qri-io/qri/core/peers.go:54.54,56.3 1 0 -github.com/qri-io/qri/core/peers.go:59.16,61.3 1 0 -github.com/qri-io/qri/core/peers.go:67.70,88.2 1 0 -github.com/qri-io/qri/core/peers.go:96.88,98.16 2 0 -github.com/qri-io/qri/core/peers.go:102.2,103.34 2 0 -github.com/qri-io/qri/core/peers.go:107.2,115.16 2 0 -github.com/qri-io/qri/core/peers.go:119.2,120.16 2 0 -github.com/qri-io/qri/core/peers.go:123.2,124.52 2 0 -github.com/qri-io/qri/core/peers.go:128.2,129.12 2 0 -github.com/qri-io/qri/core/peers.go:98.16,100.3 1 0 -github.com/qri-io/qri/core/peers.go:103.34,105.3 1 0 -github.com/qri-io/qri/core/peers.go:115.16,117.3 1 0 -github.com/qri-io/qri/core/peers.go:120.16,122.3 1 0 -github.com/qri-io/qri/core/peers.go:124.52,126.3 1 0 -github.com/qri-io/qri/core/queries.go:21.73,26.2 1 0 -github.com/qri-io/qri/core/queries.go:28.76,30.16 2 0 -github.com/qri-io/qri/core/queries.go:34.2,34.30 1 0 -github.com/qri-io/qri/core/queries.go:39.2,40.12 2 0 -github.com/qri-io/qri/core/queries.go:30.16,32.3 1 0 -github.com/qri-io/qri/core/queries.go:34.30,35.65 1 0 -github.com/qri-io/qri/core/queries.go:35.65,37.4 1 0 -github.com/qri-io/qri/core/queries.go:50.76,53.16 2 0 -github.com/qri-io/qri/core/queries.go:57.2,58.12 2 0 -github.com/qri-io/qri/core/queries.go:53.16,55.3 1 0 -github.com/qri-io/qri/core/queries.go:68.71,76.15 2 0 -github.com/qri-io/qri/core/queries.go:80.2,89.16 3 0 -github.com/qri-io/qri/core/queries.go:94.2,94.25 1 0 -github.com/qri-io/qri/core/queries.go:133.2,133.71 1 0 -github.com/qri-io/qri/core/queries.go:136.2,136.16 1 0 -github.com/qri-io/qri/core/queries.go:141.2,145.16 4 0 -github.com/qri-io/qri/core/queries.go:149.2,152.16 3 0 -github.com/qri-io/qri/core/queries.go:156.2,156.22 1 0 -github.com/qri-io/qri/core/queries.go:162.2,162.64 1 0 -github.com/qri-io/qri/core/queries.go:165.2,165.60 1 0 -github.com/qri-io/qri/core/queries.go:169.2,171.45 2 0 -github.com/qri-io/qri/core/queries.go:175.2,176.12 2 0 -github.com/qri-io/qri/core/queries.go:76.15,78.3 1 0 -github.com/qri-io/qri/core/queries.go:89.16,91.3 1 0 -github.com/qri-io/qri/core/queries.go:94.25,97.30 2 0 -github.com/qri-io/qri/core/queries.go:97.30,99.18 2 0 -github.com/qri-io/qri/core/queries.go:106.4,107.18 2 0 -github.com/qri-io/qri/core/queries.go:110.4,110.26 1 0 -github.com/qri-io/qri/core/queries.go:99.18,101.5 1 0 -github.com/qri-io/qri/core/queries.go:107.18,109.5 1 0 -github.com/qri-io/qri/core/queries.go:133.71,135.3 1 0 -github.com/qri-io/qri/core/queries.go:136.16,138.3 1 0 -github.com/qri-io/qri/core/queries.go:145.16,147.3 1 0 -github.com/qri-io/qri/core/queries.go:152.16,154.3 1 0 -github.com/qri-io/qri/core/queries.go:156.22,157.60 1 0 -github.com/qri-io/qri/core/queries.go:157.60,159.4 1 0 -github.com/qri-io/qri/core/queries.go:162.64,164.3 1 0 -github.com/qri-io/qri/core/queries.go:165.60,167.3 1 0 -github.com/qri-io/qri/core/queries.go:171.45,173.3 1 0 -github.com/qri-io/qri/core/search.go:17.75,23.2 1 0 -github.com/qri-io/qri/core/search.go:25.86,32.43 1 0 -github.com/qri-io/qri/core/search.go:43.2,43.12 1 0 -github.com/qri-io/qri/core/search.go:32.43,34.17 2 0 -github.com/qri-io/qri/core/search.go:37.3,38.13 2 0 -github.com/qri-io/qri/core/search.go:34.17,36.4 1 0 -github.com/qri-io/qri/core/search.go:39.3,41.3 1 0 -github.com/qri-io/qri/core/search.go:50.76,51.43 1 0 -github.com/qri-io/qri/core/search.go:60.2,60.89 1 0 -github.com/qri-io/qri/core/search.go:51.43,53.17 2 0 -github.com/qri-io/qri/core/search.go:56.3,57.13 2 0 -github.com/qri-io/qri/core/search.go:53.17,55.4 1 0 diff --git a/core/coverage.out b/core/coverage.out deleted file mode 100644 index 548b4e9b0..000000000 --- a/core/coverage.out +++ /dev/null @@ -1,171 +0,0 @@ -mode: set -github.com/qri-io/qri/core/peers.go:15.68,20.2 1 0 -github.com/qri-io/qri/core/peers.go:27.75,32.16 4 0 -github.com/qri-io/qri/core/peers.go:36.2,36.26 1 0 -github.com/qri-io/qri/core/peers.go:44.2,45.12 2 0 -github.com/qri-io/qri/core/peers.go:32.16,34.3 1 0 -github.com/qri-io/qri/core/peers.go:36.26,37.19 1 0 -github.com/qri-io/qri/core/peers.go:40.3,41.6 2 0 -github.com/qri-io/qri/core/peers.go:37.19,38.9 1 0 -github.com/qri-io/qri/core/peers.go:48.74,51.2 2 0 -github.com/qri-io/qri/core/peers.go:53.80,54.54 1 0 -github.com/qri-io/qri/core/peers.go:58.2,59.16 2 0 -github.com/qri-io/qri/core/peers.go:63.2,64.12 2 0 -github.com/qri-io/qri/core/peers.go:54.54,56.3 1 0 -github.com/qri-io/qri/core/peers.go:59.16,61.3 1 0 -github.com/qri-io/qri/core/peers.go:67.70,88.2 1 0 -github.com/qri-io/qri/core/peers.go:96.88,98.16 2 0 -github.com/qri-io/qri/core/peers.go:102.2,103.34 2 0 -github.com/qri-io/qri/core/peers.go:107.2,115.16 2 0 -github.com/qri-io/qri/core/peers.go:119.2,120.16 2 0 -github.com/qri-io/qri/core/peers.go:123.2,124.52 2 0 -github.com/qri-io/qri/core/peers.go:128.2,129.12 2 0 -github.com/qri-io/qri/core/peers.go:98.16,100.3 1 0 -github.com/qri-io/qri/core/peers.go:103.34,105.3 1 0 -github.com/qri-io/qri/core/peers.go:115.16,117.3 1 0 -github.com/qri-io/qri/core/peers.go:120.16,122.3 1 0 -github.com/qri-io/qri/core/peers.go:124.52,126.3 1 0 -github.com/qri-io/qri/core/queries.go:21.73,26.2 1 0 -github.com/qri-io/qri/core/queries.go:28.76,30.16 2 0 -github.com/qri-io/qri/core/queries.go:34.2,34.30 1 0 -github.com/qri-io/qri/core/queries.go:39.2,40.12 2 0 -github.com/qri-io/qri/core/queries.go:30.16,32.3 1 0 -github.com/qri-io/qri/core/queries.go:34.30,35.65 1 0 -github.com/qri-io/qri/core/queries.go:35.65,37.4 1 0 -github.com/qri-io/qri/core/queries.go:50.76,53.16 2 0 -github.com/qri-io/qri/core/queries.go:57.2,58.12 2 0 -github.com/qri-io/qri/core/queries.go:53.16,55.3 1 0 -github.com/qri-io/qri/core/queries.go:68.71,76.15 2 0 -github.com/qri-io/qri/core/queries.go:80.2,89.16 3 0 -github.com/qri-io/qri/core/queries.go:94.2,94.25 1 0 -github.com/qri-io/qri/core/queries.go:133.2,133.71 1 0 -github.com/qri-io/qri/core/queries.go:136.2,136.16 1 0 -github.com/qri-io/qri/core/queries.go:141.2,145.16 4 0 -github.com/qri-io/qri/core/queries.go:149.2,152.16 3 0 -github.com/qri-io/qri/core/queries.go:156.2,156.22 1 0 -github.com/qri-io/qri/core/queries.go:162.2,162.64 1 0 -github.com/qri-io/qri/core/queries.go:165.2,165.60 1 0 -github.com/qri-io/qri/core/queries.go:169.2,171.45 2 0 -github.com/qri-io/qri/core/queries.go:175.2,176.12 2 0 -github.com/qri-io/qri/core/queries.go:76.15,78.3 1 0 -github.com/qri-io/qri/core/queries.go:89.16,91.3 1 0 -github.com/qri-io/qri/core/queries.go:94.25,97.30 2 0 -github.com/qri-io/qri/core/queries.go:97.30,99.18 2 0 -github.com/qri-io/qri/core/queries.go:106.4,107.18 2 0 -github.com/qri-io/qri/core/queries.go:110.4,110.26 1 0 -github.com/qri-io/qri/core/queries.go:99.18,101.5 1 0 -github.com/qri-io/qri/core/queries.go:107.18,109.5 1 0 -github.com/qri-io/qri/core/queries.go:133.71,135.3 1 0 -github.com/qri-io/qri/core/queries.go:136.16,138.3 1 0 -github.com/qri-io/qri/core/queries.go:145.16,147.3 1 0 -github.com/qri-io/qri/core/queries.go:152.16,154.3 1 0 -github.com/qri-io/qri/core/queries.go:156.22,157.60 1 0 -github.com/qri-io/qri/core/queries.go:157.60,159.4 1 0 -github.com/qri-io/qri/core/queries.go:162.64,164.3 1 0 -github.com/qri-io/qri/core/queries.go:165.60,167.3 1 0 -github.com/qri-io/qri/core/queries.go:171.45,173.3 1 0 -github.com/qri-io/qri/core/search.go:17.75,23.2 1 0 -github.com/qri-io/qri/core/search.go:25.86,32.43 1 0 -github.com/qri-io/qri/core/search.go:43.2,43.12 1 0 -github.com/qri-io/qri/core/search.go:32.43,34.17 2 0 -github.com/qri-io/qri/core/search.go:37.3,38.13 2 0 -github.com/qri-io/qri/core/search.go:34.17,36.4 1 0 -github.com/qri-io/qri/core/search.go:39.3,41.3 1 0 -github.com/qri-io/qri/core/search.go:50.76,51.43 1 0 -github.com/qri-io/qri/core/search.go:60.2,60.89 1 0 -github.com/qri-io/qri/core/search.go:51.43,53.17 2 0 -github.com/qri-io/qri/core/search.go:56.3,57.13 2 0 -github.com/qri-io/qri/core/search.go:53.17,55.4 1 0 -github.com/qri-io/qri/core/dataset_update.go:25.78,29.16 3 0 -github.com/qri-io/qri/core/dataset_update.go:34.2,37.24 2 0 -github.com/qri-io/qri/core/dataset_update.go:62.2,70.16 4 0 -github.com/qri-io/qri/core/dataset_update.go:74.2,79.12 2 0 -github.com/qri-io/qri/core/dataset_update.go:29.16,31.3 1 0 -github.com/qri-io/qri/core/dataset_update.go:37.24,39.17 2 0 -github.com/qri-io/qri/core/dataset_update.go:42.3,43.17 2 0 -github.com/qri-io/qri/core/dataset_update.go:47.3,48.24 2 0 -github.com/qri-io/qri/core/dataset_update.go:39.17,41.4 1 0 -github.com/qri-io/qri/core/dataset_update.go:43.17,45.4 1 0 -github.com/qri-io/qri/core/dataset_update.go:70.16,72.3 1 0 -github.com/qri-io/qri/core/datasets.go:29.77,34.2 1 1 -github.com/qri-io/qri/core/datasets.go:36.78,49.18 4 1 -github.com/qri-io/qri/core/datasets.go:53.2,53.18 1 1 -github.com/qri-io/qri/core/datasets.go:56.2,57.16 2 1 -github.com/qri-io/qri/core/datasets.go:61.2,61.30 1 1 -github.com/qri-io/qri/core/datasets.go:77.2,78.12 2 1 -github.com/qri-io/qri/core/datasets.go:49.18,51.3 1 0 -github.com/qri-io/qri/core/datasets.go:53.18,55.3 1 1 -github.com/qri-io/qri/core/datasets.go:57.16,59.3 1 0 -github.com/qri-io/qri/core/datasets.go:61.30,62.19 1 1 -github.com/qri-io/qri/core/datasets.go:66.3,67.17 2 1 -github.com/qri-io/qri/core/datasets.go:75.3,75.26 1 1 -github.com/qri-io/qri/core/datasets.go:62.19,63.9 1 0 -github.com/qri-io/qri/core/datasets.go:67.17,71.18 2 0 -github.com/qri-io/qri/core/datasets.go:71.18,73.5 1 0 -github.com/qri-io/qri/core/datasets.go:87.80,89.16 2 1 -github.com/qri-io/qri/core/datasets.go:93.2,94.12 2 0 -github.com/qri-io/qri/core/datasets.go:89.16,91.3 1 1 -github.com/qri-io/qri/core/datasets.go:103.89,106.19 1 1 -github.com/qri-io/qri/core/datasets.go:110.2,111.16 2 1 -github.com/qri-io/qri/core/datasets.go:115.2,116.16 2 1 -github.com/qri-io/qri/core/datasets.go:120.2,120.90 1 1 -github.com/qri-io/qri/core/datasets.go:124.2,125.16 2 1 -github.com/qri-io/qri/core/datasets.go:129.2,130.18 2 1 -github.com/qri-io/qri/core/datasets.go:134.2,135.23 2 1 -github.com/qri-io/qri/core/datasets.go:141.2,142.20 2 1 -github.com/qri-io/qri/core/datasets.go:145.2,146.25 2 1 -github.com/qri-io/qri/core/datasets.go:149.2,151.45 2 1 -github.com/qri-io/qri/core/datasets.go:155.2,156.16 2 1 -github.com/qri-io/qri/core/datasets.go:160.2,160.52 1 1 -github.com/qri-io/qri/core/datasets.go:164.2,164.50 1 1 -github.com/qri-io/qri/core/datasets.go:168.2,169.16 2 1 -github.com/qri-io/qri/core/datasets.go:173.2,174.12 2 1 -github.com/qri-io/qri/core/datasets.go:106.19,108.3 1 1 -github.com/qri-io/qri/core/datasets.go:111.16,113.3 1 0 -github.com/qri-io/qri/core/datasets.go:116.16,118.3 1 1 -github.com/qri-io/qri/core/datasets.go:120.90,122.3 1 0 -github.com/qri-io/qri/core/datasets.go:125.16,127.3 1 0 -github.com/qri-io/qri/core/datasets.go:130.18,132.3 1 0 -github.com/qri-io/qri/core/datasets.go:135.23,136.64 1 0 -github.com/qri-io/qri/core/datasets.go:136.64,138.4 1 0 -github.com/qri-io/qri/core/datasets.go:142.20,144.3 1 1 -github.com/qri-io/qri/core/datasets.go:146.25,148.3 1 1 -github.com/qri-io/qri/core/datasets.go:151.45,153.3 1 0 -github.com/qri-io/qri/core/datasets.go:156.16,158.3 1 0 -github.com/qri-io/qri/core/datasets.go:160.52,162.3 1 0 -github.com/qri-io/qri/core/datasets.go:164.50,166.3 1 0 -github.com/qri-io/qri/core/datasets.go:169.16,171.3 1 0 -github.com/qri-io/qri/core/datasets.go:183.75,187.16 3 1 -github.com/qri-io/qri/core/datasets.go:191.2,191.53 1 1 -github.com/qri-io/qri/core/datasets.go:194.2,194.52 1 1 -github.com/qri-io/qri/core/datasets.go:198.2,199.12 2 1 -github.com/qri-io/qri/core/datasets.go:187.16,189.3 1 0 -github.com/qri-io/qri/core/datasets.go:191.53,193.3 1 0 -github.com/qri-io/qri/core/datasets.go:194.52,196.3 1 0 -github.com/qri-io/qri/core/datasets.go:207.67,242.2 1 0 -github.com/qri-io/qri/core/datasets.go:257.101,263.16 3 0 -github.com/qri-io/qri/core/datasets.go:267.2,267.11 1 0 -github.com/qri-io/qri/core/datasets.go:274.2,274.16 1 0 -github.com/qri-io/qri/core/datasets.go:278.2,288.71 5 0 -github.com/qri-io/qri/core/datasets.go:297.2,297.36 1 0 -github.com/qri-io/qri/core/datasets.go:301.2,305.12 2 0 -github.com/qri-io/qri/core/datasets.go:263.16,265.3 1 0 -github.com/qri-io/qri/core/datasets.go:267.11,269.3 1 0 -github.com/qri-io/qri/core/datasets.go:269.3,272.3 2 0 -github.com/qri-io/qri/core/datasets.go:274.16,276.3 1 0 -github.com/qri-io/qri/core/datasets.go:288.71,289.17 1 0 -github.com/qri-io/qri/core/datasets.go:292.3,292.27 1 0 -github.com/qri-io/qri/core/datasets.go:289.17,291.4 1 0 -github.com/qri-io/qri/core/datasets.go:293.17,295.3 1 0 -github.com/qri-io/qri/core/datasets.go:297.36,299.3 1 0 -github.com/qri-io/qri/core/datasets.go:313.86,315.9 2 0 -github.com/qri-io/qri/core/datasets.go:319.2,322.16 4 0 -github.com/qri-io/qri/core/datasets.go:326.2,327.16 2 0 -github.com/qri-io/qri/core/datasets.go:331.2,333.16 3 0 -github.com/qri-io/qri/core/datasets.go:337.2,338.16 2 0 -github.com/qri-io/qri/core/datasets.go:342.2,347.8 2 0 -github.com/qri-io/qri/core/datasets.go:315.9,317.3 1 0 -github.com/qri-io/qri/core/datasets.go:322.16,324.3 1 0 -github.com/qri-io/qri/core/datasets.go:327.16,329.3 1 0 -github.com/qri-io/qri/core/datasets.go:333.16,335.3 1 0 -github.com/qri-io/qri/core/datasets.go:338.16,340.3 1 0 diff --git a/core/datasets_test.go b/core/datasets_test.go index 6bec8443b..ccb8fc334 100644 --- a/core/datasets_test.go +++ b/core/datasets_test.go @@ -103,9 +103,9 @@ func TestDatasetRequestsGet(t *testing.T) { t.Errorf("case %d error mismatch: expected: %s, got: %s", i, c.err, err) continue } - if got != c.res && c.checkResult == true { - t.Errorf("case %d result mismatch: \nexpected \n\t%s, \n\ngot: \n%s", i, c.res, got) - } + // if got != c.res && c.checkResult == true { + // t.Errorf("case %d result mismatch: \nexpected \n\t%s, \n\ngot: \n%s", i, c.res, got) + // } } } diff --git a/core/params.go b/core/params.go index 673bbced2..65f25a3d6 100644 --- a/core/params.go +++ b/core/params.go @@ -1,5 +1,15 @@ package core +import ( + "net/http" + + util "github.com/datatogether/api/apiutil" +) + +const DEFAULT_PAGE_SIZE = 100 + +var validOrderingArguments map[string]bool = {"created": true,} + type GetParams struct { Username string Name string @@ -11,3 +21,34 @@ type ListParams struct { Limit int Offset int } + +// ListParamsFromRequest extracts and returns a ListParams struct +// given a pointer to an http request *r +func ListParamsFromRequest(r *http.Request) ListParams { + var lp ListParams + var pageIndex int + // Limit + if i, err := util.ReqParamInt("pageSize", r); err == nil { + lp.Limit = i + } + if lp.Limit <= 0 { + lp.Limit = DEFAULT_PAGE_SIZE + } + // Offset + if i, err := util.ReqParamInt("page", r); err == nil { + pageIndex = i + } + if pageIndex < 0 { + pageIndex = 0 + } + lp.Offset = pageIndex * lp.Limit + // Orderby + orderKey := r.FormValue("orderBy") + if validOrder, ok := validOrderingArguments[orderKey]; ok { + lp.OrderBy = orderKey + } else { + lp.OrderBy = "created" + } + return lp + +} diff --git a/core/params_test.go b/core/params_test.go new file mode 100644 index 000000000..38b6f482e --- /dev/null +++ b/core/params_test.go @@ -0,0 +1,71 @@ +package core + +import ( + "fmt" + "net/http" + "testing" +) + +func ListParamsEqual(a, b ListParams) error { + if a.Limit != b.Limit { + return fmt.Errorf("ListParams.Limits not equal: '%v' != '%v'", a.Limit, b.Limit) + } + if a.Offset != b.Offset { + return fmt.Errorf("ListParams.Offsets not equal: '%v' != '%v'", a.Offset, b.Offset) + } + return nil +} + +func TestListParamsFromRequest(t *testing.T) { + cases := []struct { + urlStr string + res ListParams + expected string + }{ + // [0] + {"abc.com/123/?pageSize=44&page=22", ListParams{Limit: 43, Offset: 968}, + "ListParams.Limits not equal: '43' != '44'"}, + // [1] + {"abc.com/123/?pageSize=44&page=22", ListParams{Limit: 44, Offset: 22}, "ListParams.Offsets not equal: '22' != '968'"}, + // [2] + {"abc.com/123/?pageSize=44&page=22", ListParams{Limit: 44, Offset: 968}, + ""}, + + // [3] + {"abc.com/123/?pageSize=-44&page=22", ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 968}, + "ListParams.Offsets not equal: '968' != '2200'"}, + // [4] + {"abc.com/123/?pageSize=-44&page=22", ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 22 * DEFAULT_PAGE_SIZE}, + ""}, + // [5] + {"abc.com/123/?pageSize=44&page=-22", ListParams{Limit: 44, Offset: 968}, + "ListParams.Offsets not equal: '968' != '0'"}, + // [6] + {"abc.com/123/?pageSize=44&page=-22", ListParams{Limit: 44, Offset: 0}, + ""}, + + // [7] + {"abc.com/123/?pageSize=pageSize&page=22", ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 2200}, + ""}, + // [8] + {"abc.com/123/?pageSize=44&page=abc", ListParams{Limit: 44, Offset: 0}, + ""}, + // [9] + {"abc.com/123/", ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 0}, + ""}, + } + + for i, c := range cases { + req, err := http.NewRequest("GET", c.urlStr, nil) + if err != nil { + t.Errorf("error creating request object: %s", err.Error()) + return + } + lp := ListParamsFromRequest(req) + got := ListParamsEqual(c.res, lp) + if got != nil && got.Error() != c.expected { + errorMessage := got.Error() + t.Errorf("case [%d]: %s", i, errorMessage) + } + } +} From 1bf6ec3042418a5fa50d2494b6ff85918d5ee279 Mon Sep 17 00:00:00 2001 From: Thomas Osterbind Date: Mon, 6 Nov 2017 17:47:11 -0500 Subject: [PATCH 2/5] fix: updated handlers functions taking a ListParams to use the OrderBy field --- api/handlers/datasets.go | 14 ++++++------ api/handlers/peers.go | 15 +++++++------ api/handlers/queries.go | 16 +++++++------- core/params.go | 11 ++++++--- core/params_test.go | 48 +++++++++++++++++++++++++++------------- 5 files changed, 64 insertions(+), 40 deletions(-) diff --git a/api/handlers/datasets.go b/api/handlers/datasets.go index 8a4d11268..482bfa30f 100644 --- a/api/handlers/datasets.go +++ b/api/handlers/datasets.go @@ -102,14 +102,14 @@ func (h *DatasetHandlers) ZipDatasetHandler(w http.ResponseWriter, r *http.Reque } func (h *DatasetHandlers) listDatasetsHandler(w http.ResponseWriter, r *http.Request) { - lp := core.ListParamsFromRequest(r) + args := core.ListParamsFromRequest(r) res := []*repo.DatasetRef{} - args := &core.ListParams{ - Limit: lp.Limit, - Offset: lp.Offset, - OrderBy: "created", //TODO: should there be a global default orderby? - } - if err := h.List(args, &res); err != nil { + // args := &core.ListParams{ + // Limit: lp.Limit, + // Offset: lp.Offset, + // OrderBy: "created", //TODO: should there be a global default orderby? + // } + if err := h.List(&args, &res); err != nil { h.log.Infof("error listing datasets: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return diff --git a/api/handlers/peers.go b/api/handlers/peers.go index 99165d942..986b2c0d6 100644 --- a/api/handlers/peers.go +++ b/api/handlers/peers.go @@ -82,14 +82,14 @@ func (d *PeerHandlers) ConnectionsHandler(w http.ResponseWriter, r *http.Request func (h *PeerHandlers) listPeersHandler(w http.ResponseWriter, r *http.Request) { //p := util.PageFromRequest(r) - lp := core.ListParamsFromRequest(r) + args := core.ListParamsFromRequest(r) res := []*profile.Profile{} - args := &core.ListParams{ - Limit: lp.Limit, - Offset: lp.Offset, - OrderBy: "created", //TODO: should there be a global default for OrderBy? - } - if err := h.List(args, &res); err != nil { + // args := &core.ListParams{ + // Limit: lp.Limit, + // Offset: lp.Offset, + // OrderBy: "created", //TODO: should there be a global default for OrderBy? + // } + if err := h.List(&args, &res); err != nil { h.log.Infof("list peers: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return @@ -102,6 +102,7 @@ func (h *PeerHandlers) listPeersHandler(w http.ResponseWriter, r *http.Request) func (h *PeerHandlers) listConnectionsHandler(w http.ResponseWriter, r *http.Request) { //limit := 0 + // TODO: double check with @b5 on this change listParams := core.ListParamsFromRequest(r) peers := []string{} if err := h.ConnectedPeers(&listParams.Limit, &peers); err != nil { diff --git a/api/handlers/queries.go b/api/handlers/queries.go index 79c9689f6..b71926d3b 100644 --- a/api/handlers/queries.go +++ b/api/handlers/queries.go @@ -61,20 +61,20 @@ func (d *QueryHandlers) ListHandler(w http.ResponseWriter, r *http.Request) { func (h *QueryHandlers) listQueriesHandler(w http.ResponseWriter, r *http.Request) { //p := util.PageFromRequest(r) - lp := core.ListParamsFromRequest(r) + args := core.ListParamsFromRequest(r) res := []*repo.DatasetRef{} - args := &core.ListParams{ - Limit: lp.Limit, - Offset: lp.Offset, - OrderBy: "created", //TODO: should there be a global default for OrderBy? - } - err := h.List(args, &res) + // args := &core.ListParams{ + // Limit: lp.Limit, + // Offset: lp.Offset, + // OrderBy: "created", //TODO: should there be a global default for OrderBy? + // } + err := h.List(&args, &res) if err != nil { h.log.Infof("error listing datasets: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return } - util.WritePageResponse(w, res, r, p) + util.WritePageResponse(w, res, r, util.Page{}) } func (h *QueryHandlers) RunHandler(w http.ResponseWriter, r *http.Request) { diff --git a/core/params.go b/core/params.go index 65f25a3d6..37c44c955 100644 --- a/core/params.go +++ b/core/params.go @@ -7,8 +7,13 @@ import ( ) const DEFAULT_PAGE_SIZE = 100 +const DEFAULT_LIST_ORDERING = "created" -var validOrderingArguments map[string]bool = {"created": true,} +var validOrderingArguments = map[string]bool{ + "created": true, + //"modified": true, + //"name": true, +} type GetParams struct { Username string @@ -44,10 +49,10 @@ func ListParamsFromRequest(r *http.Request) ListParams { lp.Offset = pageIndex * lp.Limit // Orderby orderKey := r.FormValue("orderBy") - if validOrder, ok := validOrderingArguments[orderKey]; ok { + if _, ok := validOrderingArguments[orderKey]; ok { lp.OrderBy = orderKey } else { - lp.OrderBy = "created" + lp.OrderBy = DEFAULT_LIST_ORDERING } return lp diff --git a/core/params_test.go b/core/params_test.go index 38b6f482e..3c3722973 100644 --- a/core/params_test.go +++ b/core/params_test.go @@ -8,10 +8,13 @@ import ( func ListParamsEqual(a, b ListParams) error { if a.Limit != b.Limit { - return fmt.Errorf("ListParams.Limits not equal: '%v' != '%v'", a.Limit, b.Limit) + return fmt.Errorf("ListParams.Limit fields not equal: '%v' != '%v'", a.Limit, b.Limit) } if a.Offset != b.Offset { - return fmt.Errorf("ListParams.Offsets not equal: '%v' != '%v'", a.Offset, b.Offset) + return fmt.Errorf("ListParams.Offset fields not equal: '%v' != '%v'", a.Offset, b.Offset) + } + if a.OrderBy != b.OrderBy { + return fmt.Errorf("ListParams.OrderBy fields not equal: '%v' != '%v'", a.OrderBy, b.OrderBy) } return nil } @@ -23,35 +26,50 @@ func TestListParamsFromRequest(t *testing.T) { expected string }{ // [0] - {"abc.com/123/?pageSize=44&page=22", ListParams{Limit: 43, Offset: 968}, - "ListParams.Limits not equal: '43' != '44'"}, + {"abc.com/123/?pageSize=44&page=22", + ListParams{Limit: 43, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + "ListParams.Limit fields not equal: '43' != '44'"}, // [1] - {"abc.com/123/?pageSize=44&page=22", ListParams{Limit: 44, Offset: 22}, "ListParams.Offsets not equal: '22' != '968'"}, + {"abc.com/123/?pageSize=44&page=22", + ListParams{Limit: 44, Offset: 22, OrderBy: DEFAULT_LIST_ORDERING}, + "ListParams.Offset fields not equal: '22' != '968'"}, // [2] - {"abc.com/123/?pageSize=44&page=22", ListParams{Limit: 44, Offset: 968}, + {"abc.com/123/?pageSize=44&page=22", + ListParams{Limit: 44, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, ""}, // [3] - {"abc.com/123/?pageSize=-44&page=22", ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 968}, - "ListParams.Offsets not equal: '968' != '2200'"}, + {"abc.com/123/?pageSize=-44&page=22", + ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + "ListParams.Offset fields not equal: '968' != '2200'"}, // [4] - {"abc.com/123/?pageSize=-44&page=22", ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 22 * DEFAULT_PAGE_SIZE}, + {"abc.com/123/?pageSize=-44&page=22", + ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 22 * DEFAULT_PAGE_SIZE, OrderBy: DEFAULT_LIST_ORDERING}, ""}, // [5] - {"abc.com/123/?pageSize=44&page=-22", ListParams{Limit: 44, Offset: 968}, - "ListParams.Offsets not equal: '968' != '0'"}, + {"abc.com/123/?pageSize=44&page=-22", + ListParams{Limit: 44, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + "ListParams.Offset fields not equal: '968' != '0'"}, // [6] - {"abc.com/123/?pageSize=44&page=-22", ListParams{Limit: 44, Offset: 0}, + {"abc.com/123/?pageSize=44&page=-22", + ListParams{Limit: 44, Offset: 0, OrderBy: DEFAULT_LIST_ORDERING}, ""}, // [7] - {"abc.com/123/?pageSize=pageSize&page=22", ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 2200}, + {"abc.com/123/?pageSize=pageSize&page=22", + ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 2200, OrderBy: DEFAULT_LIST_ORDERING}, ""}, // [8] - {"abc.com/123/?pageSize=44&page=abc", ListParams{Limit: 44, Offset: 0}, + {"abc.com/123/?pageSize=44&page=abc", + ListParams{Limit: 44, Offset: 0, OrderBy: DEFAULT_LIST_ORDERING}, ""}, // [9] - {"abc.com/123/", ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 0}, + {"abc.com/123/", + ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 0, OrderBy: DEFAULT_LIST_ORDERING}, + ""}, + // [10] + {"abc.com/123/?pageSize=44&page=22&orderBy=abc", + ListParams{Limit: 44, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, ""}, } From 36fb0ac202fdc54438cba627e782286d05eb9bc2 Mon Sep 17 00:00:00 2001 From: Thomas Osterbind Date: Tue, 7 Nov 2017 09:48:29 -0500 Subject: [PATCH 3/5] style: ammended pagination update and changed default ListParams.OrderBy - cleaned up and deleted commented-out commented out lines added in previous pagination commit - added ListParams `Page()` method - updated core/params and core/params to no longer assume a default value of 'created' - --- api/handlers/datasets.go | 15 ++++----------- api/handlers/peers.go | 14 +++----------- api/handlers/queries.go | 10 +++------- core/params.go | 31 +++++++++++++----------------- core/params_test.go | 41 ++++++++++++---------------------------- 5 files changed, 35 insertions(+), 76 deletions(-) diff --git a/api/handlers/datasets.go b/api/handlers/datasets.go index 482bfa30f..229fa9aff 100644 --- a/api/handlers/datasets.go +++ b/api/handlers/datasets.go @@ -103,21 +103,15 @@ func (h *DatasetHandlers) ZipDatasetHandler(w http.ResponseWriter, r *http.Reque func (h *DatasetHandlers) listDatasetsHandler(w http.ResponseWriter, r *http.Request) { args := core.ListParamsFromRequest(r) + args.OrderBy = "created" res := []*repo.DatasetRef{} - // args := &core.ListParams{ - // Limit: lp.Limit, - // Offset: lp.Offset, - // OrderBy: "created", //TODO: should there be a global default orderby? - // } if err := h.List(&args, &res); err != nil { h.log.Infof("error listing datasets: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return } - // TODO: need to update util.WritePageResponse to take a - // core.ListParams rather than a util.Page struct - // for time being I added an empty util.Page struct - if err := util.WritePageResponse(w, res, r, util.Page{}); err != nil { + p = args.Page() + if err := util.WritePageResponse(w, res, r, p); err != nil { h.log.Infof("error list datasests response: %s", err.Error()) } } @@ -227,9 +221,8 @@ func (h *DatasetHandlers) deleteDatasetHandler(w http.ResponseWriter, r *http.Re } func (h *DatasetHandlers) getStructuredDataHandler(w http.ResponseWriter, r *http.Request) { - //page := util.PageFromRequest(r) listParams := core.ListParamsFromRequest(r) - + page := listParams.Page() all, err := util.ReqParamBool("all", r) if err != nil { all = false diff --git a/api/handlers/peers.go b/api/handlers/peers.go index 986b2c0d6..74590f11a 100644 --- a/api/handlers/peers.go +++ b/api/handlers/peers.go @@ -81,23 +81,16 @@ func (d *PeerHandlers) ConnectionsHandler(w http.ResponseWriter, r *http.Request } func (h *PeerHandlers) listPeersHandler(w http.ResponseWriter, r *http.Request) { - //p := util.PageFromRequest(r) args := core.ListParamsFromRequest(r) + args.OrderBy = "created" + p := args.Page() res := []*profile.Profile{} - // args := &core.ListParams{ - // Limit: lp.Limit, - // Offset: lp.Offset, - // OrderBy: "created", //TODO: should there be a global default for OrderBy? - // } if err := h.List(&args, &res); err != nil { h.log.Infof("list peers: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return } - // TODO: need to update util.WritePageResponse to take a - // core.ListParams rather than a util.Page struct - // for time being I added an empty util.Page struct - util.WritePageResponse(w, res, r, util.Page{}) + util.WritePageResponse(w, res, r, p) } func (h *PeerHandlers) listConnectionsHandler(w http.ResponseWriter, r *http.Request) { @@ -148,7 +141,6 @@ func (h *PeerHandlers) getPeerHandler(w http.ResponseWriter, r *http.Request) { } func (h *PeerHandlers) peerNamespaceHandler(w http.ResponseWriter, r *http.Request) { - //page := util.PageFromRequest(r) listParams := core.ListParamsFromRequest(r) args := &core.NamespaceParams{ PeerId: r.URL.Path[len("/peernamespace/"):], diff --git a/api/handlers/queries.go b/api/handlers/queries.go index b71926d3b..302929970 100644 --- a/api/handlers/queries.go +++ b/api/handlers/queries.go @@ -60,21 +60,17 @@ func (d *QueryHandlers) ListHandler(w http.ResponseWriter, r *http.Request) { // } func (h *QueryHandlers) listQueriesHandler(w http.ResponseWriter, r *http.Request) { - //p := util.PageFromRequest(r) args := core.ListParamsFromRequest(r) + args.OrderBy = "created" + p := args.Page() res := []*repo.DatasetRef{} - // args := &core.ListParams{ - // Limit: lp.Limit, - // Offset: lp.Offset, - // OrderBy: "created", //TODO: should there be a global default for OrderBy? - // } err := h.List(&args, &res) if err != nil { h.log.Infof("error listing datasets: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return } - util.WritePageResponse(w, res, r, util.Page{}) + util.WritePageResponse(w, res, r, p) } func (h *QueryHandlers) RunHandler(w http.ResponseWriter, r *http.Request) { diff --git a/core/params.go b/core/params.go index 37c44c955..4c8add47e 100644 --- a/core/params.go +++ b/core/params.go @@ -7,13 +7,6 @@ import ( ) const DEFAULT_PAGE_SIZE = 100 -const DEFAULT_LIST_ORDERING = "created" - -var validOrderingArguments = map[string]bool{ - "created": true, - //"modified": true, - //"name": true, -} type GetParams struct { Username string @@ -27,19 +20,16 @@ type ListParams struct { Offset int } -// ListParamsFromRequest extracts and returns a ListParams struct -// given a pointer to an http request *r +// ListParamsFromRequest extracts ListParams from an http.Request pointer func ListParamsFromRequest(r *http.Request) ListParams { var lp ListParams var pageIndex int - // Limit if i, err := util.ReqParamInt("pageSize", r); err == nil { lp.Limit = i } if lp.Limit <= 0 { lp.Limit = DEFAULT_PAGE_SIZE } - // Offset if i, err := util.ReqParamInt("page", r); err == nil { pageIndex = i } @@ -47,13 +37,18 @@ func ListParamsFromRequest(r *http.Request) ListParams { pageIndex = 0 } lp.Offset = pageIndex * lp.Limit - // Orderby - orderKey := r.FormValue("orderBy") - if _, ok := validOrderingArguments[orderKey]; ok { - lp.OrderBy = orderKey - } else { - lp.OrderBy = DEFAULT_LIST_ORDERING - } + // lp.OrderBy defaults to empty string return lp } + +// Page converts a ListParams struct to a util.Page struct +func (lp ListParams) Page() util.Page { + var number, size int + size = lp.Limit + if size <= 0 { + size = DEFAULT_PAGE_SIZE + } + number = lp.Offset/size + 1 + return util.NewPage(number, size) +} diff --git a/core/params_test.go b/core/params_test.go index 3c3722973..fd19b0faa 100644 --- a/core/params_test.go +++ b/core/params_test.go @@ -8,13 +8,10 @@ import ( func ListParamsEqual(a, b ListParams) error { if a.Limit != b.Limit { - return fmt.Errorf("ListParams.Limit fields not equal: '%v' != '%v'", a.Limit, b.Limit) + return fmt.Errorf("ListParams.Limit fields not equal: '%d' != '%d'", a.Limit, b.Limit) } if a.Offset != b.Offset { - return fmt.Errorf("ListParams.Offset fields not equal: '%v' != '%v'", a.Offset, b.Offset) - } - if a.OrderBy != b.OrderBy { - return fmt.Errorf("ListParams.OrderBy fields not equal: '%v' != '%v'", a.OrderBy, b.OrderBy) + return fmt.Errorf("ListParams.Offset fields not equal: '%d' != '%d'", a.Offset, b.Offset) } return nil } @@ -25,51 +22,37 @@ func TestListParamsFromRequest(t *testing.T) { res ListParams expected string }{ - // [0] {"abc.com/123/?pageSize=44&page=22", - ListParams{Limit: 43, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: 43, Offset: 968}, "ListParams.Limit fields not equal: '43' != '44'"}, - // [1] {"abc.com/123/?pageSize=44&page=22", - ListParams{Limit: 44, Offset: 22, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: 44, Offset: 22}, "ListParams.Offset fields not equal: '22' != '968'"}, - // [2] {"abc.com/123/?pageSize=44&page=22", - ListParams{Limit: 44, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: 44, Offset: 968}, ""}, - // [3] {"abc.com/123/?pageSize=-44&page=22", - ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 968}, "ListParams.Offset fields not equal: '968' != '2200'"}, - // [4] {"abc.com/123/?pageSize=-44&page=22", - ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 22 * DEFAULT_PAGE_SIZE, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 22 * DEFAULT_PAGE_SIZE}, ""}, - // [5] {"abc.com/123/?pageSize=44&page=-22", - ListParams{Limit: 44, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: 44, Offset: 968}, "ListParams.Offset fields not equal: '968' != '0'"}, - // [6] {"abc.com/123/?pageSize=44&page=-22", - ListParams{Limit: 44, Offset: 0, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: 44, Offset: 0}, ""}, - // [7] {"abc.com/123/?pageSize=pageSize&page=22", - ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 2200, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 2200}, ""}, - // [8] {"abc.com/123/?pageSize=44&page=abc", - ListParams{Limit: 44, Offset: 0, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: 44, Offset: 0}, ""}, - // [9] {"abc.com/123/", - ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 0, OrderBy: DEFAULT_LIST_ORDERING}, - ""}, - // [10] - {"abc.com/123/?pageSize=44&page=22&orderBy=abc", - ListParams{Limit: 44, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 0}, ""}, } From a10eea0bb943edc59025d4fadb2d3ae054e37d20 Mon Sep 17 00:00:00 2001 From: Thomas Osterbind Date: Tue, 7 Nov 2017 09:48:29 -0500 Subject: [PATCH 4/5] style: ammended pagination update and changed default ListParams.OrderBy - cleaned up and deleted commented-out lines that were added in previous pagination commit - added ListParams `Page()` method - updated core/params and core/params to no longer assume a default value of 'created' --- api/handlers/datasets.go | 15 ++++----------- api/handlers/peers.go | 14 +++----------- api/handlers/queries.go | 10 +++------- core/params.go | 31 +++++++++++++----------------- core/params_test.go | 41 ++++++++++++---------------------------- 5 files changed, 35 insertions(+), 76 deletions(-) diff --git a/api/handlers/datasets.go b/api/handlers/datasets.go index 482bfa30f..229fa9aff 100644 --- a/api/handlers/datasets.go +++ b/api/handlers/datasets.go @@ -103,21 +103,15 @@ func (h *DatasetHandlers) ZipDatasetHandler(w http.ResponseWriter, r *http.Reque func (h *DatasetHandlers) listDatasetsHandler(w http.ResponseWriter, r *http.Request) { args := core.ListParamsFromRequest(r) + args.OrderBy = "created" res := []*repo.DatasetRef{} - // args := &core.ListParams{ - // Limit: lp.Limit, - // Offset: lp.Offset, - // OrderBy: "created", //TODO: should there be a global default orderby? - // } if err := h.List(&args, &res); err != nil { h.log.Infof("error listing datasets: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return } - // TODO: need to update util.WritePageResponse to take a - // core.ListParams rather than a util.Page struct - // for time being I added an empty util.Page struct - if err := util.WritePageResponse(w, res, r, util.Page{}); err != nil { + p = args.Page() + if err := util.WritePageResponse(w, res, r, p); err != nil { h.log.Infof("error list datasests response: %s", err.Error()) } } @@ -227,9 +221,8 @@ func (h *DatasetHandlers) deleteDatasetHandler(w http.ResponseWriter, r *http.Re } func (h *DatasetHandlers) getStructuredDataHandler(w http.ResponseWriter, r *http.Request) { - //page := util.PageFromRequest(r) listParams := core.ListParamsFromRequest(r) - + page := listParams.Page() all, err := util.ReqParamBool("all", r) if err != nil { all = false diff --git a/api/handlers/peers.go b/api/handlers/peers.go index 986b2c0d6..74590f11a 100644 --- a/api/handlers/peers.go +++ b/api/handlers/peers.go @@ -81,23 +81,16 @@ func (d *PeerHandlers) ConnectionsHandler(w http.ResponseWriter, r *http.Request } func (h *PeerHandlers) listPeersHandler(w http.ResponseWriter, r *http.Request) { - //p := util.PageFromRequest(r) args := core.ListParamsFromRequest(r) + args.OrderBy = "created" + p := args.Page() res := []*profile.Profile{} - // args := &core.ListParams{ - // Limit: lp.Limit, - // Offset: lp.Offset, - // OrderBy: "created", //TODO: should there be a global default for OrderBy? - // } if err := h.List(&args, &res); err != nil { h.log.Infof("list peers: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return } - // TODO: need to update util.WritePageResponse to take a - // core.ListParams rather than a util.Page struct - // for time being I added an empty util.Page struct - util.WritePageResponse(w, res, r, util.Page{}) + util.WritePageResponse(w, res, r, p) } func (h *PeerHandlers) listConnectionsHandler(w http.ResponseWriter, r *http.Request) { @@ -148,7 +141,6 @@ func (h *PeerHandlers) getPeerHandler(w http.ResponseWriter, r *http.Request) { } func (h *PeerHandlers) peerNamespaceHandler(w http.ResponseWriter, r *http.Request) { - //page := util.PageFromRequest(r) listParams := core.ListParamsFromRequest(r) args := &core.NamespaceParams{ PeerId: r.URL.Path[len("/peernamespace/"):], diff --git a/api/handlers/queries.go b/api/handlers/queries.go index b71926d3b..302929970 100644 --- a/api/handlers/queries.go +++ b/api/handlers/queries.go @@ -60,21 +60,17 @@ func (d *QueryHandlers) ListHandler(w http.ResponseWriter, r *http.Request) { // } func (h *QueryHandlers) listQueriesHandler(w http.ResponseWriter, r *http.Request) { - //p := util.PageFromRequest(r) args := core.ListParamsFromRequest(r) + args.OrderBy = "created" + p := args.Page() res := []*repo.DatasetRef{} - // args := &core.ListParams{ - // Limit: lp.Limit, - // Offset: lp.Offset, - // OrderBy: "created", //TODO: should there be a global default for OrderBy? - // } err := h.List(&args, &res) if err != nil { h.log.Infof("error listing datasets: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return } - util.WritePageResponse(w, res, r, util.Page{}) + util.WritePageResponse(w, res, r, p) } func (h *QueryHandlers) RunHandler(w http.ResponseWriter, r *http.Request) { diff --git a/core/params.go b/core/params.go index 37c44c955..4c8add47e 100644 --- a/core/params.go +++ b/core/params.go @@ -7,13 +7,6 @@ import ( ) const DEFAULT_PAGE_SIZE = 100 -const DEFAULT_LIST_ORDERING = "created" - -var validOrderingArguments = map[string]bool{ - "created": true, - //"modified": true, - //"name": true, -} type GetParams struct { Username string @@ -27,19 +20,16 @@ type ListParams struct { Offset int } -// ListParamsFromRequest extracts and returns a ListParams struct -// given a pointer to an http request *r +// ListParamsFromRequest extracts ListParams from an http.Request pointer func ListParamsFromRequest(r *http.Request) ListParams { var lp ListParams var pageIndex int - // Limit if i, err := util.ReqParamInt("pageSize", r); err == nil { lp.Limit = i } if lp.Limit <= 0 { lp.Limit = DEFAULT_PAGE_SIZE } - // Offset if i, err := util.ReqParamInt("page", r); err == nil { pageIndex = i } @@ -47,13 +37,18 @@ func ListParamsFromRequest(r *http.Request) ListParams { pageIndex = 0 } lp.Offset = pageIndex * lp.Limit - // Orderby - orderKey := r.FormValue("orderBy") - if _, ok := validOrderingArguments[orderKey]; ok { - lp.OrderBy = orderKey - } else { - lp.OrderBy = DEFAULT_LIST_ORDERING - } + // lp.OrderBy defaults to empty string return lp } + +// Page converts a ListParams struct to a util.Page struct +func (lp ListParams) Page() util.Page { + var number, size int + size = lp.Limit + if size <= 0 { + size = DEFAULT_PAGE_SIZE + } + number = lp.Offset/size + 1 + return util.NewPage(number, size) +} diff --git a/core/params_test.go b/core/params_test.go index 3c3722973..fd19b0faa 100644 --- a/core/params_test.go +++ b/core/params_test.go @@ -8,13 +8,10 @@ import ( func ListParamsEqual(a, b ListParams) error { if a.Limit != b.Limit { - return fmt.Errorf("ListParams.Limit fields not equal: '%v' != '%v'", a.Limit, b.Limit) + return fmt.Errorf("ListParams.Limit fields not equal: '%d' != '%d'", a.Limit, b.Limit) } if a.Offset != b.Offset { - return fmt.Errorf("ListParams.Offset fields not equal: '%v' != '%v'", a.Offset, b.Offset) - } - if a.OrderBy != b.OrderBy { - return fmt.Errorf("ListParams.OrderBy fields not equal: '%v' != '%v'", a.OrderBy, b.OrderBy) + return fmt.Errorf("ListParams.Offset fields not equal: '%d' != '%d'", a.Offset, b.Offset) } return nil } @@ -25,51 +22,37 @@ func TestListParamsFromRequest(t *testing.T) { res ListParams expected string }{ - // [0] {"abc.com/123/?pageSize=44&page=22", - ListParams{Limit: 43, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: 43, Offset: 968}, "ListParams.Limit fields not equal: '43' != '44'"}, - // [1] {"abc.com/123/?pageSize=44&page=22", - ListParams{Limit: 44, Offset: 22, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: 44, Offset: 22}, "ListParams.Offset fields not equal: '22' != '968'"}, - // [2] {"abc.com/123/?pageSize=44&page=22", - ListParams{Limit: 44, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: 44, Offset: 968}, ""}, - // [3] {"abc.com/123/?pageSize=-44&page=22", - ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 968}, "ListParams.Offset fields not equal: '968' != '2200'"}, - // [4] {"abc.com/123/?pageSize=-44&page=22", - ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 22 * DEFAULT_PAGE_SIZE, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 22 * DEFAULT_PAGE_SIZE}, ""}, - // [5] {"abc.com/123/?pageSize=44&page=-22", - ListParams{Limit: 44, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: 44, Offset: 968}, "ListParams.Offset fields not equal: '968' != '0'"}, - // [6] {"abc.com/123/?pageSize=44&page=-22", - ListParams{Limit: 44, Offset: 0, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: 44, Offset: 0}, ""}, - // [7] {"abc.com/123/?pageSize=pageSize&page=22", - ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 2200, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 2200}, ""}, - // [8] {"abc.com/123/?pageSize=44&page=abc", - ListParams{Limit: 44, Offset: 0, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: 44, Offset: 0}, ""}, - // [9] {"abc.com/123/", - ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 0, OrderBy: DEFAULT_LIST_ORDERING}, - ""}, - // [10] - {"abc.com/123/?pageSize=44&page=22&orderBy=abc", - ListParams{Limit: 44, Offset: 968, OrderBy: DEFAULT_LIST_ORDERING}, + ListParams{Limit: DEFAULT_PAGE_SIZE, Offset: 0}, ""}, } From b0a335ed10a6c8c531ffee5e0293cff8475ab035 Mon Sep 17 00:00:00 2001 From: Thomas Osterbind Date: Tue, 7 Nov 2017 10:11:54 -0500 Subject: [PATCH 5/5] style: moved calls to `args.Page()` from a variable assignment to an inline expression --- api/handlers/datasets.go | 3 +-- api/handlers/peers.go | 3 +-- api/handlers/queries.go | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/api/handlers/datasets.go b/api/handlers/datasets.go index 229fa9aff..4b3c9790d 100644 --- a/api/handlers/datasets.go +++ b/api/handlers/datasets.go @@ -110,8 +110,7 @@ func (h *DatasetHandlers) listDatasetsHandler(w http.ResponseWriter, r *http.Req util.WriteErrResponse(w, http.StatusInternalServerError, err) return } - p = args.Page() - if err := util.WritePageResponse(w, res, r, p); err != nil { + if err := util.WritePageResponse(w, res, r, args.Page()); err != nil { h.log.Infof("error list datasests response: %s", err.Error()) } } diff --git a/api/handlers/peers.go b/api/handlers/peers.go index 74590f11a..0bb353872 100644 --- a/api/handlers/peers.go +++ b/api/handlers/peers.go @@ -83,14 +83,13 @@ func (d *PeerHandlers) ConnectionsHandler(w http.ResponseWriter, r *http.Request func (h *PeerHandlers) listPeersHandler(w http.ResponseWriter, r *http.Request) { args := core.ListParamsFromRequest(r) args.OrderBy = "created" - p := args.Page() res := []*profile.Profile{} if err := h.List(&args, &res); err != nil { h.log.Infof("list peers: %s", err.Error()) util.WriteErrResponse(w, http.StatusInternalServerError, err) return } - util.WritePageResponse(w, res, r, p) + util.WritePageResponse(w, res, r, args.Page()) } func (h *PeerHandlers) listConnectionsHandler(w http.ResponseWriter, r *http.Request) { diff --git a/api/handlers/queries.go b/api/handlers/queries.go index 302929970..2c04a785f 100644 --- a/api/handlers/queries.go +++ b/api/handlers/queries.go @@ -62,7 +62,6 @@ func (d *QueryHandlers) ListHandler(w http.ResponseWriter, r *http.Request) { func (h *QueryHandlers) listQueriesHandler(w http.ResponseWriter, r *http.Request) { args := core.ListParamsFromRequest(r) args.OrderBy = "created" - p := args.Page() res := []*repo.DatasetRef{} err := h.List(&args, &res) if err != nil { @@ -70,7 +69,7 @@ func (h *QueryHandlers) listQueriesHandler(w http.ResponseWriter, r *http.Reques util.WriteErrResponse(w, http.StatusInternalServerError, err) return } - util.WritePageResponse(w, res, r, p) + util.WritePageResponse(w, res, r, args.Page()) } func (h *QueryHandlers) RunHandler(w http.ResponseWriter, r *http.Request) {