Board Review: Azure Communication Services [Common] (.NET, JS, Java, Python, Android, iOS)

[petrsvihlik]

Contacts and Timeline

• Responsible service team: ACS Dev Security (former part of the ACS Auth team)
• Main contacts: Dominik Messinger (@DominikMe), Petr Svihlik (@petrsvihlik)
• Expected code complete date: 2022-07-01
• Expected release date: 2022-07-31

About the Service

• Link to documentation introducing/describing the service: N/A - not a service client library
• Link to the service REST APIs: N/A - not a service client library
• Link to GitHub issue for previous review sessions, if applicable: N/A - not a service client library

About the client library

• Name of the client library: Azure.Communication.Common
• Languages for this review: C#, JS, Java (+ Android), Python, iOS

Artifacts required (per language)

Please read through “API Review” section here to understand how these artifacts are generated. It is critical that these artifacts are present and are in the right format. If not, the language architects cannot review them with the SDK Team’s API review tool.

.NET

• APIView Link no. 1: https://apiview.dev/Assemblies/Review/0d13d0a5ceaa494f8a920f01b59d206d?diffOnly=True&diffRevisionId=4d8ca4f70e6d42a98baf0ccce503a5af
• APIView Link no. 2: https://apiview.dev/Assemblies/Review/a6145ac78446422c92a07a1888baa2d9?diffRevisionId=f370ec95a9af450e9a35ee9ca4bcd170&doc=False&diffOnly=False&revisionId=510b0178283f45d5a8e4d4f7826492fd
• PR no. 1: [Azure.Communication.Common] Added support for rawId ⟷ CommunicationIdentifier conversion azure-sdk-for-net#28574
• PR no. 2: [Azure.Communication.Common] - Overridden operators for the CommunicationIdentifier azure-sdk-for-net#30075

Java

• APIView Link: https://apiview.dev/Assemblies/Review/c236ca96fddc4b00a242f80eb58a1a0d?diffRevisionId=f8cf7a20da7643ada4f46109b8ab6f96&doc=False&diffOnly=False&revisionId=1659814d80124d0b8348367fad4e4f7c
• PR: [CommunicationIdentifier] Added support for rawId ⟷ CommunicationIdentifier conversion azure-sdk-for-java#28997

Android

• APIView Link: https://apiview.dev/Assemblies/Review/1dad7ec250684647aaeb2b17c59ad7e3?diffRevisionId=8f155a7222354750af6bf288f569000d&doc=False&diffOnly=False&revisionId=3663a826c0514d25b56f7c939e281600
• PR: [CommunicationIdentifier] Added support for rawId ⟷ CommunicationIdentifier conversion azure-sdk-for-android#1116

Python

• APIView Link: https://apiview.dev/Assemblies/Review/fce877aea4a4400391f55e985f1d92b4/1b734e3549974121b9705a8f8bdb4ac6?diffRevisionId=ad28b2d974a747acb32f28539a10a2a6&doc=False&diffOnly=True
• PR: [communication-identity] Add methods to translate CommunicationIdentifier to / from a stable string format azure-sdk-for-python#24507

TypeScript

• APIView Link: https://apiview.dev/Assemblies/Review/e921ff42f06b4844ac55ff0787bf2535?diffOnly=True&diffRevisionId=5d31b068c24d48f58296cc015e1183fe
• PR: [communication-common] Communication identifier to raw id translation azure-sdk-for-js#21699

iOS

• APIView Link: https://apiview.dev/Assemblies/Review/86f76588c938454b9f7a8b27bfbe683a?diffRevisionId=f05f1a59368c4cc1a4fd358be57215e7&doc=False&diffOnly=False&revisionId=d03da9637ba24201b53ab504138b3b6a
• PR: [Feature] Introduce new method createCommunicationIdentifier(from rawId:) azure-sdk-for-ios#1312
• Considered approaches:
  • Approach 1:
    • https://gist.github.com/JoshuaLai/9808644195f0d258ab6711c3b3a273bf
  • Approach 2:
    • https://gist.github.com/JoshuaLai/cb229389308bb2f0fa90ebc97e9f60fe
  • Approach 3:
    • https://gist.github.com/JoshuaLai/8922535a3dac9a2a7c09c6218c08880e

