Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

listing discussion #1988

Open
ramfox opened this issue Dec 3, 2021 · 1 comment
Open

listing discussion #1988

ramfox opened this issue Dec 3, 2021 · 1 comment
Assignees
Labels
discussion Open-ended request for feedback

Comments

@ramfox
Copy link
Member

ramfox commented Dec 3, 2021

List Params

What is the best way to organize our list params, both below lib, as consumed by lib, and in the API

Needs:

  • how to translate between a URL and list params, how to make that consistent for the API
  • string syntax for filtering
  • string syntax for sorting
  • simplification so we don't need to reinvent listing each time we have a new endpoint

discussion

Limit, offset

  • remove all references to page/pageSize, only use limit/offset
    • we are using page/pageSize for Get requests
    • remote.Feed
  • the apiutil.Page is used in cmd (log, list, peers, search, get)

Filters

  • If we use a consistent filtering syntax, we can basically reduce all "listing params" to params.List, since we can filter by any arbitrary field in any desired way
  • Only really found best practices for this about REST APIs, had specific field names per thing to be filtered by. eg /list?workflow=true&commitTime=>2021-11-05
  • alternatives? - https://docs.microsoft.com/en-us/power-bi/collaborate-share/service-url-filters#operators
    /list?filter=workflow eq true and commitTime gt 2021-11-04&order=+name
    /list?filter=workflow:true,commitTime:>2021-11-04&order=+name
  • because you can filter by arbitrary fields, we can reduce the number of params to maintain. If they need specific validation, we can always embed the params.List into the specific struct

Sort

  • ?order=-updated,+name
  • ?order=updated[DESC],name[ASC]
  • ?order_by=created,ASC,name,DESC
  • ?orderBy[name]=ASC&orderBy[created]=DESC

I think I personally like the first one since it seems easier to type for the user

params.List

  • add method ParamsFromRequest (or some other name), that parses the query and turns it into a list params, overriding any params already in the struct
    • but Kasey, we had something like this and we removed it. Yes, but this is a special case.
  • create new ListParam interface, that only params.List satisfies. Or we use type assertion.
  • Order field becomes slice of Order struct
  • Filter field becomes list of Filter struct
type List {
   Limit int
   Offset int
   Filter []Filter
   Order []Order

Lib/http

  • in DecodeParams, add check for new interface ListParam, calling ParamsFromRequest that will pull any url query params into the list params
  • this essentially adds "sugar" syntax for listing/pagination for our listing urls
  • embed params.List into all listing params:
// CollectionParams (previously lib.ListParams)
type CollectionParams {
  params.List
  ProfileID profile.ID
  Username string
  Public bool
}

Current endpoints that list & their associated parameters:

Endpoint Param
list ListParams
activity ActivityParams
get GetParams
peer list PeerListParams
remote feeds EmptyParams
registry search SearchParams
registry follow list FollowGetParams
connections ConnectionsParams
qri connections ConnectionsParams
// ListParams is the general input for any sort of Paginated Request
// ListParams define limits & offsets, not pages & page sizes.
// TODO - rename this to PageParams.
type ListParams struct {
	// TODO(b5): what is this being used for?
	ProfileID profile.ID `json:"-" docs:"hidden"`
	// term to filter list by; e.g. "population"
	Term string `json:"term,omitempty"`
	// username to filter collection by; e.g. "ramfox"
	Username string `json:"username,omitempty"`
	// field name to order list by; e.g. "created"
	OrderBy string `json:"orderBy,omitempty"`
	// maximum number of datasets to use. use -1 to list all datasets; e.g. 50
	Limit int `json:"limit"`
	// number of items to skip; e.g. 0
	Offset int `json:"offset"`
	// Public only applies to listing datasets, shows only datasets that are
	// set to visible
	Public bool `json:"public,omitempty"`
}

// ActivityParams defines parameters for the Activity method
type ActivityParams struct {
	ListParams
	// Reference to data to fetch history for; e.g. "b5/world_bank_population"
	Ref string `json:"ref"`
	// if true, pull any datasets that aren't stored locally; e.g. false
	Pull bool `json:"pull"`
}

// GetParams defines parameters for looking up the head or body of a dataset
type GetParams struct {
	// dataset reference to fetch; e.g. "b5/world_bank_population"
	Ref string `json:"ref"`
	// a component or nested field names to extract from the dataset; e.g. "body"
	Selector string `json:"selector"`
	// number of results to limit to. only applies when selector is "body"
	Limit int `json:"limit"`
	// number of results to skip. only applies when selector is "body"
	Offset int `json:"offset"`
	// TODO(dustmop): Remove `All` once `Cursor` is in use. Instead, callers should
	// loop over their `Cursor` in order to get all rows.
	All bool `json:"all" docs:"hidden"`
}

// PeerListParams defines parameters for the List method
type PeerListParams struct {
	Limit  int `json:"limit"`
	Offset int `json:"offset"`
	// Cached == true will return offline peers from the repo
	// as well as online peers, default is to list connected peers only
	Cached bool `json:"cached"`
}

// SearchParams encapsulates parameters provided to Searchable.Search
type SearchParams struct {
	Q             string
	Limit, Offset int
}

// FollowGetParams encapsulates parameters provided to Follower.Get
type FollowGetParams struct {
	Username string `json:"username"`
	Limit    int    `json:"limit"`
	Offset   int    `json:"offset"`
	// TODO(arqu): order by fields are missing
}

// ConnectionsParams defines parameters for the Connections method
type ConnectionsParams struct {
	Limit  int `json:"limit"`
	Offset int `json:"offset"`
}
@ramfox ramfox added the discussion Open-ended request for feedback label Dec 3, 2021
@ramfox ramfox self-assigned this Dec 3, 2021
@ramfox
Copy link
Member Author

ramfox commented Dec 3, 2021

filtering:

  • what are the things that dirve this functionality, and start from there
  • we need public/private
  • currently we use filtering on search and want to use it on collection

Avoid filtering for now! needs research (microsoft, open api, elasticsearch)

Note, by dustin, it is an anti-pattern to allow enormous limits for limit

KASEY REMEMBER TO ADD BREAKING CHANGE NOTES IN COMMIT WHEN YOU REMOVE PAGE/PAGESIZE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Open-ended request for feedback
Projects
None yet
Development

No branches or pull requests

1 participant