Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@

import java.io.Closeable;
import java.io.IOException;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -316,7 +315,7 @@ public Map<String, Object> usageStats() {
stats.put("evictions", cacheStats.getEvictions());
stats.put("hits_time_in_millis", TimeValue.nsecToMSec(hitsTimeInNanos.sum()));
stats.put("misses_time_in_millis", TimeValue.nsecToMSec(missesTimeInNanos.sum()));
return Collections.unmodifiableMap(stats);
return stats;
}

private static final class BitsetCacheKey {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@

import org.elasticsearch.cluster.node.DiscoveryNodeUtils;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.test.AbstractWireSerializingTestCase;

import java.io.IOException;
import java.util.Map;
import java.util.Objects;

import static org.hamcrest.Matchers.equalTo;

public class GetSecurityStatsNodeResponseTests extends AbstractWireSerializingTestCase<GetSecurityStatsNodeResponse> {

@Override
Expand Down Expand Up @@ -46,4 +49,20 @@ protected GetSecurityStatsNodeResponse mutateInstance(GetSecurityStatsNodeRespon
default -> throw new IllegalStateException("Unexpected value");
};
}

public void testRolesStatsInOrderSerialization() throws IOException {
final Map<String, Object> rolesStats = Maps.newLinkedHashMapWithExpectedSize(3);
rolesStats.put("one", "value");
rolesStats.put("two", "value");
rolesStats.put("three", "value");
final GetSecurityStatsNodeResponse in = new GetSecurityStatsNodeResponse(
DiscoveryNodeUtils.create(randomAlphaOfLength(10)),
rolesStats
);

final GetSecurityStatsNodeResponse out = copyInstance(in);
assertThat(in, equalTo(out));

assertThat(out.getRolesStoreStats().keySet().toArray(new String[0]), equalTo(new String[] { "one", "two", "three" }));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.lessThan;
import static org.hamcrest.Matchers.not;
Expand Down Expand Up @@ -529,6 +530,11 @@ public void testHitsMissesAndEvictionsStats() throws Exception {
assertThat(cache.usageStats(), equalTo(finalStats));
}

public void testUsageStatsAreOrdered() {
final Map<String, Object> stats = newCache(Settings.EMPTY).usageStats();
assertThat("needs to be LinkedHashMap for order in transport", stats, instanceOf(LinkedHashMap.class));
}

private void runTestOnIndex(CheckedBiConsumer<SearchExecutionContext, LeafReaderContext, Exception> body) throws Exception {
runTestOnIndices(1, ctx -> {
final TestIndexContext indexContext = ctx.get(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.util.concurrent.ReleasableLock;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.util.set.Sets;
Expand Down Expand Up @@ -670,7 +671,9 @@ Iterable<ProjectScoped<RoleKey>> cachedRoles() {
}

public Map<String, Object> usageStatsWithJustDls() {
return Map.of("dls", Map.of("bit_set_cache", dlsBitsetCache.usageStats()));
final Map<String, Object> usageStats = Maps.newLinkedHashMapWithExpectedSize(1);
usageStats.put("dls", Map.of("bit_set_cache", dlsBitsetCache.usageStats()));
return usageStats; // return LinkedHashMap for deterministic order in transport
Comment on lines +674 to +676
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really needed right now since only one key so order is preserved

but added to leave a stronger note for anyone touching this code in the future to consider ordering

}

public void usageStats(ActionListener<Map<String, Object>> listener) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -3754,6 +3755,27 @@ public void testGetRoleDescriptorsListUsesRoleStoreToResolveRoleWithInternalRole
assertThat(future.actionGet(), equalTo(Set.of(expectedRoleDescriptor)));
}

public void testOrderedUsageStatsWithJustDls() {
final CompositeRolesStore compositeRolesStore = buildCompositeRolesStore(
SECURITY_ENABLED_SETTINGS,
null,
null,
null,
null,
null,
null,
null,
null,
null
);

assertThat(
"needs LinkedHashMap to be ordered in transport",
compositeRolesStore.usageStatsWithJustDls(),
instanceOf(LinkedHashMap.class)
);
}

private Role getRoleForRoleNames(CompositeRolesStore store, String... roleNames) {
final PlainActionFuture<Role> future = new PlainActionFuture<>();
getRoleForRoleNames(store, Set.of(roleNames), future);
Expand Down