HasPrivilegesResponse use TreeSet for fields (#36329)

For class fields of type collection whose order is not important 
and for which duplicates are not permitted we declare them as `Set`s.
Usually the definition is a `HashSet` but in this case `TreeSet` is used
instead to aid testing.
This commit is contained in:
Albert Zaharovits 2018-12-15 08:34:54 +02:00 committed by GitHub
parent 4e4022b7ef
commit a30e8c2fa3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 36 additions and 31 deletions

View File

@ -328,7 +328,7 @@ public final class CcrLicenseChecker {
message.append(indices.length == 1 ? " index " : " indices ");
message.append(Arrays.toString(indices));
HasPrivilegesResponse.ResourcePrivileges resourcePrivileges = response.getIndexPrivileges().get(0);
HasPrivilegesResponse.ResourcePrivileges resourcePrivileges = response.getIndexPrivileges().iterator().next();
for (Map.Entry<String, Boolean> entry : resourcePrivileges.getPrivileges().entrySet()) {
if (entry.getValue() == false) {
message.append(", privilege for action [");

View File

@ -13,14 +13,14 @@ import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.TreeSet;
/**
* Response for a {@link HasPrivilegesRequest}
@ -29,8 +29,8 @@ public class HasPrivilegesResponse extends ActionResponse implements ToXContentO
private String username;
private boolean completeMatch;
private Map<String, Boolean> cluster;
private List<ResourcePrivileges> index;
private Map<String, List<ResourcePrivileges>> application;
private Set<ResourcePrivileges> index;
private Map<String, Set<ResourcePrivileges>> application;
public HasPrivilegesResponse() {
this("", true, Collections.emptyMap(), Collections.emptyList(), Collections.emptyMap());
@ -41,15 +41,17 @@ public class HasPrivilegesResponse extends ActionResponse implements ToXContentO
super();
this.username = username;
this.completeMatch = completeMatch;
this.cluster = new HashMap<>(cluster);
this.index = sorted(new ArrayList<>(index));
this.application = new HashMap<>();
application.forEach((key, val) -> this.application.put(key, Collections.unmodifiableList(sorted(new ArrayList<>(val)))));
this.cluster = Collections.unmodifiableMap(cluster);
this.index = Collections.unmodifiableSet(sorted(index));
final Map<String, Set<ResourcePrivileges>> applicationPrivileges = new HashMap<>();
application.forEach((key, val) -> applicationPrivileges.put(key, Collections.unmodifiableSet(sorted(val))));
this.application = Collections.unmodifiableMap(applicationPrivileges);
}
private static List<ResourcePrivileges> sorted(List<ResourcePrivileges> resources) {
Collections.sort(resources, Comparator.comparing(o -> o.resource));
return resources;
private static Set<ResourcePrivileges> sorted(Collection<ResourcePrivileges> resources) {
final Set<ResourcePrivileges> set = new TreeSet<>(Comparator.comparing(o -> o.resource));
set.addAll(resources);
return set;
}
public String getUsername() {
@ -61,19 +63,19 @@ public class HasPrivilegesResponse extends ActionResponse implements ToXContentO
}
public Map<String, Boolean> getClusterPrivileges() {
return Collections.unmodifiableMap(cluster);
return cluster;
}
public List<ResourcePrivileges> getIndexPrivileges() {
return Collections.unmodifiableList(index);
public Set<ResourcePrivileges> getIndexPrivileges() {
return index;
}
/**
* Retrieves the results from checking application privileges,
* @return A {@code Map} keyed by application-name
*/
public Map<String, List<ResourcePrivileges>> getApplicationPrivileges() {
return Collections.unmodifiableMap(application);
public Map<String, Set<ResourcePrivileges>> getApplicationPrivileges() {
return application;
}
@Override
@ -112,15 +114,15 @@ public class HasPrivilegesResponse extends ActionResponse implements ToXContentO
}
}
private static List<ResourcePrivileges> readResourcePrivileges(StreamInput in) throws IOException {
private static Set<ResourcePrivileges> readResourcePrivileges(StreamInput in) throws IOException {
final int count = in.readVInt();
final List<ResourcePrivileges> list = new ArrayList<>(count);
final Set<ResourcePrivileges> set = new TreeSet<>(Comparator.comparing(o -> o.resource));
for (int i = 0; i < count; i++) {
final String index = in.readString();
final Map<String, Boolean> privileges = in.readMap(StreamInput::readString, StreamInput::readBoolean);
list.add(new ResourcePrivileges(index, privileges));
set.add(new ResourcePrivileges(index, privileges));
}
return list;
return set;
}
@Override
@ -139,7 +141,7 @@ public class HasPrivilegesResponse extends ActionResponse implements ToXContentO
}
}
private static void writeResourcePrivileges(StreamOutput out, List<ResourcePrivileges> privileges) throws IOException {
private static void writeResourcePrivileges(StreamOutput out, Set<ResourcePrivileges> privileges) throws IOException {
out.writeVInt(privileges.size());
for (ResourcePrivileges priv : privileges) {
out.writeString(priv.resource);
@ -179,7 +181,7 @@ public class HasPrivilegesResponse extends ActionResponse implements ToXContentO
return builder;
}
private void appendResources(XContentBuilder builder, String field, List<HasPrivilegesResponse.ResourcePrivileges> privileges)
private void appendResources(XContentBuilder builder, String field, Set<HasPrivilegesResponse.ResourcePrivileges> privileges)
throws IOException {
builder.startObject(field);
for (HasPrivilegesResponse.ResourcePrivileges privilege : privileges) {

View File

@ -44,9 +44,11 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import static java.util.Collections.emptyMap;
import static org.elasticsearch.common.util.set.Sets.newHashSet;
@ -136,7 +138,7 @@ public class TransportHasPrivilegesActionTests extends ESTestCase {
assertThat(response.getClusterPrivileges().get(ClusterHealthAction.NAME), equalTo(true));
assertThat(response.getIndexPrivileges(), Matchers.iterableWithSize(1));
final ResourcePrivileges result = response.getIndexPrivileges().get(0);
final ResourcePrivileges result = response.getIndexPrivileges().iterator().next();
assertThat(result.getResource(), equalTo("academy"));
assertThat(result.getPrivileges().size(), equalTo(2));
assertThat(result.getPrivileges().get(DeleteAction.NAME), equalTo(true));
@ -174,9 +176,10 @@ public class TransportHasPrivilegesActionTests extends ESTestCase {
assertThat(response.getClusterPrivileges().get("manage"), equalTo(false));
assertThat(response.getIndexPrivileges(), Matchers.iterableWithSize(3));
final ResourcePrivileges academy = response.getIndexPrivileges().get(0);
final ResourcePrivileges initiative = response.getIndexPrivileges().get(1);
final ResourcePrivileges school = response.getIndexPrivileges().get(2);
final Iterator<ResourcePrivileges> indexPrivilegesIterator = response.getIndexPrivileges().iterator();
final ResourcePrivileges academy = indexPrivilegesIterator.next();
final ResourcePrivileges initiative = indexPrivilegesIterator.next();
final ResourcePrivileges school = indexPrivilegesIterator.next();
assertThat(academy.getResource(), equalTo("academy"));
assertThat(academy.getPrivileges().size(), equalTo(3));
@ -213,7 +216,7 @@ public class TransportHasPrivilegesActionTests extends ESTestCase {
assertThat(response.getUsername(), is(user.principal()));
assertThat(response.isCompleteMatch(), is(false));
assertThat(response.getIndexPrivileges(), Matchers.iterableWithSize(1));
final ResourcePrivileges result = response.getIndexPrivileges().get(0);
final ResourcePrivileges result = response.getIndexPrivileges().iterator().next();
assertThat(result.getResource(), equalTo("academy"));
assertThat(result.getPrivileges().size(), equalTo(2));
assertThat(result.getPrivileges().get("read"), equalTo(false));
@ -309,7 +312,7 @@ public class TransportHasPrivilegesActionTests extends ESTestCase {
new ResourcePrivileges("a*xyz", mapBuilder().put("read", false).put("write", true).put("manage", false).map())
));
assertThat(response.getApplicationPrivileges().entrySet(), Matchers.iterableWithSize(1));
final List<ResourcePrivileges> kibanaPrivileges = response.getApplicationPrivileges().get("kibana");
final Set<ResourcePrivileges> kibanaPrivileges = response.getApplicationPrivileges().get("kibana");
assertThat(kibanaPrivileges, Matchers.iterableWithSize(3));
assertThat(Strings.collectionToCommaDelimitedString(kibanaPrivileges), kibanaPrivileges, containsInAnyOrder(
new ResourcePrivileges("*", mapBuilder().put("read", true).put("write", false).map()),
@ -378,7 +381,7 @@ public class TransportHasPrivilegesActionTests extends ESTestCase {
assertThat(response.isCompleteMatch(), is(false));
assertThat(response.getIndexPrivileges(), Matchers.emptyIterable());
assertThat(response.getApplicationPrivileges().entrySet(), Matchers.iterableWithSize(2));
final List<ResourcePrivileges> app1 = response.getApplicationPrivileges().get("app1");
final Set<ResourcePrivileges> app1 = response.getApplicationPrivileges().get("app1");
assertThat(app1, Matchers.iterableWithSize(4));
assertThat(Strings.collectionToCommaDelimitedString(app1), app1, containsInAnyOrder(
new ResourcePrivileges("foo/1", MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
@ -390,7 +393,7 @@ public class TransportHasPrivilegesActionTests extends ESTestCase {
new ResourcePrivileges("baz/bar/foo", MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
.put("read", false).put("write", false).put("all", false).map())
));
final List<ResourcePrivileges> app2 = response.getApplicationPrivileges().get("app2");
final Set<ResourcePrivileges> app2 = response.getApplicationPrivileges().get("app2");
assertThat(app2, Matchers.iterableWithSize(4));
assertThat(Strings.collectionToCommaDelimitedString(app2), app2, containsInAnyOrder(
new ResourcePrivileges("foo/1", MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())