honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Simplify TransportGetRolesAction (elastic/elasticsearch#3888) 

TransportGetRolesAction optimizes for single role case while this
optimization can be simply inside the NativeRoleStore and being
way more contained.

Original commit: elastic/x-pack-elasticsearch@c43d8ba341
This commit is contained in:
Simon Willnauer 2016-10-26 14:55:39 +02:00 committed by GitHub
parent 007e49c5d9
commit 6e1287bab9
3 changed files with 104 additions and 203 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
elasticsearch/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java
View File

@ -67,44 +67,15 @@ public class TransportGetRolesAction extends HandledTransportAction<GetRolesRequ
roles.addAll(reservedRolesStore.roleDescriptors()); roles.addAll(reservedRolesStore.roleDescriptors());
} }
if (rolesToSearchFor.size() == 1) { if (specificRolesRequested && rolesToSearchFor.isEmpty()) {
final String rolename = rolesToSearchFor.get(0);
// We can fetch a single role with a get, much easier
nativeRolesStore.getRoleDescriptor(rolename, new ActionListener<RoleDescriptor>() {
@Override
public void onResponse(RoleDescriptor roleD) {
if (roleD != null) {
roles.add(roleD);
}
listener.onResponse(new GetRolesResponse(roles.toArray(new RoleDescriptor[roles.size()])));
}
@Override
public void onFailure(Exception t) {
logger.error((Supplier<?>) () -> new ParameterizedMessage("failed to retrieve role [{}]", rolename), t);
listener.onFailure(t);
}
});
} else if (specificRolesRequested && rolesToSearchFor.isEmpty()) {
// specific roles were requested but they were built in only, no need to hit the store // specific roles were requested but they were built in only, no need to hit the store
listener.onResponse(new GetRolesResponse(roles.toArray(new RoleDescriptor[roles.size()]))); listener.onResponse(new GetRolesResponse(roles.toArray(new RoleDescriptor[roles.size()])));
} else { } else {
nativeRolesStore.getRoleDescriptors( String[] roleNames = rolesToSearchFor.toArray(new String[rolesToSearchFor.size()]);
rolesToSearchFor.toArray(new String[rolesToSearchFor.size()]), new ActionListener<List<RoleDescriptor>>() { nativeRolesStore.getRoleDescriptors(roleNames, ActionListener.wrap((foundRoles) -> {
@Override roles.addAll(foundRoles);
public void onResponse(List<RoleDescriptor> foundRoles) { listener.onResponse(new GetRolesResponse(roles.toArray(new RoleDescriptor[roles.size()])));
roles.addAll(foundRoles); }, listener::onFailure));
listener.onResponse(new GetRolesResponse(roles.toArray(new RoleDescriptor[roles.size()])));
}
@Override
public void onFailure(Exception t) {
logger.error(
(Supplier<?>) () -> new ParameterizedMessage(
"failed to retrieve role [{}]", arrayToDelimitedString(request.names(), ",")), t);
listener.onFailure(t);
}
});
} }
} }
} }

137
elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java
View File

