-
Notifications
You must be signed in to change notification settings - Fork 488
Modified VisibilityFilter to handle multiple Authorizations objects #6402
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,10 @@ | |
| import static java.nio.charset.StandardCharsets.UTF_8; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.ArrayList; | ||
| import java.util.Collection; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
|
|
||
| import org.apache.accumulo.access.InvalidAccessExpressionException; | ||
| import org.apache.accumulo.core.client.IteratorSetting; | ||
|
|
@@ -39,6 +42,8 @@ | |
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import com.google.common.base.Preconditions; | ||
|
|
||
| /** | ||
| * A SortedKeyValueIterator that filters based on ColumnVisibility. | ||
| */ | ||
|
|
@@ -50,7 +55,8 @@ public class VisibilityFilter extends Filter implements OptionDescriber { | |
|
|
||
| private static final Logger log = LoggerFactory.getLogger(VisibilityFilter.class); | ||
|
|
||
| private static final String AUTHS = "auths"; | ||
| private static final String NUM_AUTHS = "numAuths"; | ||
| private static final String AUTH_PREFIX = "auth_"; | ||
| private static final String FILTER_INVALID_ONLY = "filterInvalid"; | ||
|
|
||
| private boolean filterInvalid; | ||
|
|
@@ -63,11 +69,26 @@ public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> op | |
| this.filterInvalid = Boolean.parseBoolean(options.get(FILTER_INVALID_ONLY)); | ||
|
|
||
| if (!filterInvalid) { | ||
| String auths = options.get(AUTHS); | ||
| Authorizations authObj = auths == null || auths.isEmpty() ? new Authorizations() | ||
| : new Authorizations(auths.getBytes(UTF_8)); | ||
|
|
||
| this.accessEvaluator = BytesAccess.newEvaluator(authObj); | ||
| String numAuthsParameter = options.get(NUM_AUTHS); | ||
| Objects.requireNonNull(numAuthsParameter, "NUM_AUTHS option not set."); | ||
| int numAuths = Integer.parseInt(numAuthsParameter); | ||
| Preconditions.checkArgument(numAuths >= 0, NUM_AUTHS + " must be a positive integer"); | ||
|
|
||
| Collection<Authorizations> authSet = new ArrayList<>(); | ||
| if (numAuths == 0) { | ||
| authSet.add(new Authorizations()); | ||
| } else { | ||
| for (int idx = 0; idx < numAuths; idx++) { | ||
| String auths = options.get(AUTH_PREFIX + idx); | ||
| Authorizations authObj = auths == null || auths.isEmpty() ? new Authorizations() | ||
| : new Authorizations(auths.getBytes(UTF_8)); | ||
| authSet.add(authObj); | ||
| } | ||
| String auths = options.get(AUTH_PREFIX + numAuths); | ||
| Preconditions.checkArgument(auths == null, | ||
| "NUM_AUTHS is set incorrectly, should be at least: " + NUM_AUTHS + " = " + 1); | ||
| } | ||
| this.accessEvaluator = BytesAccess.newEvaluator(authSet); | ||
| } | ||
| this.cache = new LRUMap<>(1000); | ||
| } | ||
|
|
@@ -136,14 +157,25 @@ public IteratorOptions describeOptions() { | |
| io.addNamedOption(FILTER_INVALID_ONLY, | ||
| "if 'true', the iterator is instructed to ignore the authorizations and" | ||
| + " only filter invalid visibility labels (default: false)"); | ||
| io.addNamedOption(AUTHS, | ||
| "the serialized set of authorizations to filter against (default: empty" | ||
| + " string, accepts only entries visible by all)"); | ||
|
Comment on lines
-139
to
-141
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't remove this option. The configuration parameter is the main API for this class. Removing or renaming it breaks it. |
||
| io.addNamedOption(NUM_AUTHS, | ||
| "The number of serialized authorizations to filter against (default 0)"); | ||
| io.addUnnamedOption(AUTH_PREFIX | ||
| + "N, where the value is a serialized set of authorizations. N must be between zero and NUM_AUTHS."); | ||
|
Comment on lines
+160
to
+163
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure these descriptions really communicate how these are used. Is the idea that you specify some config like: And then it does something like: Is the idea to replace multiple executions of the iterator with a single one? Instead, I think it might make sense to have a config that preserves the existing behavior, and extends the existing config: This is simpler, and backwards compatible. The description can be extended to say: "if multiple serialized sets are configured, separated by semi-colons, then entries are accepted only if they would be accepted by each of the sets independently" |
||
| return io; | ||
| } | ||
|
|
||
| public static void setAuthorizations(IteratorSetting setting, Authorizations auths) { | ||
| setting.addOption(AUTHS, auths.serialize()); | ||
| setting.addOption(NUM_AUTHS, "1"); | ||
| setting.addOption(AUTH_PREFIX + 0, auths.serialize()); | ||
| } | ||
|
|
||
| public static void setAuthorizations(IteratorSetting setting, Collection<Authorizations> auths) { | ||
| setting.addOption(NUM_AUTHS, Integer.toString(auths.size())); | ||
| int idx = 0; | ||
| for (Authorizations auth : auths) { | ||
| setting.addOption(AUTH_PREFIX + idx, auth.serialize()); | ||
| idx++; | ||
| } | ||
| } | ||
|
|
||
| public static void filterInvalidLabelsOnly(IteratorSetting setting, boolean featureEnabled) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment is warranted here to explain why ISO-8859-1 is used.