SDL0238 'Keyboard Enhancements/r4': apply code review comments, add a couple of additional tests to cover custom keys trim logic

2 files changed, 38 insertions, 16 deletions

diff --git a/SmartDeviceLink/private/SDLWindowCapability+ScreenManagerExtensions.m b/SmartDeviceLink/private/SDLWindowCapability+ScreenManagerExtensions.m
index 0006daa4c..a17dac200 100644
--- a/SmartDeviceLink/private/SDLWindowCapability+ScreenManagerExtensions.m
+++ b/SmartDeviceLink/private/SDLWindowCapability+ScreenManagerExtensions.m
@@ -56,33 +56,35 @@
     return NO;
 }
 
-- (nullable SDLKeyboardProperties *)createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFromConfiguration:(nullable SDLKeyboardProperties *)inKeyboardProperties {
+- (nullable SDLKeyboardProperties *)createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFromConfiguration:(nullable SDLKeyboardProperties *)keyboardConfiguration {
     // If there are no keyboard capabilities, if there is no passed keyboard configuration, or if there is no layout to the passed keyboard configuration, just pass back the passed in configuration
-    if ((self.keyboardCapabilities == nil) || (inKeyboardProperties == nil) || (inKeyboardProperties.keyboardLayout == nil)) {
-        return inKeyboardProperties;
+    if ((self.keyboardCapabilities == nil) || (keyboardConfiguration == nil) || (keyboardConfiguration.keyboardLayout == nil)) {
+        return keyboardConfiguration;
     }
 
+    // Find the capability for the keyboard configuration's layout
     SDLKeyboardLayoutCapability *selectedLayoutCapability = nil;
     for (SDLKeyboardLayoutCapability *layoutCapability in self.keyboardCapabilities.supportedKeyboards) {
-        if ([layoutCapability.keyboardLayout isEqualToEnum:inKeyboardProperties.keyboardLayout]) {
+        if ([layoutCapability.keyboardLayout isEqualToEnum:keyboardConfiguration.keyboardLayout]) {
             selectedLayoutCapability = layoutCapability;
             break;
         }
     }
     if (selectedLayoutCapability == nil) {
-        SDLLogE(@"Configured keyboard layout is not supported: %@", inKeyboardProperties.keyboardLayout);
+        SDLLogE(@"Configured keyboard layout is not supported: %@", keyboardConfiguration.keyboardLayout);
         return nil;
     }
 
-    SDLKeyboardProperties *modifiedKeyboardConfiguration = [inKeyboardProperties copy];
-    if (inKeyboardProperties.customKeys.count == 0) {
+    // Modify the keyboard configuration if there are fewer customKeys allowed than were set, or if an empty array was set.
+    SDLKeyboardProperties *modifiedKeyboardConfiguration = [keyboardConfiguration copy];
+    if (keyboardConfiguration.customKeys.count == 0) {
         modifiedKeyboardConfiguration.customKeys = nil;
     } else {
         // If there are more custom keys than are allowed for the selected keyboard layout, we need to trim the number of keys to only use the first n number of custom keys, where n is the number of allowed custom keys for that layout.
         NSUInteger numConfigurableKeys = selectedLayoutCapability.numConfigurableKeys.unsignedIntegerValue;
         if (modifiedKeyboardConfiguration.customKeys.count > numConfigurableKeys) {
-            modifiedKeyboardConfiguration.customKeys = [modifiedKeyboardConfiguration.customKeys subarrayWithRange:NSMakeRange(0, numConfigurableKeys)];
             SDLLogW(@"%lu custom keys set, but the selected layout: %@ only supports %lu. Dropping the rest.", (unsigned long)modifiedKeyboardConfiguration.customKeys.count, modifiedKeyboardConfiguration.keyboardLayout, (unsigned long)numConfigurableKeys);
+            modifiedKeyboardConfiguration.customKeys = [modifiedKeyboardConfiguration.customKeys subarrayWithRange:NSMakeRange(0, numConfigurableKeys)];
         }
     }
 
diff --git a/SmartDeviceLinkTests/RPCSpecs/StructSpecs/SDLWindowCapabilitySpec.m b/SmartDeviceLinkTests/RPCSpecs/StructSpecs/SDLWindowCapabilitySpec.m
index e846fd1be..ee5d34ef5 100644
--- a/SmartDeviceLinkTests/RPCSpecs/StructSpecs/SDLWindowCapabilitySpec.m
+++ b/SmartDeviceLinkTests/RPCSpecs/StructSpecs/SDLWindowCapabilitySpec.m
@@ -185,21 +185,21 @@ describe(@"getter/setter tests", ^{
     });
 });
 
-describe(@"method createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFromConfiguration:", ^{
-    const UInt8 numConfigurableKeys = 7;
+describe(@"creating a valid keyboard configuration based on keyboard capabilities", ^{
+    UInt8 numConfigurableKeys = 7;
 
     beforeEach(^{
         testStruct = [[SDLWindowCapability alloc] init];
         testStruct.keyboardCapabilities = nil;
     });
 
-    context(@"when .keyboardCapabilities is nil or empty", ^{
+    context(@"when keyboardCapabilities is nil or empty", ^{
         it(@"result should be nil when the argument is nil", ^{
             SDLKeyboardProperties *resultProperties = [testStruct createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFromConfiguration:nil];
             expect(resultProperties).to(beNil());
         });
 
-        it(@"result should be equal to the argument when .keyboardLayout is nil", ^{
+        it(@"result should be equal to the argument when keyboardLayout is nil", ^{
             SDLKeyboardProperties *testKeyboardProperties = [[SDLKeyboardProperties alloc] init];
             testKeyboardProperties.maskInputCharacters = SDLKeyboardInputMaskUserChoiceInputKeyMask;
             SDLKeyboardProperties *resultProperties = [testStruct createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFromConfiguration:testKeyboardProperties];
@@ -208,7 +208,7 @@ describe(@"method createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFro
             expect(resultProperties.maskInputCharacters).to(equal(SDLKeyboardInputMaskUserChoiceInputKeyMask));
         });
 
-        it(@"result should be nil when the argument is not nil and .keyboardCapabilities is empty", ^{
+        it(@"result should be nil when the argument is not nil and keyboardCapabilities is empty", ^{
             testStruct.keyboardCapabilities = [[SDLKeyboardCapabilities alloc] init];
             SDLKeyboardProperties *testKeyboardProperties = [[SDLKeyboardProperties alloc] init];
             testKeyboardProperties.keyboardLayout = SDLKeyboardLayoutNumeric;
@@ -217,11 +217,14 @@ describe(@"method createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFro
             expect(resultProperties).to(beNil());
         });
 
-        context(@"when .keyboardCapabilities is not empty", ^{
+        context(@"when keyboardCapabilities is not empty", ^{
             __block SDLKeyboardProperties *testKeyboardProperties = nil;
             __block SDLKeyboardCapabilities *keyboardCapabilities = nil;
+            __block NSArray *testCustomKeysLong = nil;
 
             beforeEach(^{
+                // create a long array that contains more custom keys than <numConfigurableKeys>
+                testCustomKeysLong = [@"a ä æ b c d e ê f j h i j k l m n o p q r s ß t u v w x y z" componentsSeparatedByString:@" "];
                 NSArray *arrayLayouts = @[SDLKeyboardLayoutQWERTY, SDLKeyboardLayoutQWERTZ, SDLKeyboardLayoutAZERTY, SDLKeyboardLayoutNumeric];
                 NSMutableArray *arrayLayoutCapability = [[NSMutableArray alloc] initWithCapacity:arrayLayouts.count];
                 for (SDLKeyboardLayout layout in arrayLayouts) {
@@ -235,7 +238,7 @@ describe(@"method createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFro
                 testKeyboardProperties = [[SDLKeyboardProperties alloc] init];
                 testKeyboardProperties.keyboardLayout = SDLKeyboardLayoutNumeric;
                 // create custom keys array longer than <numConfigurableKeys>
-                testKeyboardProperties.customKeys = [@"a ä æ b c d e ê f j h i j k l m n o p q r s ß t u v w x y z" componentsSeparatedByString:@" "];
+                testKeyboardProperties.customKeys = testCustomKeysLong;
                 testKeyboardProperties.maskInputCharacters = SDLKeyboardInputMaskUserChoiceInputKeyMask;
                 testKeyboardProperties.keyboardLayout = SDLKeyboardLayoutAZERTY;
             });
@@ -257,9 +260,26 @@ describe(@"method createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFro
                 expect(resultProperties.customKeys.count).to(equal(numConfigurableKeys));
                 expect(resultProperties.maskInputCharacters).to(beNil());
             });
+
+            it(@"customKeys should be trimmed to contain <numConfigurableKeys> items", ^{
+                NSArray *expectedCustomKeys = [testCustomKeysLong subarrayWithRange:NSMakeRange(0, numConfigurableKeys)];
+                testKeyboardProperties.customKeys = testCustomKeysLong;
+                SDLKeyboardProperties *resultProperties = [testStruct createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFromConfiguration:testKeyboardProperties];
+                expect(resultProperties).notTo(beNil());
+                expect(resultProperties.customKeys.count).to(equal(numConfigurableKeys));
+                expect(resultProperties.customKeys).to(equal(expectedCustomKeys));
+            });
+
+            it(@"customKeys should not be trimmed and should be equal to the initial array", ^{
+                NSArray *testCustomKeys = [testCustomKeysLong subarrayWithRange:NSMakeRange(0, numConfigurableKeys)];
+                testKeyboardProperties.customKeys = testCustomKeys;
+                SDLKeyboardProperties *resultProperties = [testStruct createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFromConfiguration:testKeyboardProperties];
+                expect(resultProperties).notTo(beNil());
+                expect(resultProperties.customKeys.count).to(equal(numConfigurableKeys));
+                expect(resultProperties.customKeys).to(equal(testCustomKeys));
+            });
         });
     });
 });
 
-
 QuickSpecEnd