Skip to content

Commit

Permalink
chore: remove is_last_page response field (#531)
Browse files Browse the repository at this point in the history
Closes #524
  • Loading branch information
zepatrik committed Apr 7, 2021
1 parent 6ea239d commit 99b7244
Show file tree
Hide file tree
Showing 21 changed files with 58 additions and 129 deletions.
4 changes: 1 addition & 3 deletions .schema/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1050,10 +1050,8 @@
"getRelationTuplesResponse": {
"type": "object",
"properties": {
"is_last_page": {
"type": "boolean"
},
"next_page_token": {
"description": "The opaque token to provide in a subsequent request\nto get the next page. It is the empty string iff this is\nthe last page.",
"type": "string"
},
"relation_tuples": {
Expand Down
2 changes: 1 addition & 1 deletion cmd/relationtuple/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func newGetCmd() *cobra.Command {

cmdx.PrintTable(cmd, &responseOutput{
RelationTuples: relationtuple.NewProtoRelationCollection(resp.RelationTuples),
IsLastPage: resp.IsLastPage,
IsLastPage: resp.NextPageToken == "",
NextPageToken: resp.NextPageToken,
})
return nil
Expand Down
9 changes: 4 additions & 5 deletions docs/docs/reference/proto-api.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,10 @@ Example use cases (namespace is always required):

The response of a ReadService.ListRelationTuples RPC.

| Field | Type | Label | Description |
| --------------- | ----------------------------------------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------ |
| relation_tuples | [RelationTuple](#ory.keto.acl.v1alpha1.RelationTuple) | repeated | The relation tuples matching the list request. |
| next_page_token | [string](#string) | | The token required to get the next page. Please use the `is_last_page` field to determine whether this was the last page. |
| is_last_page | [bool](#bool) | | Whether this is the last page. Using the `next_page_token` in a subsequent request if this field is true will return an error. |
| Field | Type | Label | Description |
| --------------- | ----------------------------------------------------- | -------- | ------------------------------------------------------------------------------------------------------ |
| relation_tuples | [RelationTuple](#ory.keto.acl.v1alpha1.RelationTuple) | repeated | The relation tuples matching the list request. |
| next_page_token | [string](#string) | | The token required to get the next page. If this is the last page, the token will be the empty string. |

## ory/keto/acl/v1alpha1/version.proto

Expand Down
11 changes: 4 additions & 7 deletions docs/docs/reference/rest-api.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,6 @@ Status Code **500**

```json
{
"is_last_page": true,
"next_page_token": "string",
"relation_tuples": [
{
Expand Down Expand Up @@ -1996,7 +1995,6 @@ _Represents the response for a check request._

```json
{
"is_last_page": true,
"next_page_token": "string",
"relation_tuples": [
{
Expand All @@ -2011,11 +2009,10 @@ _Represents the response for a check request._

#### Properties

| Name | Type | Required | Restrictions | Description |
| --------------- | ------------------------------------------------------- | -------- | ------------ | ----------- |
| is_last_page | boolean | false | none | none |
| next_page_token | string | false | none | none |
| relation_tuples | [[InternalRelationTuple](#schemainternalrelationtuple)] | false | none | none |
| Name | Type | Required | Restrictions | Description |
| --------------- | ------------------------------------------------------- | -------- | ------------ | ----------------------------------------------------------------------------------------------------------------------------------- |
| next_page_token | string | false | none | The opaque token to provide in a subsequent request<br/>to get the next page. It is the empty string iff this is<br/>the last page. |
| relation_tuples | [[InternalRelationTuple](#schemainternalrelationtuple)] | false | none | none |

<a id="tocShealthnotreadystatus"></a>

Expand Down
2 changes: 1 addition & 1 deletion internal/check/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (e *Engine) checkOneIndirectionFurther(ctx context.Context, requested *rela
allowed, err := e.subjectIsAllowed(ctx, requested, nextRels)

// loop through pages until either allowed, end of pages, or an error occurred
if allowed || nextPage == x.PageTokenEnd || err != nil {
if allowed || nextPage == "" || err != nil {
return allowed, err
}

Expand Down
4 changes: 3 additions & 1 deletion internal/e2e/cases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ func runCases(c client, nspaces []*namespace.Namespace) func(*testing.T) {
resp relationtuple.GetResponse
nPages int
)
for ; !resp.IsLastPage; nPages++ {
// do ... while resp.NextPageToken != ""
for ok := true; ok; ok = resp.NextPageToken != "" {
resp = *c.queryTuple(t,
&relationtuple.RelationQuery{
Namespace: nspaces[0].Name,
Expand All @@ -107,6 +108,7 @@ func runCases(c client, nspaces []*namespace.Namespace) func(*testing.T) {
x.WithToken(resp.NextPageToken),
x.WithSize(1),
)
nPages++
assert.Len(t, resp.RelationTuples, 1)
}

Expand Down
1 change: 0 additions & 1 deletion internal/e2e/grpc_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ func (g *grpcClient) queryTuple(t require.TestingT, q *relationtuple.RelationQue
return &relationtuple.GetResponse{
RelationTuples: irs,
NextPageToken: resp.NextPageToken,
IsLastPage: resp.IsLastPage,
}
}

Expand Down
1 change: 0 additions & 1 deletion internal/e2e/http_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ func (c *sdkClient) queryTuple(t require.TestingT, q *relationtuple.RelationQuer
getResp := &relationtuple.GetResponse{
RelationTuples: make([]*relationtuple.InternalRelationTuple, len(resp.Payload.RelationTuples)),
NextPageToken: resp.Payload.NextPageToken,
IsLastPage: resp.Payload.IsLastPage,
}

for i, rt := range resp.Payload.RelationTuples {
Expand Down
3 changes: 2 additions & 1 deletion internal/expand/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ func (e *Engine) BuildTree(ctx context.Context, subject relationtuple.Subject, r
rels []*relationtuple.InternalRelationTuple
nextPage string
)
for nextPage != x.PageTokenEnd {
// do ... while nextPage != ""
for ok := true; ok; ok = nextPage != "" {
var err error
rels, nextPage, err = e.d.RelationTupleManager().GetRelationTuples(
ctx,
Expand Down
7 changes: 3 additions & 4 deletions internal/httpclient/models/get_relation_tuples_response.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 0 additions & 6 deletions internal/persistence/sql/persister.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,6 @@ func internalPaginationFromOptions(opts ...x.PaginationOptionSetter) (*internalP
}

func (p *internalPagination) parsePageToken(t string) error {
if t == x.PageTokenEnd {
p.PerPage = 0
p.Page = 1
return nil
}

if t == "" {
p.Page = 1
return nil
Expand Down
10 changes: 5 additions & 5 deletions internal/persistence/sql/relationtuples.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (p *Persister) DeleteRelationTuples(ctx context.Context, rs ...*relationtup
func (p *Persister) GetRelationTuples(ctx context.Context, query *relationtuple.RelationQuery, options ...x.PaginationOptionSetter) ([]*relationtuple.InternalRelationTuple, string, error) {
pagination, err := internalPaginationFromOptions(options...)
if err != nil {
return nil, x.PageTokenEnd, err
return nil, "", err
}

var wheres []whereStmts
Expand All @@ -143,7 +143,7 @@ func (p *Persister) GetRelationTuples(ctx context.Context, query *relationtuple.

n, err := p.namespaces.GetNamespace(ctx, query.Namespace)
if err != nil {
return nil, x.PageTokenEnd, err
return nil, "", err
}

sqlQuery := p.connection(context.WithValue(ctx, namespaceContextKey, n)).
Expand All @@ -156,12 +156,12 @@ func (p *Persister) GetRelationTuples(ctx context.Context, query *relationtuple.

var res relationTuples
if err := sqlQuery.All(&res); err != nil {
return nil, x.PageTokenEnd, sqlcon.HandleError(err)
return nil, "", sqlcon.HandleError(err)
}

nextPageToken := pagination.encodeNextPageToken()
if sqlQuery.Paginator.Page >= sqlQuery.Paginator.TotalPages {
nextPageToken = x.PageTokenEnd
nextPageToken = ""
}

internalRes := make([]*relationtuple.InternalRelationTuple, len(res))
Expand All @@ -171,7 +171,7 @@ func (p *Persister) GetRelationTuples(ctx context.Context, query *relationtuple.
var err error
internalRes[i], err = r.toInternal()
if err != nil {
return nil, x.PageTokenEnd, err
return nil, "", err
}
}

Expand Down
6 changes: 4 additions & 2 deletions internal/relationtuple/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ type (
// swagger:model getRelationTuplesResponse
type GetResponse struct {
RelationTuples []*InternalRelationTuple `json:"relation_tuples"`
NextPageToken string `json:"next_page_token"`
IsLastPage bool `json:"is_last_page"`
// The opaque token to provide in a subsequent request
// to get the next page. It is the empty string iff this is
// the last page.
NextPageToken string `json:"next_page_token"`
}

const (
Expand Down
10 changes: 5 additions & 5 deletions internal/relationtuple/manager_requirements.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func ManagerTest(t *testing.T, m Manager, addNamespace func(context.Context, *te

resp, nextPage, err := m.GetRelationTuples(context.Background(), (*RelationQuery)(&tupC))
require.NoError(t, err)
assert.Equal(t, x.PageTokenEnd, nextPage)
assert.Equal(t, "", nextPage)
assert.Equal(t, []*InternalRelationTuple{&tupC}, resp)
}
})
Expand Down Expand Up @@ -168,7 +168,7 @@ func ManagerTest(t *testing.T, m Manager, addNamespace func(context.Context, *te
t.Run(fmt.Sprintf("case=%d", i), func(t *testing.T) {
res, nextPage, err := m.GetRelationTuples(context.Background(), tc.query)
require.NoError(t, err)
assert.Equal(t, x.PageTokenEnd, nextPage)
assert.Equal(t, "", nextPage)

// assert equal elements but not equal order
assert.Equal(t, len(tc.expected), len(res))
Expand Down Expand Up @@ -216,7 +216,7 @@ func ManagerTest(t *testing.T, m Manager, addNamespace func(context.Context, *te
Relation: "r",
}, x.WithSize(1), x.WithToken(nextPage))
require.NoError(t, err)
assert.NotEqual(t, x.PageTokenEnd, nextPage)
assert.NotEqual(t, "", nextPage)
require.Len(t, res, 1)

var found bool
Expand All @@ -237,7 +237,7 @@ func ManagerTest(t *testing.T, m Manager, addNamespace func(context.Context, *te
Relation: "r",
}, x.WithSize(1), x.WithToken(nextPage))
require.NoError(t, err)
assert.Equal(t, x.PageTokenEnd, nextPage)
assert.Equal(t, "", nextPage)
assert.Len(t, res, 1)
assert.Equal(t, notEncounteredTuples, res)
})
Expand All @@ -252,7 +252,7 @@ func ManagerTest(t *testing.T, m Manager, addNamespace func(context.Context, *te

assert.NoError(t, err)
assert.Equal(t, []*InternalRelationTuple{}, res)
assert.Equal(t, x.PageTokenEnd, nextPage)
assert.Equal(t, "", nextPage)
})
})

Expand Down
2 changes: 0 additions & 2 deletions internal/relationtuple/read_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ func (h *handler) ListRelationTuples(ctx context.Context, req *acl.ListRelationT
resp := &acl.ListRelationTuplesResponse{
RelationTuples: make([]*acl.RelationTuple, len(rels)),
NextPageToken: nextPage,
IsLastPage: nextPage == x.PageTokenEnd,
}
for i, r := range rels {
resp.RelationTuples[i] = r.ToProto()
Expand Down Expand Up @@ -119,7 +118,6 @@ func (h *handler) getRelations(w http.ResponseWriter, r *http.Request, _ httprou
resp := &GetResponse{
RelationTuples: rels,
NextPageToken: nextPage,
IsLastPage: nextPage == x.PageTokenEnd,
}

h.d.Writer().Write(w, r, resp)
Expand Down
12 changes: 4 additions & 8 deletions internal/relationtuple/read_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ func TestReadHandlers(t *testing.T) {

assert.Equal(t, relationtuple.GetResponse{
RelationTuples: []*relationtuple.InternalRelationTuple{},
NextPageToken: x.PageTokenEnd,
IsLastPage: true,
NextPageToken: "",
}, respMsg)
})

Expand Down Expand Up @@ -86,8 +85,7 @@ func TestReadHandlers(t *testing.T) {
for i := range rts {
assert.Contains(t, respMsg.RelationTuples, rts[i])
}
assert.Equal(t, x.PageTokenEnd, respMsg.NextPageToken)
assert.True(t, respMsg.IsLastPage)
assert.Equal(t, "", respMsg.NextPageToken)
})

t.Run("case=returns bad request on malformed subject", func(t *testing.T) {
Expand Down Expand Up @@ -132,8 +130,7 @@ func TestReadHandlers(t *testing.T) {
require.NoError(t, json.NewDecoder(resp.Body).Decode(&firstResp))
require.Len(t, firstResp.RelationTuples, 1)
assert.Contains(t, rts, firstResp.RelationTuples[0])
assert.NotEqual(t, x.PageTokenEnd, firstResp.NextPageToken)
assert.False(t, firstResp.IsLastPage)
assert.NotEqual(t, "", firstResp.NextPageToken)
})

t.Run("case=second page", func(t *testing.T) {
Expand All @@ -152,8 +149,7 @@ func TestReadHandlers(t *testing.T) {

assert.NotEqual(t, firstResp.RelationTuples, secondResp.RelationTuples)
assert.Contains(t, rts, secondResp.RelationTuples[0])
assert.Equal(t, x.PageTokenEnd, secondResp.NextPageToken)
assert.True(t, secondResp.IsLastPage)
assert.Equal(t, "", secondResp.NextPageToken)
})
})

Expand Down
4 changes: 0 additions & 4 deletions internal/x/pagination.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ type (
PaginationOptionSetter func(*PaginationOptions) *PaginationOptions
)

const (
PageTokenEnd = "no other page"
)

func WithToken(t string) PaginationOptionSetter {
return func(opts *PaginationOptions) *PaginationOptions {
opts.Token = t
Expand Down
51 changes: 19 additions & 32 deletions proto/ory/keto/acl/v1alpha1/read_service.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 99b7244

Please sign in to comment.