From 874a0a4bdd50308ddab5c2ef5c95d3be7fd244dd Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Wed, 1 Jun 2016 11:56:37 -0700 Subject: [PATCH] MetadataResource: Fix handling of includeDisabled. (#3042) --- .../druid/server/http/MetadataResource.java | 142 ++++++++---------- 1 file changed, 60 insertions(+), 82 deletions(-) diff --git a/server/src/main/java/io/druid/server/http/MetadataResource.java b/server/src/main/java/io/druid/server/http/MetadataResource.java index e480121b8b9..bdc66da012d 100644 --- a/server/src/main/java/io/druid/server/http/MetadataResource.java +++ b/server/src/main/java/io/druid/server/http/MetadataResource.java @@ -22,8 +22,9 @@ package io.druid.server.http; import com.google.common.base.Function; import com.google.common.base.Predicate; import com.google.common.collect.Collections2; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; +import com.google.common.collect.Sets; import com.google.inject.Inject; import com.metamx.common.Pair; import com.sun.jersey.spi.container.ResourceFilters; @@ -51,11 +52,10 @@ import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import java.io.IOException; -import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; /** */ @@ -82,71 +82,18 @@ public class MetadataResource @Path("/datasources") @Produces(MediaType.APPLICATION_JSON) public Response getDatabaseDataSources( - @QueryParam("full") String full, - @QueryParam("includeDisabled") String includeDisabled, + @QueryParam("full") final String full, + @QueryParam("includeDisabled") final String includeDisabled, @Context final HttpServletRequest req ) { - Response.ResponseBuilder builder = Response.status(Response.Status.OK); - - final Collection druidDataSources; - if (authConfig.isEnabled()) { - // This is an experimental feature, see - https://github.com/druid-io/druid/pull/2424 - final Map, Access> resourceAccessMap = new HashMap<>(); - final AuthorizationInfo authorizationInfo = (AuthorizationInfo) req.getAttribute(AuthConfig.DRUID_AUTH_TOKEN); - if (includeDisabled != null) { - return builder.entity( - Collections2.filter( - metadataSegmentManager.getAllDatasourceNames(), - new Predicate() - { - @Override - public boolean apply(String input) - { - Resource resource = new Resource(input, ResourceType.DATASOURCE); - Action action = Action.READ; - Pair key = new Pair<>(resource, action); - if (resourceAccessMap.containsKey(key)) { - return resourceAccessMap.get(key).isAllowed(); - } else { - Access access = authorizationInfo.isAuthorized(key.lhs, key.rhs); - resourceAccessMap.put(key, access); - return access.isAllowed(); - } - } - } - )).build(); - } else { - druidDataSources = - Collections2.filter( - metadataSegmentManager.getInventory(), - new Predicate() - { - @Override - public boolean apply(DruidDataSource input) - { - Resource resource = new Resource(input.getName(), ResourceType.DATASOURCE); - Action action = Action.READ; - Pair key = new Pair<>(resource, action); - if (resourceAccessMap.containsKey(key)) { - return resourceAccessMap.get(key).isAllowed(); - } else { - Access access = authorizationInfo.isAuthorized(key.lhs, key.rhs); - resourceAccessMap.put(key, access); - return access.isAllowed(); - } - } - } - ); - } - } else { - druidDataSources = metadataSegmentManager.getInventory(); - } - + final Set dataSourceNamesPreAuth; if (includeDisabled != null) { - return builder.entity( - Collections2.transform( - druidDataSources, + dataSourceNamesPreAuth = Sets.newTreeSet(metadataSegmentManager.getAllDatasourceNames()); + } else { + dataSourceNamesPreAuth = Sets.newTreeSet( + Iterables.transform( + metadataSegmentManager.getInventory(), new Function() { @Override @@ -156,29 +103,60 @@ public class MetadataResource } } ) - ).build(); - } - if (full != null) { - return builder.entity(druidDataSources).build(); + ); } - List dataSourceNames = Lists.newArrayList( - Iterables.transform( - druidDataSources, - new Function() - { - @Override - public String apply(DruidDataSource dataSource) + final Set dataSourceNamesPostAuth; + + if (authConfig.isEnabled()) { + // This is an experimental feature, see - https://github.com/druid-io/druid/pull/2424 + final Map, Access> resourceAccessMap = new HashMap<>(); + final AuthorizationInfo authorizationInfo = (AuthorizationInfo) req.getAttribute(AuthConfig.DRUID_AUTH_TOKEN); + dataSourceNamesPostAuth = ImmutableSet.copyOf( + Sets.filter( + dataSourceNamesPreAuth, + new Predicate() { - return dataSource.getName(); + @Override + public boolean apply(String input) + { + Resource resource = new Resource(input, ResourceType.DATASOURCE); + Action action = Action.READ; + Pair key = new Pair<>(resource, action); + if (resourceAccessMap.containsKey(key)) { + return resourceAccessMap.get(key).isAllowed(); + } else { + Access access = authorizationInfo.isAuthorized(key.lhs, key.rhs); + resourceAccessMap.put(key, access); + return access.isAllowed(); + } + } } - } - ) - ); + ) + ); + } else { + dataSourceNamesPostAuth = dataSourceNamesPreAuth; + } - Collections.sort(dataSourceNames); - - return builder.entity(dataSourceNames).build(); + // Cannot do both includeDisabled and full, let includeDisabled take priority + // Always use dataSourceNamesPostAuth to determine the set of returned dataSources + if (full != null && includeDisabled == null) { + return Response.ok().entity( + Collections2.filter( + metadataSegmentManager.getInventory(), + new Predicate() + { + @Override + public boolean apply(DruidDataSource input) + { + return dataSourceNamesPostAuth.contains(input.getName()); + } + } + ) + ).build(); + } else { + return Response.ok().entity(dataSourceNamesPostAuth).build(); + } } @GET