Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CommunicationIdentifier] Introduce new rawId and creation of identifer #1255

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,6 @@ class IdentifierSerializerTests: XCTestCase {
try serializePhoneNumber(expectedId: testRawId)
}

func test_SerializePhoneNumber_ExpectedIdNil() throws {
try serializePhoneNumber(expectedId: nil)
}

func serializePhoneNumber(expectedId: String?) throws {
let model = try IdentifierSerializer
.serialize(identifier: PhoneNumberIdentifier(phoneNumber: testPhoneNumber, rawId: expectedId))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
882F28D425A632CA009689E3 /* CommunicationTokenRefreshOptions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 882F28D325A632CA009689E3 /* CommunicationTokenRefreshOptions.swift */; };
8856CEAC253A376D00044559 /* CommunicationAccessToken.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8856CEAB253A376D00044559 /* CommunicationAccessToken.swift */; };
8856CEE5253E3AEF00044559 /* ObjCCommunicationTokenCredentialTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 8856CEE4253E3AEF00044559 /* ObjCCommunicationTokenCredentialTests.m */; };
8889890E28245398002C5C82 /* CommunicationIdentifierHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8889890D28245398002C5C82 /* CommunicationIdentifierHelper.swift */; };
8889891028246763002C5C82 /* CommunicationIdentifierHelperTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8889890F28246763002C5C82 /* CommunicationIdentifierHelperTests.swift */; };
88CC8C35254750260028977C /* ObjCCommunicationTokenCredentialAsyncTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 88CC8C34254750260028977C /* ObjCCommunicationTokenCredentialAsyncTests.m */; };
88F1F570254A07BC00876BC4 /* ObjCTokenParserTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 88F1F56F254A07BC00876BC4 /* ObjCTokenParserTests.m */; };
C1ABE73016C6539A7175199B /* Pods_AzureCommunicationCommon.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = D532B0F56C67A1E89941A6D6 /* Pods_AzureCommunicationCommon.framework */; };
Expand Down Expand Up @@ -60,6 +62,8 @@
8856CEAB253A376D00044559 /* CommunicationAccessToken.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CommunicationAccessToken.swift; sourceTree = "<group>"; };
8856CEE3253E3AEF00044559 /* AzureCommunicationCommonTests-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "AzureCommunicationCommonTests-Bridging-Header.h"; sourceTree = "<group>"; };
8856CEE4253E3AEF00044559 /* ObjCCommunicationTokenCredentialTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ObjCCommunicationTokenCredentialTests.m; sourceTree = "<group>"; };
8889890D28245398002C5C82 /* CommunicationIdentifierHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CommunicationIdentifierHelper.swift; sourceTree = "<group>"; };
8889890F28246763002C5C82 /* CommunicationIdentifierHelperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CommunicationIdentifierHelperTests.swift; sourceTree = "<group>"; };
88CC8C34254750260028977C /* ObjCCommunicationTokenCredentialAsyncTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ObjCCommunicationTokenCredentialAsyncTests.m; sourceTree = "<group>"; };
88F1F56F254A07BC00876BC4 /* ObjCTokenParserTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ObjCTokenParserTests.m; sourceTree = "<group>"; };
CC958AD71D0F5F2AAAF8CD78 /* Pods-AzureCommunicationCommon.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-AzureCommunicationCommon.debug.xcconfig"; path = "Target Support Files/Pods-AzureCommunicationCommon/Pods-AzureCommunicationCommon.debug.xcconfig"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -125,6 +129,7 @@
1D2E7F6E24E4536500447964 /* Authentication */,
1DC3550524D9F02D0095ABD9 /* Supporting Files */,
1D2E7F6F24E4589100447964 /* Identifiers.swift */,
8889890D28245398002C5C82 /* CommunicationIdentifierHelper.swift */,
F1540EBC25BF893C0056B087 /* CommunicationCloudEnvironment.swift */,
);
path = Source;
Expand All @@ -140,6 +145,7 @@
8856CEE3253E3AEF00044559 /* AzureCommunicationCommonTests-Bridging-Header.h */,
88F1F56F254A07BC00876BC4 /* ObjCTokenParserTests.m */,
F1540EC025BFD6910056B087 /* CommunicationIdentifierTest.swift */,
8889890F28246763002C5C82 /* CommunicationIdentifierHelperTests.swift */,
);
path = Tests;
sourceTree = "<group>";
Expand Down Expand Up @@ -362,6 +368,7 @@
F183A5EB24AF9D9000F0E0D5 /* CommunicationTokenCredentialProviding.swift in Sources */,
1D2E7F7024E4589100447964 /* Identifiers.swift in Sources */,
8856CEAC253A376D00044559 /* CommunicationAccessToken.swift in Sources */,
8889890E28245398002C5C82 /* CommunicationIdentifierHelper.swift in Sources */,
1DE4DB7924C1063E00631921 /* ThreadSafeRefreshableAccessTokenCache.swift in Sources */,
1DE4DB7724C0FE8300631921 /* AutoRefreshTokenCredential.swift in Sources */,
F1540EBD25BF893C0056B087 /* CommunicationCloudEnvironment.swift in Sources */,
Expand All @@ -380,6 +387,7 @@
88F1F570254A07BC00876BC4 /* ObjCTokenParserTests.m in Sources */,
8856CEE5253E3AEF00044559 /* ObjCCommunicationTokenCredentialTests.m in Sources */,
F1540EC125BFD6910056B087 /* CommunicationIdentifierTest.swift in Sources */,
8889891028246763002C5C82 /* CommunicationIdentifierHelperTests.swift in Sources */,
1D07222D24C8FB0F00C2EF4E /* CommunicationTokenCredentialTests.swift in Sources */,
88CC8C35254750260028977C /* ObjCCommunicationTokenCredentialAsyncTests.m in Sources */,
);
Expand Down
8 changes: 8 additions & 0 deletions sdk/communication/AzureCommunicationCommon/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Release History

