From 2776bd941b164f22732367a2e77c230e07379417 Mon Sep 17 00:00:00 2001 From: Joel Fischer Date: Wed, 24 Jul 2019 15:18:27 -0400 Subject: [PATCH] Update permission manager * Check that updated permissions actually changed. If they didn't, then exclude them from the update * Add spec test --- SmartDeviceLink/SDLPermissionManager.m | 32 +++++++++++++------ .../DevAPISpecs/SDLPermissionsManagerSpec.m | 31 ++++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/SmartDeviceLink/SDLPermissionManager.m b/SmartDeviceLink/SDLPermissionManager.m index 6904190e2..ada837ba7 100644 --- a/SmartDeviceLink/SDLPermissionManager.m +++ b/SmartDeviceLink/SDLPermissionManager.m @@ -185,7 +185,7 @@ - (void)sdl_permissionsDidChange:(SDLRPCNotificationNotification *)notification NSArray *currentFilters = [self.filters copy]; // We can eliminate calling those filters who had no permission changes, so we'll filter down and see which had permissions that changed - NSArray *modifiedFilters = [self.class sdl_filterPermissionChangesForFilters:currentFilters updatedPermissions:newPermissionItems]; + NSArray *modifiedFilters = [self.class sdl_filterPermissionChangesForFilters:currentFilters currentPermissions:self.permissions updatedPermissions:newPermissionItems]; // We need the old group status and new group status for all allowed filters so we know if they should be called NSDictionary *> *allAllowedFiltersWithOldStatus = [self sdl_currentStatusForFilters:modifiedFilters]; @@ -312,19 +312,20 @@ - (BOOL)sdl_didFilterChange:(SDLPermissionFilter *)filter fromHMILevel:(SDLHMILe } /** - * Takes a set of filters and a set of updated permission items. Loops through each permission for each filter and determines if the filter contains a permission that was updated. Returns the set of filters that contain an updated permission. - * - * @param filters The set of filters to check - * @param permissionItems The set of updated permissions to test each filter against - * - * @return An array of filters that contained one of the passed permissions + Takes a set of filters and a set of updated permission items. Loops through each permission for each filter and determines if the filter contains a permission that was updated. Returns the set of filters that contain an updated permission. + + @param filters The set of filters to check + @param currentPermissions The current set of permissions to check the updated permissions and make sure they were modified + @param updatedPermissions The set of updated permissions to test each filter against + @return An array of filters that contained one of the passed permissions */ -+ (NSArray *)sdl_filterPermissionChangesForFilters:(NSArray *)filters updatedPermissions:(NSArray *)permissionItems { ++ (NSArray *)sdl_filterPermissionChangesForFilters:(NSArray *)filters currentPermissions:(NSMutableDictionary *)currentPermissions updatedPermissions:(NSArray *)updatedPermissions { NSMutableArray *modifiedFilters = [NSMutableArray arrayWithCapacity:filters.count]; // Loop through each updated permission item for each filter, if the filter had something modified, store it and go to the next filter for (SDLPermissionFilter *filter in filters) { - for (SDLPermissionItem *item in permissionItems) { + NSArray *modifiedPermissionItems = [self sdl_modifiedUpdatedPermissions:updatedPermissions comparedToCurrentPermissions:currentPermissions]; + for (SDLPermissionItem *item in modifiedPermissionItems) { if ([filter.rpcNames containsObject:item.rpcName]) { [modifiedFilters addObject:filter]; break; @@ -335,6 +336,19 @@ - (BOOL)sdl_didFilterChange:(SDLPermissionFilter *)filter fromHMILevel:(SDLHMILe return [modifiedFilters copy]; } ++ (NSArray *)sdl_modifiedUpdatedPermissions:(NSArray *)permissionItems comparedToCurrentPermissions:(NSMutableDictionary *)currentPermissions { + NSMutableArray *modifiedPermissions = [NSMutableArray arrayWithCapacity:permissionItems.count]; + + for (SDLPermissionItem *item in permissionItems) { + SDLPermissionItem *currentItem = currentPermissions[item.rpcName]; + if (![item isEqual:currentItem]) { + [modifiedPermissions addObject:item]; + } + } + + return [modifiedPermissions copy]; +} + @end NS_ASSUME_NONNULL_END diff --git a/SmartDeviceLinkTests/DevAPISpecs/SDLPermissionsManagerSpec.m b/SmartDeviceLinkTests/DevAPISpecs/SDLPermissionsManagerSpec.m index f0e99b902..05d70a6db 100644 --- a/SmartDeviceLinkTests/DevAPISpecs/SDLPermissionsManagerSpec.m +++ b/SmartDeviceLinkTests/DevAPISpecs/SDLPermissionsManagerSpec.m @@ -449,6 +449,37 @@ @interface SDLPermissionManager () NSNumber *allDisallowed = changeDicts[1][testRPCNameAllDisallowed]; expect(allDisallowed).to(equal(@NO)); }); + + describe(@"when the permission has not changed", ^{ + __block SDLOnPermissionsChange *testPermissionChangeUpdateNoChange = nil; + __block SDLPermissionItem *testPermissionUpdatedNoChange = nil; + + beforeEach(^{ + numberOfTimesObserverCalled = 0; + + // Create a permission update disallowing our current HMI level for the observed permission + SDLParameterPermissions *testParameterPermissions = [[SDLParameterPermissions alloc] init]; + SDLHMIPermissions *testHMIPermissionsUpdated = [[SDLHMIPermissions alloc] init]; + testHMIPermissionsUpdated.allowed = @[SDLHMILevelBackground, SDLHMILevelFull]; + testHMIPermissionsUpdated.userDisallowed = @[SDLHMILevelLimited, SDLHMILevelNone]; + + testPermissionUpdatedNoChange = [[SDLPermissionItem alloc] init]; + testPermissionUpdatedNoChange.rpcName = testRPCNameAllAllowed; + testPermissionUpdatedNoChange.hmiPermissions = testHMIPermissionsUpdated; + testPermissionUpdatedNoChange.parameterPermissions = testParameterPermissions; + + testPermissionChangeUpdateNoChange = [[SDLOnPermissionsChange alloc] init]; + testPermissionChangeUpdateNoChange.permissionItem = [NSArray arrayWithObject:testPermissionUpdated]; + + // Send the permission update + SDLRPCNotificationNotification *updatedNotification = [[SDLRPCNotificationNotification alloc] initWithName:SDLDidChangePermissionsNotification object:nil rpcNotification:testPermissionChangeUpdate]; + [[NSNotificationCenter defaultCenter] postNotification:updatedNotification]; + }); + + it(@"should not call the filter observer again", ^{ + expect(numberOfTimesObserverCalled).to(equal(0)); + }); + }); }); context(@"to match an all allowed observer", ^{