KeyPackage -> Conversation Internal API

[stephen-smith]

https://wearezeta.atlassian.net/browse/FS-532 (partial, brig changes)

New endpoints are not in use, but brig will need to be deployed before galley, which I think is the status quo.

Checklist

• The PR Title explains the impact of the change.
• The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
• changelog.d contains the following bits of information (details):
  • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.
  • If internal end-points have been added or changed: which services have to be deployed in a specific order?

[pcapriotti]

Looks good, but it's not entirely clear to me if this is the API that we'll need. For example, when dereferencing a recipient of a welcome message, it might be useful to fetch both the user and the corresponding conversation at the same time, instead of making two calls to brig.

But this is ok for now, we can always add more endpoints later.

[pcapriotti]

Isn't this test basically contained in testKeyPackageClaim in API.MLS?

[stephen-smith]

This isn't a test itself, it's a helper function to get a valid, claimed ref.

[pcapriotti]

Suggested change

	kpcPut :: HasCallStack => Brig -> KeyPackageRef -> Qualified ConvId -> Http ()
	kpcPut brig ref qConv = do
	resp <-
	put
	( brig
	. paths ["i", "mls", "key-packages", toByteString' $ toUrlPiece ref, "conversation"]
	. contentJson
	. json qConv
	)
	liftIO $ assertEqual "PUT i/mls/key-packages/:ref/conversation failed" 204 (statusCode resp)
	kpcPut brig ref qConv =
	put
	( brig
	. paths ["i", "mls", "key-packages", toByteString' $ toUrlPiece ref, "conversation"]
	. contentJson
	. json qConv
	)
	!!! const 204 === statusCode

[stephen-smith]

I think I prefer the custom message, since this failure might occur inside 3-4 different tests.

[pcapriotti]

This is probably ok, but we might also want to consider the more simple-minded approach of checking existence first, then updating, as we do in various other places.

[mdimjasevic]

For example, when dereferencing a recipient of a welcome message, it might be useful to fetch both the user and the corresponding conversation at the same time, instead of making two calls to brig.

I had this in PR #2368 (commit 320c309), but later I realised I don't need it for that PR and reverted the commit. @stephen-smith , feel free to adopt it as part of your PR.

[stephen-smith]

For example, when dereferencing a recipient of a welcome message, it might be useful to fetch both the user and the corresponding conversation at the same time, instead of making two calls to brig.

I had this in PR #2368 (commit 320c309), but later I realised I don't need it for that PR and reverted the commit. @stephen-smith , feel free to adopt it as part of your PR.

I didn't want to change the public API (and disrupt other teams) for this intra-service communication feature, since it's still on the horizon to merge galley and brig, but I'll look into it. One query to grab the whole row is going to be more efficient for the computers, certainly.

[stephen-smith]

For example, when dereferencing a recipient of a welcome message, it might be useful to fetch both the user and the corresponding conversation at the same time, instead of making two calls to brig.

I had this in PR #2368 (commit 320c309)

I didn't want to change the public API

Hmm, that's actually in the Internal API. Going to merge as-is for now, but since both those queries are internal I might merge them as part of the next round of changes.