LRS-72 Handle deduplicated attachment multiparts

[milt]

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

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

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!

[kelvinqian00]

This assumes that attachments with different data will have different SHAs, right?

[milt]

Yeah, even if it isn't mathematically true 🙈 . Unfortunately there isn't a stable way to tie an attachment to a specific referencing object in a statement batch really, so there isn't much to be done about it.

[kelvinqian00]

In that case you may want to add that to the documentation.

[milt]

I think the remote, theoretical possibility of a hash collision is outside the scope of the docs. At any rate, the docs link to this PR 😄

[kelvinqian00]

Oh so hash collisions aren't that common with SHAs? From our past convos I got the impression that the SHA2 algorithm was busted and that collisions were the order of the day. 😆

[milt]

If it's like SHA-256 it's quite unlikely. I'm actually a little confused about the spec language here, it only says that:

A Learning Record Provider SHOULD use SHA-256, SHA-384, or SHA-512 to populate the "sha2" property.

And of course doesn't require the LRS to validate it, because how would it know the alg?

[milt]

In general, yes we like to assume that impossible things will happen, it's just that in this instance the spec limits us.