yetanalytics / lrs

Protocols, specifications, and logic for building an xAPI Learning Record Store (LRS) in Clojure(Script).
https://www.yetanalytics.com/lrs
Apache License 2.0
4 stars 1 forks source link

LRS-72 Handle deduplicated attachment multiparts #81

Closed milt closed 2 years ago

milt commented 2 years ago

LRS-72 Currently lrs expects attachment requests to contain one attachment part for every attachment reference in statement data. The spec language is ambiguous in this regard: https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Communication.md#requirements-for-attachment-statement-batches

It does however say that LRPs should normalize/deduplicate attachments being sent, so LRS (and its impls) should handle this.

Example with multiple references + deduplicated attachments that currently fails on our system:

failing example ````http POST /xapi/statements HTTP/1.1 X-Experience-API-Version: 1.0.3 Content-Type: multipart/mixed; boundary=105423a5219f5a63362a375ba7a64a8f234da19c7d01e56800c3c64b26bb2fa0 Authorization: Host: localhost:8080 Connection: close User-Agent: Paw/3.3.1 (Macintosh; OS X/12.3.1) GCDHTTPRequest Content-Length: 1841 --105423a5219f5a63362a375ba7a64a8f234da19c7d01e56800c3c64b26bb2fa0 Content-Type:application/json { "actor": { "mbox": "mailto:sample.agent@example.com", "name": "Sample Agent", "objectType": "Agent" }, "verb": { "id": "http://adlnet.gov/expapi/verbs/answered", "display": { "en-US": "answered" } }, "object": { "id": "http://www.example.com/tincan/activities/multipart", "objectType": "Activity", "definition": { "name": { "en-US": "Multi Part Activity" }, "description": { "en-US": "Multi Part Activity Description" } } }, "attachments": [ { "usageType": "http://example.com/attachment-usage/test", "display": { "en-US": "A test attachment" }, "description": { "en-US": "A test attachment (description)" }, "contentType": "text/plain; charset=ascii", "length": 27, "sha2": "495395e777cd98da653df9615d09c0fd6bb2f8d4788394cd53c56a3bfdcd848a" }, { "usageType": "http://example.com/attachment-usage/test", "display": { "en-US": "A test attachment" }, "description": { "en-US": "A test attachment (description)" }, "contentType": "text/plain; charset=ascii", "length": 27, "sha2": "495395e777cd98da653df9615d09c0fd6bb2f8d4788394cd53c56a3bfdcd848a" } ] } --105423a5219f5a63362a375ba7a64a8f234da19c7d01e56800c3c64b26bb2fa0 Content-Type:text/plain Content-Transfer-Encoding:binary X-Experience-API-Hash:495395e777cd98da653df9615d09c0fd6bb2f8d4788394cd53c56a3bfdcd848a here is a simple attachment --105423a5219f5a63362a375ba7a64a8f234da19c7d01e56800c3c64b26bb2fa0-- ````

Example with multiple references and duplicated attachments that succeeds:

passing example ````http POST /xapi/statements HTTP/1.1 X-Experience-API-Version: 1.0.3 Content-Type: multipart/mixed; boundary=105423a5219f5a63362a375ba7a64a8f234da19c7d01e56800c3c64b26bb2fa0 Authorization: Host: localhost:8080 Connection: close User-Agent: Paw/3.3.1 (Macintosh; OS X/12.3.1) GCDHTTPRequest Content-Length: 2081 --105423a5219f5a63362a375ba7a64a8f234da19c7d01e56800c3c64b26bb2fa0 Content-Type:application/json { "actor": { "mbox": "mailto:sample.agent@example.com", "name": "Sample Agent", "objectType": "Agent" }, "verb": { "id": "http://adlnet.gov/expapi/verbs/answered", "display": { "en-US": "answered" } }, "object": { "id": "http://www.example.com/tincan/activities/multipart", "objectType": "Activity", "definition": { "name": { "en-US": "Multi Part Activity" }, "description": { "en-US": "Multi Part Activity Description" } } }, "attachments": [ { "usageType": "http://example.com/attachment-usage/test", "display": { "en-US": "A test attachment" }, "description": { "en-US": "A test attachment (description)" }, "contentType": "text/plain; charset=ascii", "length": 27, "sha2": "495395e777cd98da653df9615d09c0fd6bb2f8d4788394cd53c56a3bfdcd848a" }, { "usageType": "http://example.com/attachment-usage/test", "display": { "en-US": "A test attachment" }, "description": { "en-US": "A test attachment (description)" }, "contentType": "text/plain; charset=ascii", "length": 27, "sha2": "495395e777cd98da653df9615d09c0fd6bb2f8d4788394cd53c56a3bfdcd848a" } ] } --105423a5219f5a63362a375ba7a64a8f234da19c7d01e56800c3c64b26bb2fa0 Content-Type:text/plain Content-Transfer-Encoding:binary X-Experience-API-Hash:495395e777cd98da653df9615d09c0fd6bb2f8d4788394cd53c56a3bfdcd848a here is a simple attachment --105423a5219f5a63362a375ba7a64a8f234da19c7d01e56800c3c64b26bb2fa0 Content-Type:text/plain Content-Transfer-Encoding:binary X-Experience-API-Hash:495395e777cd98da653df9615d09c0fd6bb2f8d4788394cd53c56a3bfdcd848a here is a simple attachment --105423a5219f5a63362a375ba7a64a8f234da19c7d01e56800c3c64b26bb2fa0-- ````

Additionally, passing to the impl is an open question:

So this leads to another question, which is passing them to the impl: Efficiency would dictate that it not do what xapipe does (denormalize/duplicate by copying attachments so they are 1-1 with attachment objects) and just pass them normalized/deduplicated to the impl. This would however likely require some code changes on the impl to handle it (since they have been getting denormalized attachments since their inception)

So the options for passing to the impl are:

  1. denormalize/duplicate them (no changes to current impls, less efficient)
  2. normalize/deduplicate them (some changes to current impls, more efficient)
  3. pass them as received (pushes handing to impl) - This scares me, because a client could duplicate some but not all attachments and still not run afoul of the spec

In its current state this PR takes option 2, but that's up for discussion!