Allow for attachment to be a single object in user data #783

Merged
floatingghost merged 2 commits from single-attachment into develop 2024-05-27 01:44:54 +00:00

single object or array is ok, anything else is a no go

single object or array is ok, anything else is a no go
floatingghost added 1 commit 2024-05-26 16:10:20 +00:00
Allow for attachment to be a single object in user data
Some checks failed
ci/woodpecker/push/build-amd64 Pipeline is pending
ci/woodpecker/push/build-arm64 Pipeline is pending
ci/woodpecker/push/docs Pipeline is pending
ci/woodpecker/push/lint Pipeline is pending
ci/woodpecker/push/test Pipeline is pending
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/build-arm64 unknown status
ci/woodpecker/pr/build-amd64 unknown status
ci/woodpecker/pr/docs unknown status
da67e69af5
Oneric reviewed 2024-05-26 18:00:00 +00:00
Oneric left a comment
Member

looks good; one suggestion

on a tangential note looking at the CI failures, it seems the 10s tolerance in the recently added backoff test isn't quite enough (it was one second off) and i suspect the two test creates new app with (default|custom) scopes failures were also timing related given it complained about something not showing within a second

looks good; one suggestion on a tangential note looking at the CI failures, it seems the 10s tolerance in the recently added backoff test isn't quite enough (it was one second off) and i suspect the two `test creates new app with (default|custom) scopes` failures were also timing related given it complained about something not showing within a second
@ -1550,3 +1554,4 @@
data
|> Map.get("attachment", [])
|> normalize_attachment()
|> Enum.filter(fn %{"type" => t} -> t == "PropertyValue" end)
Member

Just a though feel free to dismiss: in case an attachment list contains external objects minified to just their ID or something else entirely, it might be good to also change the filter to:

fn
  %{"type" => t} -> t == "PropertyValue"
  _ -> false
end

Then we’d only discard unsupported attachments rather than rejecting the entire actor. On the downside, dropped entries might take longer to spot increasing the time until a federation issue gets fixed; not sure how relevant though

Just a though feel free to dismiss: in case an attachment list contains external objects minified to just their ID or something else entirely, it might be good to also change the filter to: ```elixir fn %{"type" => t} -> t == "PropertyValue" _ -> false end ``` Then we’d only discard unsupported attachments rather than rejecting the entire actor. On the downside, dropped entries might take longer to spot increasing the time until a federation issue gets fixed; not sure how relevant though
floatingghost added 1 commit 2024-05-27 01:10:06 +00:00
Add extra test case for nonsense field, increase timeouts
Some checks failed
ci/woodpecker/push/build-amd64 Pipeline is pending
ci/woodpecker/push/build-arm64 Pipeline is pending
ci/woodpecker/push/docs Pipeline is pending
ci/woodpecker/push/lint Pipeline is pending
ci/woodpecker/push/test Pipeline is pending
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/build-arm64 unknown status
ci/woodpecker/pr/build-amd64 unknown status
ci/woodpecker/pr/docs unknown status
f15eded3e1
floatingghost merged commit 5bdef8c724 into develop 2024-05-27 01:44:54 +00:00
floatingghost deleted branch single-attachment 2024-05-27 01:44:54 +00:00
Sign in to join this conversation.
No description provided.