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

Fix the Permission Manager sending too many observer updates #1359

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 23 additions & 9 deletions SmartDeviceLink/SDLPermissionManager.m
Expand Up @@ -185,7 +185,7 @@ - (void)sdl_permissionsDidChange:(SDLRPCNotificationNotification *)notification
NSArray<SDLPermissionFilter *> *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<SDLPermissionFilter *> *modifiedFilters = [self.class sdl_filterPermissionChangesForFilters:currentFilters updatedPermissions:newPermissionItems];
NSArray<SDLPermissionFilter *> *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<SDLPermissionObserverIdentifier, NSNumber<SDLInt> *> *allAllowedFiltersWithOldStatus = [self sdl_currentStatusForFilters:modifiedFilters];
Expand Down Expand Up @@ -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<SDLPermissionFilter *> *)sdl_filterPermissionChangesForFilters:(NSArray<SDLPermissionFilter *> *)filters updatedPermissions:(NSArray<SDLPermissionItem *> *)permissionItems {
+ (NSArray<SDLPermissionFilter *> *)sdl_filterPermissionChangesForFilters:(NSArray<SDLPermissionFilter *> *)filters currentPermissions:(NSMutableDictionary<SDLPermissionRPCName, SDLPermissionItem *> *)currentPermissions updatedPermissions:(NSArray<SDLPermissionItem *> *)updatedPermissions {
NSMutableArray<SDLPermissionFilter *> *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<SDLPermissionItem *> *modifiedPermissionItems = [self sdl_modifiedUpdatedPermissions:updatedPermissions comparedToCurrentPermissions:currentPermissions];
for (SDLPermissionItem *item in modifiedPermissionItems) {
if ([filter.rpcNames containsObject:item.rpcName]) {
[modifiedFilters addObject:filter];
break;
Expand All @@ -335,6 +336,19 @@ - (BOOL)sdl_didFilterChange:(SDLPermissionFilter *)filter fromHMILevel:(SDLHMILe
return [modifiedFilters copy];
}

+ (NSArray<SDLPermissionItem *> *)sdl_modifiedUpdatedPermissions:(NSArray<SDLPermissionItem *> *)permissionItems comparedToCurrentPermissions:(NSMutableDictionary<SDLPermissionRPCName, SDLPermissionItem *> *)currentPermissions {
NSMutableArray<SDLPermissionItem *> *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
31 changes: 31 additions & 0 deletions SmartDeviceLinkTests/DevAPISpecs/SDLPermissionsManagerSpec.m
Expand Up @@ -449,6 +449,37 @@ @interface SDLPermissionManager ()
NSNumber<SDLBool> *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", ^{
Expand Down