## 1.1.0 (upcoming)
### New Features
- Introduce new `CommunicationIdentifierHelper` to create `CommunicationIdentifier` based on rawIds

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- Introduce new `rawId` getter property in the `CommunicationIdentifier` protocol

### Key Bug Fixes
- `PhoneNumberIdentifier` no longer allows the `rawId` to not correspond with the phone number

## 1.0.3 (2022-03-10)
### New Features
- Update `AzureCore` dependency to beta 15.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// --------------------------------------------------------------------------
//
// Copyright (c) Microsoft Corporation. All rights reserved.
//
// The MIT License (MIT)
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the ""Software""), to
// deal in the Software without restriction, including without limitation the
// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
// sell copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED *AS IS*, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
// IN THE SOFTWARE.
//
// --------------------------------------------------------------------------

import Foundation
/**
Helper class to easily create CommunicationIdentifiers
*/
@objcMembers
public class CommunicationIdentifierHelper: NSObject {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm never a fan of anything that ends in Helper, Util or Manager :-).

@tjprescott Any suggestions for good Swift-idiomatic naming?

Ideally we would have liked a static factory method on CommunicationIdentifier itself but can't do it because it's a protocol. Another alternative is a free function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not add a required init to the protocol that takes a rawId and then provide the contents of createCommunicationIdentifier(from:) as a default implementation via extension?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I agree you don't really want this helper thingy as part of the public API.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was something I was trying. I initially was thinking something like this

@objc public protocol CommunicationIdentifier: NSObjectProtocol {
    @objc var rawId: String { get }
    static func createCommunicationIdentifier(from rawId: String) -> CommunicationIdentifier
}
extension CommunicationIdentifier {
    public static func createCommunicationIdentifier(from rawId: String) -> CommunicationIdentifier {
        
    }
}

But because the CommunicationIdentifier needed to be exposed to Obj-C I can't create a function inside my extension nicely.
Otherwise I would be forcing the identifiers to all require this method and every method would do the exact same thing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tjprescott @DominikMe Had a discussion about this implementation. I think in terms of functionality this approah that we have with the helper most closely aligns with the other platforms.
That being said, the only reason we are doing this is because of obj-c support. I do think we should slowly start exploring having 2 implementations. One design to favour swift, and this implementation to favour obj-c. We can have in the documentation to guide the developers which approach they should use.

