Skip to content

Make query.FilteredPaginate use generics, and remove the accumulate parameter #12242

Closed
@ValarDragon

Description

Summary

Currently query.FilteredPaginate creates a silly developer overhead for callers, as they have to maintain the entire slice to return at the end. (and handle the complexity for the accumulate parameter.

Ideally this is instead handled by generic routines, and not the developer writing the query. This previously wasn't cleanly doable, due to needing to maintain the return type callee side. But now that generics have landed, the filtered paginate queries can easily use a generic type parameter for the slice type. Furthermore lets make this parameter a proto.message, so the serialization / deserialization can be handled for the caller.

Problem Definition

Reduce the developer overhead of writing a filtered paginate query. Currently even the simplest ones, net around 40 lines for the caller.

Proposal

Change filtered paginate from having the caller have to define the list of whats accumulated.

This should change the following API

var authorizations []*authz.Grant
pageRes, err := query.FilteredPaginate(grantsStore, req.Pagination, func(key []byte, value []byte, accumulate bool) (bool, error) {
auth, err := unmarshalAuthorization(k.cdc, value)
if err != nil {
return false, err
}
auth1, err := auth.GetAuthorization()
if err != nil {
return false, err
}
if accumulate {
msg, ok := auth1.(proto.Message)
if !ok {
return false, status.Errorf(codes.Internal, "can't protomarshal %T", msg)
}
authorizationAny, err := codectypes.NewAnyWithValue(msg)
if err != nil {
return false, status.Errorf(codes.Internal, err.Error())
}
authorizations = append(authorizations, &authz.Grant{
Authorization: authorizationAny,
Expiration: auth.Expiration,
})
}
return true, nil
})
if err != nil {
return nil, err
}
return &authz.QueryGrantsResponse{
Grants: authorizations,
Pagination: pageRes,
}, nil
to

	return query.FilteredPaginate[Authorization](grantsStore, req.Pagination, k.cdc, func(key []byte, auth Authorization) (Authorization, error) {
		auth1, err := auth.GetAuthorization()
		if err != nil {
			return false, err
		}

		// (This ok check seems useless even in he original code)	
		msg, ok := auth1.(proto.Message)
		if !ok {
			return false, status.Errorf(codes.Internal, "can't protomarshal %T", msg)
		}

		authorizationAny, err := codectypes.NewAnyWithValue(msg)
		if err != nil {
			return false, status.Errorf(codes.Internal, err.Error())
		}
	
		return authorizationAny, nil
	})

Perhaps the new function needs a new name for backwards API compatibility? I'm not too sure though, I feel like this is a code reduction that should be universally accepted here.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

  • Status

    No status

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions