{"action":"created","issue":{"url":"https://api.github.com/repos/Metaswitch/sprout/issues/1111","labels_url":"https://api.github.com/repos/Metaswitch/sprout/issues/1111/labels{/name}","comments_url":"https://api.github.com/repos/Metaswitch/sprout/issues/1111/comments","events_url":"https://api.github.com/repos/Metaswitch/sprout/issues/1111/events","html_url":"https://github.com/Metaswitch/sprout/pull/1111","id":90521627,"number":1111,"title":"Reviewer Matt - Store the ACR in the AsChain","user":{"login":"bossmc","id":1073983,"avatar_url":"https://avatars.githubusercontent.com/u/1073983?v=3","gravatar_id":"","url":"https://api.github.com/users/bossmc","html_url":"https://github.com/bossmc","followers_url":"https://api.github.com/users/bossmc/followers","following_url":"https://api.github.com/users/bossmc/following{/other_user}","gists_url":"https://api.github.com/users/bossmc/gists{/gist_id}","starred_url":"https://api.github.com/users/bossmc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bossmc/subscriptions","organizations_url":"https://api.github.com/users/bossmc/orgs","repos_url":"https://api.github.com/users/bossmc/repos","events_url":"https://api.github.com/users/bossmc/events{/privacy}","received_events_url":"https://api.github.com/users/bossmc/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"mirw","id":2963879,"avatar_url":"https://avatars.githubusercontent.com/u/2963879?v=3","gravatar_id":"","url":"https://api.github.com/users/mirw","html_url":"https://github.com/mirw","followers_url":"https://api.github.com/users/mirw/followers","following_url":"https://api.github.com/users/mirw/following{/other_user}","gists_url":"https://api.github.com/users/mirw/gists{/gist_id}","starred_url":"https://api.github.com/users/mirw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mirw/subscriptions","organizations_url":"https://api.github.com/users/mirw/orgs","repos_url":"https://api.github.com/users/mirw/repos","events_url":"https://api.github.com/users/mirw/events{/privacy}","received_events_url":"https://api.github.com/users/mirw/received_events","type":"User","site_admin":false},"milestone":null,"comments":1,"created_at":"2015-06-23T23:57:19Z","updated_at":"2015-06-24T09:40:22Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/Metaswitch/sprout/pulls/1111","html_url":"https://github.com/Metaswitch/sprout/pull/1111","diff_url":"https://github.com/Metaswitch/sprout/pull/1111.diff","patch_url":"https://github.com/Metaswitch/sprout/pull/1111.patch"},"body":"Matt, this makes the `AsChain` (which is reference-counted) be the owner of the `ACR` for a single S-CSCF phase (`orig`, `orig-cdiv` or `term`).  We discussed the change (and @mike-evans also proposed a similar approach).  I've tested through the existing UTs which ensure that we're not leaking any ACRs and that we're producing ACRs in the correct places, I've not tested live.\r\n\r\nOne thing that's a bit of a shame is I can't see a way to stop someone just reading `_acr` when they should be calling `get_acr()` to check the `_as_chain` first, any ideas?"},"comment":{"url":"https://api.github.com/repos/Metaswitch/sprout/issues/comments/114807551","html_url":"https://github.com/Metaswitch/sprout/pull/1111#issuecomment-114807551","issue_url":"https://api.github.com/repos/Metaswitch/sprout/issues/1111","id":114807551,"user":{"login":"mirw","id":2963879,"avatar_url":"https://avatars.githubusercontent.com/u/2963879?v=3","gravatar_id":"","url":"https://api.github.com/users/mirw","html_url":"https://github.com/mirw","followers_url":"https://api.github.com/users/mirw/followers","following_url":"https://api.github.com/users/mirw/following{/other_user}","gists_url":"https://api.github.com/users/mirw/gists{/gist_id}","starred_url":"https://api.github.com/users/mirw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mirw/subscriptions","organizations_url":"https://api.github.com/users/mirw/orgs","repos_url":"https://api.github.com/users/mirw/repos","events_url":"https://api.github.com/users/mirw/events{/privacy}","received_events_url":"https://api.github.com/users/mirw/received_events","type":"User","site_admin":false},"created_at":"2015-06-24T09:40:22Z","updated_at":"2015-06-24T09:40:22Z","body":"Looks good.  On preventing access, you could create an inner class with a private `ACR*` member.  You can call `get_acr()` on that, but can't see its `_acr` field.  A bit heavyweight, but should be robust.\r\n\r\nAlternatively, is it worth renaming `_acr` (e.g. `_in_dialog_acr`) so that people see that it's unusual?\r\n\r\n(Should we actually have 2 `SproutletTsxs` - one for dialog-initiating and one for in-dialog?  That's definitely more work than we want to have here, though!)"}}