@ -64,6 +64,7 @@ import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.AtomicReference;
@ -207,89 +208,77 @@ public class NativeRolesStore extends AbstractComponent implements ClusterStateL
listener.onFailure(new IllegalStateException("roles cannot be retrieved as native role service has not been started")); listener.onFailure(new IllegalStateException("roles cannot be retrieved as native role service has not been started"));
return; return;
} }
try { if (names != null && names.length == 1) {
final List<RoleDescriptor> roles = new ArrayList<>(); getRoleAndVersion(Objects.requireNonNull(names[0]), ActionListener.wrap(roleAndVersion ->
QueryBuilder query; listener.onResponse(roleAndVersion == null || roleAndVersion.getRoleDescriptor() == null ? Collections.emptyList()
if (names == null || names.length == 0) { : Collections.singletonList(roleAndVersion.getRoleDescriptor())), listener::onFailure));
query = QueryBuilders.matchAllQuery(); } else {
} else { try {
query = QueryBuilders.boolQuery().filter(QueryBuilders.idsQuery(ROLE_DOC_TYPE).addIds(names)); final List<RoleDescriptor> roles = new ArrayList<>();
} QueryBuilder query;
SearchRequest request = client.prepareSearch(SecurityTemplateService.SECURITY_INDEX_NAME) if (names == null || names.length == 0) {
.setTypes(ROLE_DOC_TYPE) query = QueryBuilders.matchAllQuery();
.setScroll(scrollKeepAlive) } else {
.setQuery(query) query = QueryBuilders.boolQuery().filter(QueryBuilders.idsQuery(ROLE_DOC_TYPE).addIds(names));
.setSize(scrollSize) }
.setFetchSource(true) SearchRequest request = client.prepareSearch(SecurityTemplateService.SECURITY_INDEX_NAME)
.request(); .setTypes(ROLE_DOC_TYPE)
request.indicesOptions().ignoreUnavailable(); .setScroll(scrollKeepAlive)
.setQuery(query)
.setSize(scrollSize)
.setFetchSource(true)
.request();
request.indicesOptions().ignoreUnavailable();
// This function is MADNESS! But it works, don't think about it too hard... // This function is MADNESS! But it works, don't think about it too hard...
client.search(request, new ActionListener<SearchResponse>() { client.search(request, new ActionListener<SearchResponse>() {
private SearchResponse lastResponse = null; private SearchResponse lastResponse = null;
@Override @Override
public void onResponse(SearchResponse resp) { public void onResponse(SearchResponse resp) {
lastResponse = resp; lastResponse = resp;
boolean hasHits = resp.getHits().getHits().length > 0; boolean hasHits = resp.getHits().getHits().length > 0;
if (hasHits) { if (hasHits) {
for (SearchHit hit : resp.getHits().getHits()) { for (SearchHit hit : resp.getHits().getHits()) {
RoleDescriptor rd = transformRole(hit.getId(), hit.getSourceRef(), logger); RoleDescriptor rd = transformRole(hit.getId(), hit.getSourceRef(), logger);
if (rd != null) { if (rd != null) {
roles.add(rd); roles.add(rd);
}
} }
SearchScrollRequest scrollRequest = client.prepareSearchScroll(resp.getScrollId())
.setScroll(scrollKeepAlive).request();
client.searchScroll(scrollRequest, this);
} else {
if (resp.getScrollId() != null) {
clearScollRequest(resp.getScrollId());
}
// Finally, return the list of users
listener.onResponse(Collections.unmodifiableList(roles));
} }
SearchScrollRequest scrollRequest = client.prepareSearchScroll(resp.getScrollId()) }
.setScroll(scrollKeepAlive).request();
client.searchScroll(scrollRequest, this); @Override
} else { public void onFailure(Exception t) {
if (resp.getScrollId() != null) { // attempt to clear the scroll request
clearScollRequest(resp.getScrollId()); if (lastResponse != null && lastResponse.getScrollId() != null) {
clearScollRequest(lastResponse.getScrollId());
}
if (t instanceof IndexNotFoundException) {
logger.trace("could not retrieve roles because security index does not exist");
// since this is expected to happen at times, we just call the listener with an empty list
listener.onResponse(Collections.<RoleDescriptor>emptyList());
} else {
listener.onFailure(t);
} }
// Finally, return the list of users
listener.onResponse(Collections.unmodifiableList(roles));
} }
} });
} catch (Exception e) {
@Override logger.error((Supplier<?>) () -> new ParameterizedMessage("unable to retrieve roles {}", Arrays.toString(names)), e);
public void onFailure(Exception t) {
// attempt to clear the scroll request
if (lastResponse != null && lastResponse.getScrollId() != null) {
clearScollRequest(lastResponse.getScrollId());
}
if (t instanceof IndexNotFoundException) {
logger.trace("could not retrieve roles because security index does not exist");
// since this is expected to happen at times, we just call the listener with an empty list
listener.onResponse(Collections.<RoleDescriptor>emptyList());
} else {
listener.onFailure(t);
}
}
});
} catch (Exception e) {
logger.error((Supplier<?>) () -> new ParameterizedMessage("unable to retrieve roles {}", Arrays.toString(names)), e);
listener.onFailure(e);
}
}
public void getRoleDescriptor(final String role, final ActionListener<RoleDescriptor> listener) {
if (state() != State.STARTED) {
logger.trace("attempted to get role [{}] before service was started", role);
listener.onResponse(null);
}
getRoleAndVersion(role, new ActionListener<RoleAndVersion>() {
@Override
public void onResponse(RoleAndVersion roleAndVersion) {
listener.onResponse(roleAndVersion == null ? null : roleAndVersion.getRoleDescriptor());
}
@Override
public void onFailure(Exception e) {
listener.onFailure(e); listener.onFailure(e);
} }
}); }
} }
public void deleteRole(final DeleteRoleRequest deleteRoleRequest, final ActionListener<Boolean> listener) { public void deleteRole(final DeleteRoleRequest deleteRoleRequest, final ActionListener<Boolean> listener) {

129
elasticsearch/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java
View File

@ -71,15 +71,12 @@ public class TransportGetRolesActionTests extends ESTestCase {
expectedNames.remove(KibanaRole.NAME); expectedNames.remove(KibanaRole.NAME);
} }
doAnswer(new Answer() { doAnswer(invocation -> {
@Override Object[] args = invocation.getArguments();
public Void answer(InvocationOnMock invocation) throws Throwable { assert args.length == 2;
Object[] args = invocation.getArguments(); ActionListener<List<RoleDescriptor>> listener = (ActionListener<List<RoleDescriptor>>) args[1];
assert args.length == 2; listener.onResponse(Collections.emptyList());
ActionListener<List<RoleDescriptor>> listener = (ActionListener<List<RoleDescriptor>>) args[1]; return null;
listener.onResponse(Collections.emptyList());
return null;
}
}).when(rolesStore).getRoleDescriptors(aryEq(Strings.EMPTY_ARRAY), any(ActionListener.class)); }).when(rolesStore).getRoleDescriptors(aryEq(Strings.EMPTY_ARRAY), any(ActionListener.class));
GetRolesRequest request = new GetRolesRequest(); GetRolesRequest request = new GetRolesRequest();
@ -131,32 +128,13 @@ public class TransportGetRolesActionTests extends ESTestCase {
GetRolesRequest request = new GetRolesRequest(); GetRolesRequest request = new GetRolesRequest();
request.names(storeRoleDescriptors.stream().map(RoleDescriptor::getName).collect(Collectors.toList()).toArray(Strings.EMPTY_ARRAY)); request.names(storeRoleDescriptors.stream().map(RoleDescriptor::getName).collect(Collectors.toList()).toArray(Strings.EMPTY_ARRAY));
if (request.names().length == 1) { doAnswer(invocation -> {
doAnswer(new Answer() { Object[] args = invocation.getArguments();
@Override assert args.length == 2;
public Void answer(InvocationOnMock invocation) throws Throwable { ActionListener<List<RoleDescriptor>> listener = (ActionListener<List<RoleDescriptor>>) args[1];
Object[] args = invocation.getArguments(); listener.onResponse(storeRoleDescriptors);
assert args.length == 2; return null;
String requestedName = (String) args[0]; }).when(rolesStore).getRoleDescriptors(aryEq(request.names()), any(ActionListener.class));
ActionListener<RoleDescriptor> listener = (ActionListener<RoleDescriptor>) args[1];
Optional<RoleDescriptor> rd =
storeRoleDescriptors.stream().filter(r -> r.getName().equals(requestedName)).findFirst();
listener.onResponse(rd.get());
return null;
}
}).when(rolesStore).getRoleDescriptor(eq(request.names()[0]), any(ActionListener.class));
} else {
doAnswer(new Answer() {
@Override
public Void answer(InvocationOnMock invocation) throws Throwable {
Object[] args = invocation.getArguments();
assert args.length == 2;
ActionListener<List<RoleDescriptor>> listener = (ActionListener<List<RoleDescriptor>>) args[1];
listener.onResponse(storeRoleDescriptors);
return null;
}
}).when(rolesStore).getRoleDescriptors(aryEq(request.names()), any(ActionListener.class));
}
final AtomicReference<Throwable> throwableRef = new AtomicReference<>(); final AtomicReference<Throwable> throwableRef = new AtomicReference<>();
final AtomicReference<GetRolesResponse> responseRef = new AtomicReference<>(); final AtomicReference<GetRolesResponse> responseRef = new AtomicReference<>();
@ -219,40 +197,21 @@ public class TransportGetRolesActionTests extends ESTestCase {
GetRolesRequest request = new GetRolesRequest(); GetRolesRequest request = new GetRolesRequest();
request.names(requestedNames.toArray(Strings.EMPTY_ARRAY)); request.names(requestedNames.toArray(Strings.EMPTY_ARRAY));
if (specificStoreNames.size() == 1) { doAnswer(invocation -> {
doAnswer(new Answer() { Object[] args = invocation.getArguments();
@Override assert args.length == 2;
public Void answer(InvocationOnMock invocation) throws Throwable { String[] requestedNames1 = (String[]) args[0];
Object[] args = invocation.getArguments(); ActionListener<List<RoleDescriptor>> listener = (ActionListener<List<RoleDescriptor>>) args[1];
assert args.length == 2; if (requestedNames1.length == 0) {
String requestedName = (String) args[0]; listener.onResponse(storeRoleDescriptors);
ActionListener<RoleDescriptor> listener = (ActionListener<RoleDescriptor>) args[1]; } else {
Optional<RoleDescriptor> rd = List<String> requestedNamesList = Arrays.asList(requestedNames1);
storeRoleDescriptors.stream().filter(r -> r.getName().equals(requestedName)).findFirst(); listener.onResponse(storeRoleDescriptors.stream()
listener.onResponse(rd.get()); .filter(r -> requestedNamesList.contains(r.getName()))
return null; .collect(Collectors.toList()));
} }
}).when(rolesStore).getRoleDescriptor(eq(specificStoreNames.get(0)), any(ActionListener.class)); return null;
} else { }).when(rolesStore).getRoleDescriptors(aryEq(specificStoreNames.toArray(Strings.EMPTY_ARRAY)), any(ActionListener.class));
doAnswer(new Answer() {
@Override
public Void answer(InvocationOnMock invocation) throws Throwable {
Object[] args = invocation.getArguments();
assert args.length == 2;
String[] requestedNames = (String[]) args[0];
ActionListener<List<RoleDescriptor>> listener = (ActionListener<List<RoleDescriptor>>) args[1];
if (requestedNames.length == 0) {
listener.onResponse(storeRoleDescriptors);
} else {
List<String> requestedNamesList = Arrays.asList(requestedNames);
listener.onResponse(storeRoleDescriptors.stream()
.filter(r -> requestedNamesList.contains(r.getName()))
.collect(Collectors.toList()));
}
return null;
}
}).when(rolesStore).getRoleDescriptors(aryEq(specificStoreNames.toArray(Strings.EMPTY_ARRAY)), any(ActionListener.class));
}
final AtomicReference<Throwable> throwableRef = new AtomicReference<>(); final AtomicReference<Throwable> throwableRef = new AtomicReference<>();
final AtomicReference<GetRolesResponse> responseRef = new AtomicReference<>(); final AtomicReference<GetRolesResponse> responseRef = new AtomicReference<>();
@ -276,8 +235,6 @@ public class TransportGetRolesActionTests extends ESTestCase {
if (all) { if (all) {
verify(rolesStore, times(1)).getRoleDescriptors(aryEq(Strings.EMPTY_ARRAY), any(ActionListener.class)); verify(rolesStore, times(1)).getRoleDescriptors(aryEq(Strings.EMPTY_ARRAY), any(ActionListener.class));
} else if (specificStoreNames.size() == 1) {
verify(rolesStore, times(1)).getRoleDescriptor(eq(specificStoreNames.get(0)), any(ActionListener.class));
} else { } else {
verify(rolesStore, times(1)) verify(rolesStore, times(1))
.getRoleDescriptors(aryEq(specificStoreNames.toArray(Strings.EMPTY_ARRAY)), any(ActionListener.class)); .getRoleDescriptors(aryEq(specificStoreNames.toArray(Strings.EMPTY_ARRAY)), any(ActionListener.class));
@ -297,29 +254,13 @@ public class TransportGetRolesActionTests extends ESTestCase {
GetRolesRequest request = new GetRolesRequest(); GetRolesRequest request = new GetRolesRequest();
request.names(storeRoleDescriptors.stream().map(RoleDescriptor::getName).collect(Collectors.toList()).toArray(Strings.EMPTY_ARRAY)); request.names(storeRoleDescriptors.stream().map(RoleDescriptor::getName).collect(Collectors.toList()).toArray(Strings.EMPTY_ARRAY));
if (request.names().length == 1) { doAnswer(invocation -> {
doAnswer(new Answer() { Object[] args = invocation.getArguments();
@Override assert args.length == 2;
public Void answer(InvocationOnMock invocation) throws Throwable { ActionListener<List<RoleDescriptor>> listener = (ActionListener<List<RoleDescriptor>>) args[1];
Object[] args = invocation.getArguments(); listener.onFailure(e);
assert args.length == 2; return null;
ActionListener<RoleDescriptor> listener = (ActionListener<RoleDescriptor>) args[1]; }).when(rolesStore).getRoleDescriptors(aryEq(request.names()), any(ActionListener.class));
listener.onFailure(e);
return null;
}
}).when(rolesStore).getRoleDescriptor(eq(request.names()[0]), any(ActionListener.class));
} else {
doAnswer(new Answer() {
@Override
public Void answer(InvocationOnMock invocation) throws Throwable {
Object[] args = invocation.getArguments();
assert args.length == 2;
ActionListener<List<RoleDescriptor>> listener = (ActionListener<List<RoleDescriptor>>) args[1];
listener.onFailure(e);
return null;
}
}).when(rolesStore).getRoleDescriptors(aryEq(request.names()), any(ActionListener.class));
}
final AtomicReference<Throwable> throwableRef = new AtomicReference<>(); final AtomicReference<Throwable> throwableRef = new AtomicReference<>();
final AtomicReference<GetRolesResponse> responseRef = new AtomicReference<>(); final AtomicReference<GetRolesResponse> responseRef = new AtomicReference<>();