Skip to content

Commit

Permalink
Optimized permission manager for non-mutative update requests
Browse files Browse the repository at this point in the history
  • Loading branch information
billkalter committed Apr 20, 2017
1 parent 17b97ee commit e54213c
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,12 @@ public PermissionUpdateRequest revokeRest() {
public boolean isRevokeRest() {
return _revokeRest;
}

/**
* Returns true if this request could potentially modify permissions if applied. It returns true if there is at
* least one permission permitted or revoked, or {@link #revokeRest()} is true.
*/
public boolean mayModifyPermissions() {
return !_permissions.isEmpty() || _revokeRest;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,12 @@ public Set<Permission> getPermissions(String id) {

@Override
public void updatePermissions(String id, PermissionUpdateRequest updates) {
// If the request doesn't contain any modifications then still forward to the delegate but don't invalidate
// the cache.
_manager.updatePermissions(id, updates);
_cacheManager.invalidateAll();
if (updates.mayModifyPermissions()) {
_cacheManager.invalidateAll();
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.apache.shiro.authz.permission.InvalidPermissionStringException;
import org.apache.shiro.authz.permission.PermissionResolver;

import javax.annotation.Nullable;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -74,48 +73,36 @@ public void updatePermissions(String id, PermissionUpdateRequest request) {
checkNotNull(request, "request");
validateTable();

Delta delta = createDelta(request);
if (delta == null) {
// Request did not change the permissions at all. Take no action.
return;
// Only update if the request may potentially modify the permissions
if (request.mayModifyPermissions()) {
Delta delta = createDelta(request);

_dataStore.update(
_tableName,
id,
TimeUUIDs.newUUID(),
delta,
new AuditBuilder().setLocalHost().setComment("update permissions").build(),
WriteConsistency.GLOBAL);
}

_dataStore.update(
_tableName,
id,
TimeUUIDs.newUUID(),
delta,
new AuditBuilder().setLocalHost().setComment("update permissions").build(),
WriteConsistency.GLOBAL);
}

/**
* Returns a delta constructed from this request, or null if the request contained no changes.
*/
@Nullable
private Delta createDelta(PermissionUpdateRequest request) {
MapDeltaBuilder builder = Deltas.mapBuilder();
boolean modified = false;

for (String permissionString : request.getPermitted()) {
builder.put("perm_" + validated(permissionString), 1);
modified = true;
}
for (String permissionString : request.getRevoked()) {
builder.remove("perm_" + validated(permissionString));
modified = true;
}
if (request.isRevokeRest()) {
builder.removeRest();
modified = true;
}

if (modified) {
return builder.build();
}

// Request contained no changes
return null;
return builder.build();
}

/**
Expand Down

0 comments on commit e54213c

Please sign in to comment.