go-fed / activity

ActivityStreams & ActivityPub in golang, oh my!
BSD 3-Clause "New" or "Revised" License
711 stars 111 forks source link

Non-functional properties multiple "Append" => 1 element #124

Closed cjslep closed 4 years ago

cjslep commented 4 years ago

Ben indicated to me that the "non-functional" (still a stupid name for "multiple-values") attachment property wasn't actually appending more than 1 value: https://git.lubar.me/lubar-local/ravenvale/src/branch/master/fedi.go#L793

Must investigate more.

cjslep commented 4 years ago

809fd1f0414affef24779128dbaedf2ddec339e9 adds failing unit tests for this use case.

cjslep commented 4 years ago

I finally had a sec to look at this. I identified root cause. If I don't ignore the error:

err := attachmentProp.AppendType(&testContextWrapper{firstObject})
if err != nil {
  panic(err)
}

I get the error:

illegal type to set on ActivityStreamsAttachment property: *streams.testContextWrapper

So it's because the testContextWrapper isn't implementing an expected interface, like vocab.ActivityStreamsObject.

The second object in the test is able to be set correctly.

cjslep commented 4 years ago

There is a fix in 84ee3614781ac73ebdf1be5a2991fd2eac1e29e0:

Change:

type testContextWrapper struct {
  vocab.Type
}

to:

type testContextWrapper struct {
  vocab.ActivityStreamsObject
}

Note that in this case, only serialization is supported. Go-fed cannot deserialize things into concrete types that haven't been explicitly code-generated for. For proper serialization and deserialization support, ontological types should have requests added to https://github.com/go-fed/activity/issues/121