About the change

Motivation

While the CommunicationIdentifier types for both SDK runtime models as well as REST models do a good job of

• providing good auto-complete in IDEs
• avoid having customers to parse and construct MRIs with cryptic strings like 4: or 8:orgid:
• allow switch-case by type
• allow restricting communication to specific types

they don't do a good job of

• explaining how to persist them in Databases and use them as keys
• how to use them as key in dictionaries, example use case messages or read notifications by user
• how to use them as key in declarative UI frameworks such as React to avoid unnecessary re-renders
• use them as key in REST API paths, instead of having to rely on POST payloads leading to less-intuitive REST CRUD APIs

Customers can serialize identifiers themselves but it's not clear how to do so in a safe manner. For example, the MicrosoftTeamsUserIdentifier has multiple properties such as IsAnonymous or Cloud and it's not clear in which order these properties should get serialized into a string. ACS can introduce new properties in the future which can potentially break any serialization logic that the customer came up with.

One particular scenario (submitted by the UI Library team) that this feature will unblock is mapping MRIs to AAD ObjectIds to enable calling Graph API and provide a rich experience for calling participants.

Proposed change

Introduce a canonical serialization format of identifiers that can be used for all of the above cases.
Implementation details - Design change request: RawId support for (de)serialization

Champion Scenario

        public class ContosoUser
        {
            public string Name { get; set; }
            public string Email { get; set; }
            public string CommunicationIdentifier { get; set; }
        }

        public void SerializationExample()
        {
            // The value can come from the Identity API, Calling/Chat API (participants), etc.
            // It can be a CommunicationUser, PhoneNumber, MicrosoftTeamsUser
            CommunicationIdentifier communicationIdentifier;

            ContosoUser user = new ContosoUser()
            {
                Name = "John",
                Email = "john@doe.com",
                // store the MRI no matter what's the underlying type
                CommunicationIdentifier = communicationIdentifier.RawId
            };
            SaveToDb(user);
        }

        public void DeserializationExample()
        {
            ContosoUser user = GetFromDb("john@doe.com");

            string rawId = user.CommunicationIdentifier;
            CommunicationIdentifier communicationIdentifier = CommunicationIdentifier.FromRawId(rawId);
            switch (communicationIdentifier)
            {
                case MicrosoftTeamsUserIdentifier teamsUser:
                    string getPhotoUri = $"https://graph.microsoft.com/v1.0/users/{teamsUser.UserId}/photo/$value";
                    // GET /users/<oid>/photo/$value
                    break;

                case CommunicationIdentifier communicationUser:
                    // ...
                    break;
            }
        }

[kyle-patterson]

Scheduled for 7/28 9a-11a PDT

[petrsvihlik]

Should we use implicit or explicit operator? The guidelines say that implicit shouldn't be used for something that can throw.

We already use implicit for enum-like structures (e.g. CommunicationCloudEnvironment) - are there any guidelines for non-enum-like structures?

Would be cool to be able to do something like this:

            ContosoUser user = GetFromDb("john@doe.com");
            var chatParticipant = new ChatParticipant(identifier: user.RawId)
            {
                DisplayName = "UserDisplayName"
            };

Instead of this:

            var chatParticipant = new ChatParticipant(identifier: new CommunicationUserIdentifier(id: "<Access_ID>"))
            {
                DisplayName = "UserDisplayName"
            };

[tg-msft]

Recording (MS INTERNAL ONLY)

[petrsvihlik]

@joheredi @xirzec can you pls approve the APIView for JS?

It was approved last Thursday, and we'd like to get it released soon to unblock other teams.

[xirzec]

Looks like @bterlson approved, @petrsvihlik let me know if you're still blocked. 😄

[petrsvihlik]

@xirzec for some reason the pipeline doesn't accept the approval above and requires an approval in this API view: https://apiview.dev/Assemblies/Review/26aa3f641ebb441cb5a8f19d8291b97a?diffRevisionId=5d31b068c24d48f58296cc015e1183fe&doc=False&diffOnly=False&revisionId=af44898499ea429fad73cc5cd29804ed

can you please approve it too?

[joheredi]

I have approved it

[petrsvihlik]

all languages released