pagination should not require ObjectIdentity

[davepacheco]

Fixes #1262 and I consider this a blocker for #1261.

Background

Most objects in the API have identity metadata (mentioned in RFD 4). There are two flavors: asset identity metadata includes an id, time_created, and time_modified. This is used for Racks and Sleds and eventually for anything that it doesn't really make sense for a human to give a "name" to. resource identity metadata includes the same fields, plus a client-controlled "name" (a unique identifier in the parent collection -- see RFD 192) and "description".

The HTTP pagination module has some canned implementations for things like ScanByName, ScanById, and ScanByNameOrId. These are oriented around resources -- that is, things with resource identity metadata. They require that the objects you're listing impl ObjectIdentity, which has one function that returns the IdentityMetadata (which means resource identity metadata) for the object.

The problem

The simple problem is that there's not a great way today to use ScanById with things that only have asset identity metadata.

• For Racks and Sleds, we worked around this by faking up the fields they don't have. This is kind of confusing for the reasons mentioned in API views for asset-type objects get dummy "name" and "description" fields #1262.
• For Sagas, we worked around this by putting a fake IdentityMetadata into the object, but we used #[serde(skip)] so at least clients couldn't tell.
• Under add API for listing silo users #1261, I need to make this work for Silo Users. I couldn't use the workaround that we use for Sagas because it doesn't work for types that also impl Deserialize. (serde needs to put something into the in-memory object on deserialization. It can supply a default value, but that wouldn't be right for the fake field here -- its id has to match the real id of the object.) I didn't want to do what we did with Racks and Sleds in part because people would almost certainly expect "name" to mean something on a User, so filling in "no-name" for every user seemed especially bad.

The solution

Another way of stating the problem above is that in order to make HTTP pagination as ergonomic as possible to use, we implemented the ScanBy{Name,Id,NameOrId} types to encapsulate the large number of nitty details associated with pagination (defining types for the scan params and pagination params, defining the marker type, defining how to get the marker value from each object, etc.). But it went a little too far: we basically want to use ScanById, but let the caller plug in their own mechanism for mapping an item to its corresponding marker value (rather than assuming everything impls ObjectIdentity and using that to produce the marker value).

The solution here is that all callers of ResultsPage::new (so, basically all users of pagination) have to supply a new argument for mapping the items in their list to the marker value. We provide a couple of functions to be used with ScanByName, ScanById, and ScanByNameOrId when the item does impl ObjectIdentity. In other words, it's a little more verbose than it used to be, but still pretty easy to use (and importantly hard to get wrong). Callers with objects that don't impl ObjectIdentity can provide their own closure that gets the id however they want.

I'm not super happy with the proliferation of closures, but I think it's basically fine for the consumers of this API. If folks have better ways to express this that don't create a huge blast radius, that'd be cool. (For example, I considered having ScanParams take a type argument that you'd use to plug in an impl of a trait that gets the marker value out, but this has a large blast radius, its own ugliness (PhantomData fields), and I think it'd be worse for callers.) Urgency is a priority at the moment.

[david-crespo]

Yep, makes perfect sense to me. ScanParams shouldn't know about ObjectIdentity.