private static let phoneNumberPrefix = "4:"
private static let teamUserAnonymousPrefix = "8:teamsvisitor:"
private static let teamUserPublicCloudPrefix = "8:orgid:"
private static let teamUserDODCloudPrefix = "8:dod:"
private static let teamUserGCCHCloudPrefix = "8:gcch:"
private static let acsUser = "8:acs:"
private static let spoolUser = "8:spool:"
private static let dodAcsUser = "8:dod-acs:"
private static let gcchAcsUser = "8:gcch-acs:"
/**
Creates a CommunicationIdentifier from a given rawId. When storing rawIds use this function to restore the identifier that was encoded in the rawId.
Parameters: rawId The rawId to be translated to its identifier representation.
Returns: Type safe CommunicationIdentifier created. Use the `isKind(of:)` to verify identifier type
SeeAlso: ` CommunicationIdentifier`
*/
public static func createCommunicationIdentifier(from rawId: String) -> CommunicationIdentifier {
if rawId.hasPrefix(phoneNumberPrefix) {
let phoneNumber = "+" + rawId.dropFirst(phoneNumberPrefix.count)
return PhoneNumberIdentifier(phoneNumber: phoneNumber, rawId: rawId)
}
let segments = rawId.split(separator: ":")
if segments.count < 3 {
return UnknownIdentifier(rawId)
}
let scope = segments[0] + ":" + segments[1] + ":"
let suffix = String(rawId.dropFirst(scope.count))
switch scope {
case teamUserAnonymousPrefix:
return MicrosoftTeamsUserIdentifier(userId: suffix, isAnonymous: true)
case teamUserPublicCloudPrefix:
return MicrosoftTeamsUserIdentifier(
userId: suffix,
isAnonymous: false,
rawId: rawId,
cloudEnvironment: .Public
)
case teamUserDODCloudPrefix:
return MicrosoftTeamsUserIdentifier(
userId: suffix,
isAnonymous: false,
rawId: rawId,
cloudEnvironment: .Dod
)
case teamUserGCCHCloudPrefix:
return MicrosoftTeamsUserIdentifier(
userId: suffix,
isAnonymous: false,
rawId: rawId,
cloudEnvironment: .Gcch
)
case acsUser, spoolUser, dodAcsUser, gcchAcsUser:
return CommunicationUserIdentifier(rawId)
default:
return UnknownIdentifier(rawId)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,15 @@ import Foundation
Common Communication Identifier protocol for all Azure Communication Services.
All Communication Identifiers conform to this protocol.
*/
@objc public protocol CommunicationIdentifier: NSObjectProtocol {}
@objc public protocol CommunicationIdentifier: NSObjectProtocol {
var rawId: String { get }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to require:
init?(from rawId: String)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another suggestion that was brought up by @tjprescott that I really like is including a new enum or string to represent the kind. @DominikMe I know we have a similar concept in the TS library.

}

/**
Communication identifier for Communication Services Users
*/
@objcMembers public class CommunicationUserIdentifier: NSObject, CommunicationIdentifier {
public var rawId: String { return identifier }
public let identifier: String
/**
Creates a CommunicationUserIdentifier object
Expand All @@ -50,6 +54,7 @@ import Foundation
Catch-all for all other Communication identifiers for Communication Services
*/
@objcMembers public class UnknownIdentifier: NSObject, CommunicationIdentifier {
public var rawId: String { return identifier }
public let identifier: String
/**
Creates a UnknownIdentifier object
Expand All @@ -66,7 +71,7 @@ import Foundation
*/
@objcMembers public class PhoneNumberIdentifier: NSObject, CommunicationIdentifier {
public let phoneNumber: String
public let rawId: String?
public private(set) var rawId: String

/**
Creates a PhoneNumberIdentifier object
Expand All @@ -75,17 +80,36 @@ import Foundation
*/
public init(phoneNumber: String, rawId: String? = nil) {
self.phoneNumber = phoneNumber
self.rawId = rawId
if let rawId = rawId {
self.rawId = rawId
} else {
let strippedPhoneNumber = phoneNumber.replacingOccurrences(of: "+", with: "")
self.rawId = "4:" + strippedPhoneNumber
}
JoshuaLai marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Returns a Boolean value indicating whether two values are equal.
Note: In Objective-C favor isEqual() method
- Parameter lhs PhoneNumberIdentifier to compare.
- Parameter rhs Another PhoneNumberIdentifier to compare.
*/
// swiftlint:disable nsobject_prefer_isequal
public static func == (lhs: PhoneNumberIdentifier, rhs: PhoneNumberIdentifier) -> Bool {
if lhs.phoneNumber != rhs.phoneNumber {
return lhs.rawId == rhs.rawId
JoshuaLai marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Returns a Boolean value that indicates whether the receiver is equal to another given object.
This will automatically return false if object being compared to is not a PhoneNumberIdentifier.
- Parameter object The object with which to compare the receiver.
*/
override public func isEqual(_ object: Any?) -> Bool {
guard let object = object as? PhoneNumberIdentifier else {
return false
}

return lhs.rawId == nil
|| rhs.rawId == nil
|| lhs.rawId == rhs.rawId
return rawId == object.rawId
}
}

Expand All @@ -95,7 +119,7 @@ import Foundation
@objcMembers public class MicrosoftTeamsUserIdentifier: NSObject, CommunicationIdentifier {
public let userId: String
public let isAnonymous: Bool
public let rawId: String?
public private(set) var rawId: String
public let cloudEnviroment: CommunicationCloudEnvironment

/**
Expand All @@ -114,31 +138,62 @@ import Foundation
rawId: String? = nil,
cloudEnvironment: CommunicationCloudEnvironment = .Public
) {
self.rawId = rawId
JoshuaLai marked this conversation as resolved.
Show resolved Hide resolved
self.userId = userId
self.isAnonymous = isAnonymous
self.cloudEnviroment = cloudEnvironment

if let rawId = rawId {
self.rawId = rawId
} else {
if isAnonymous {
self.rawId = "8:teamsvisitor:" + userId
} else {
switch cloudEnvironment {
case .Dod:
self.rawId = "8:dod:" + userId
case .Gcch:
self.rawId = "8:gcch:" + userId
case .Public:
self.rawId = "8:orgid:" + userId
default:
self.rawId = "8:orgid:" + userId
}
}
}
}

public init(userId: String, isAnonymous: Bool) {
self.cloudEnviroment = .Public
self.rawId = nil
self.userId = userId
self.isAnonymous = isAnonymous
/**
Creates a MicrosoftTeamsUserIdentifier object. cloudEnvironment is defaulted to Public cloud.
- Parameter userId: Id of the Microsoft Teams user. If the user isn't anonymous,
the id is the AAD object id of the user.
- Parameter isAnonymous: Set this to true if the user is anonymous:
for example when joining a meeting with a share link.
*/
convenience init(userId: String, isAnonymous: Bool) {
self.init(userId: userId, isAnonymous: isAnonymous, rawId: nil, cloudEnvironment: .Public)
}

/**
Returns a Boolean value indicating whether two values are equal.
Note: In Objective-C favor isEqual() method
- Parameter lhs MicrosoftTeamsUserIdentifier to compare.
- Parameter rhs Another MicrosoftTeamsUserIdentifier to compare.
*/
// swiftlint:disable nsobject_prefer_isequal
public static func == (lhs: MicrosoftTeamsUserIdentifier, rhs: MicrosoftTeamsUserIdentifier) -> Bool {
if lhs.userId != rhs.userId ||
lhs.isAnonymous != rhs.isAnonymous {
return false
}
return lhs.rawId == rhs.rawId
}

if lhs.cloudEnviroment != rhs.cloudEnviroment {
/**
Returns a Boolean value that indicates whether the receiver is equal to another given object.
This will automatically return false if object being compared to is not a MicrosoftTeamsUserIdentifier.
- Parameter object The object with which to compare the receiver.
*/
override public func isEqual(_ object: Any?) -> Bool {
guard let object = object as? MicrosoftTeamsUserIdentifier else {
return false
}

return lhs.rawId == nil
|| rhs.rawId == nil
|| lhs.rawId == rhs.rawId
return rawId == object.rawId
}
}
Loading