adlnet / xAPI-Spec

The xAPI Specification describes communication about learner activity and experiences between technologies.
https://adlnet.gov/projects/xapi/
901 stars 405 forks source link

GET Statements Voided statement ID should accept array #1007

Open garemoko opened 7 years ago

garemoko commented 7 years ago

For 1.1 or 2.0.

I'm using voided statement id for the first time in a real situation today in order to fetch, edit and resend 15 statements. I'm realizing that it would be nice to fetch all 15 statements as a batch, rather than having to make 1 request per statement.

This would be especially important if it was a much larger number of statements (although in that case I might have been motivated to write some code to automate it, which is not worthwhile for 15)

garemoko commented 7 years ago

Also applies to unvoided statements fetched buy id.

DavidTPate commented 7 years ago

Yeah, this is something that I think should be allowed no matter which piece you're querying for. The way that we allow it is to go through and have the query parameter specified multiple times.

So: /lrs/statements?voidedStatementId=1234&voidedStatementId=54321

Another option would be to URL encode the parameter as an array. I'm not sure of limitations with different frameworks though.

brianjmiller commented 7 years ago

I think the array approach would be more consistent with the rest of the spec.

Note that URLs are going to get long, quickly. Your 1234 example is only partially accurate, ids are UUIDs, so 36 characters a piece (IOW if you are working in a browser you have a theoretical limit of 55 ids).

There are other problems with specifying this behavior (which is probably why it wasn't to begin with), such as what happens when statements aren't present? A 404 for a single resource is an easy answer, not so easy in the other direction. Note the GET /statements resource was never intended as a query interface, it is a stream of data, I get why this is wanted, but we need to be very guarded about trying to expand the "query" capabilities of something that wasn't intended for that purpose. Do other query parameters still apply to the given list? Presumably an LRS can still page it?

And the original use case, I get the "edit" statement need, but at what point does this become a plausible use case for inclusion in the spec, because "editing" 15 statements doesn't seem like it because that just isn't enough scale for an LRS to be concerned about. I'm also curious why the 15 requests were necessary, depending on what you are editing you are better off using the normal stream filter capability and getting all statements in a particular set at once. Is this really 15 requests or is this 15 sets of edits where you are fetching a single statement at a time and you just happen to be doing 15 at once, because I look at those as different use cases.

garemoko commented 7 years ago

@brianjmiller In this particular case it was two errors relating to email addresses that needed to be fixed. The first error involved 13 statements and the second involved 2 statements.

15 requests were needed because the bad statements had already been voided; otherwise I probably could have got the statements in 2 requests as you said. So actually another possible solution in this particular case would be an 'include voided' flag.

brianjmiller commented 7 years ago

I think the include voided flag is more inline with the stream nature of that resource and should work so long as a system isn't looking at a particular slice of statements where the voided and voiding statement aren't both seen, but in that case they are already doing something wonky.

Though there is the odd case where a voiding statement is stored before the voided statement. A system would have to in theory keep a map of voiding statements to unseen voided statements, this puts a fair amount of burden on the reading side which the specification has historically tried to prevent.

garemoko commented 7 years ago

I didn't envisage the 'included voided' flag would be something that's used routinely - only in scenarios where you know you're looking for statements that have been voided. In fact, the flag could possibly be "voided only", or maybe it's a three way option: all, not voided (default) or just voided.

DavidTPate commented 7 years ago

There's definitely some interesting security issues that come up with needing to support arrays (no matter the implementation) for these types of things. For example, I could attempt to crash/DoS a LRS by sending a very large array of statement ids that I wanted to retrieve and then cause a memory issue when dealing attempting to parse it. That's just one example, but one of the real use cases I've seen with regards to handling a more open query interface. Or a more simple one might be to request things with multiple verbs, agents, objects (arrays of them) and cause issues with the server for when it runs the queries.

I'd love to have a more open query interface, but I'm worried about the can of worms that could be opened due to it.