Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixes #533 - Create scanner with default user auths #744

Merged
merged 1 commit into from Nov 2, 2018
Merged

Fixes #533 - Create scanner with default user auths #744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 28 additions & 0 deletions core/src/main/java/org/apache/accumulo/core/client/AccumuloClient.java
View file
Open in desktop
Expand Up @@ -99,6 +99,20 @@ BatchScanner createBatchScanner(String tableName, Authorizations authorizations,
BatchScanner createBatchScanner(String tableName, Authorizations authorizations)
throws TableNotFoundException;

/**
* Factory method to create a BatchScanner with all of user's authorizations and the number of
* query threads configured when AccumuloClient was created. If no query threads were configured,
* defaults will be used.
*
* @param tableName
* the name of the table to query
*
* @return BatchScanner object for configuring and querying
* @throws TableNotFoundException
* when the specified table doesn't exist
*/
BatchScanner createBatchScanner(String tableName) throws TableNotFoundException, AccumuloSecurityException, AccumuloException;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of creating this new method could add javadoc to the existing method linking to securityOperations().getUserAuthorizations()

Copy link
Member

Choose a reason for hiding this comment

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

With the pluggable authorizations, a user could have an infinite set. So, it may not be sufficient to document that they retrieve and submit their authorizations. I think it makes more sense to provide the method. The first pass implementation could get the user authorizations, but later, we'll probably want to pass along a flag that says "don't intersect with user-provided auths" to the visibility filter when it evaluates visibilities.


/**
* Factory method to create BatchDeleter
*
Expand Down Expand Up @@ -211,6 +225,20 @@ BatchWriter createBatchWriter(String tableName, BatchWriterConfig config)
Scanner createScanner(String tableName, Authorizations authorizations)
throws TableNotFoundException;

/**
* Factory method to create a Scanner with all of the user's authorizations.
*
* @param tableName
* the name of the table to query data from
*
* @return Scanner object for configuring and querying data with
* @throws TableNotFoundException
* when the specified table doesn't exist
*
* @see IsolatedScanner
*/
Scanner createScanner(String tableName) throws TableNotFoundException, AccumuloSecurityException, AccumuloException;

/**
* Factory method to create a ConditionalWriter connected to Accumulo.
*
Expand Down
14 changes: 14 additions & 0 deletions core/src/main/java/org/apache/accumulo/core/client/impl/AccumuloClientImpl.java
View file
Open in desktop
Expand Up @@ -125,6 +125,13 @@ public BatchScanner createBatchScanner(String tableName, Authorizations authoriz
return createBatchScanner(tableName, authorizations, numQueryThreads);
}

@Override
public BatchScanner createBatchScanner(String tableName) throws TableNotFoundException,
AccumuloSecurityException, AccumuloException {
Authorizations auths = securityOperations().getUserAuthorizations(context.getPrincipal());
Copy link
Member

Choose a reason for hiding this comment

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

See comment above. This is okay for a first pass implementation, but not sufficient with pluggable authorization providers which could have an infinite set of auths.

return createBatchScanner(tableName, auths);
}

@Override
public BatchDeleter createBatchDeleter(String tableName, Authorizations authorizations,
int numQueryThreads, BatchWriterConfig config) throws TableNotFoundException {
Expand Down Expand Up @@ -193,6 +200,13 @@ public Scanner createScanner(String tableName, Authorizations authorizations)
return scanner;
}

@Override
public Scanner createScanner(String tableName) throws TableNotFoundException,
AccumuloSecurityException, AccumuloException {
Authorizations auths = securityOperations().getUserAuthorizations(context.getPrincipal());
return createScanner(tableName, auths);
}

@Override
public String whoami() {
ensureOpen();
Expand Down
21 changes: 20 additions & 1 deletion core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java
View file
Open in desktop
Expand Up @@ -31,8 +31,10 @@
import org.apache.accumulo.core.client.AccumuloSecurityException;
import org.apache.accumulo.core.client.BatchWriterConfig;
import org.apache.accumulo.core.client.ClientInfo;
import org.apache.accumulo.core.client.Durability;
import org.apache.accumulo.core.client.security.tokens.AuthenticationToken;
import org.apache.accumulo.core.conf.AccumuloConfiguration;
import org.apache.accumulo.core.conf.ClientProperty;
import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.metadata.RootTable;
import org.apache.accumulo.core.rpc.SaslConnectionParams;
Expand Down Expand Up @@ -243,7 +245,24 @@ public synchronized AccumuloClient getClient()
public BatchWriterConfig getBatchWriterConfig() {
ensureOpen();
if (batchWriterConfig == null) {
batchWriterConfig = ClientInfoFactory.getBatchWriterConfig(getClientInfo());
Properties props = info.getProperties();
batchWriterConfig = new BatchWriterConfig();
Long maxMemory = ClientProperty.BATCH_WRITER_MAX_MEMORY_BYTES.getLong(props);
if (maxMemory != null) {
batchWriterConfig.setMaxMemory(maxMemory);
}
Long maxLatency = ClientProperty.BATCH_WRITER_MAX_LATENCY_SEC.getLong(props);
if (maxLatency != null) {
batchWriterConfig.setMaxLatency(maxLatency, TimeUnit.SECONDS);
}
Long timeout = ClientProperty.BATCH_WRITER_MAX_TIMEOUT_SEC.getLong(props);
if (timeout != null) {
batchWriterConfig.setTimeout(timeout, TimeUnit.SECONDS);
}
String durability = ClientProperty.BATCH_WRITER_DURABILITY.getValue(props);
if (!durability.isEmpty()) {
batchWriterConfig.setDurability(Durability.valueOf(durability.toUpperCase()));
}
}
return batchWriterConfig;
}
Expand Down
59 changes: 0 additions & 59 deletions core/src/main/java/org/apache/accumulo/core/client/impl/ClientInfoFactory.java
View file
Open in desktop

This file was deleted.

4 changes: 1 addition & 3 deletions core/src/main/java/org/apache/accumulo/core/client/impl/ClientInfoImpl.java
View file
Open in desktop
Expand Up @@ -70,9 +70,7 @@ public String getPrincipal() {
@Override
public Properties getProperties() {
Properties result = new Properties();
properties.forEach((key, value) -> {
result.setProperty((String) key, (String) value);
});
properties.forEach((key, value) -> result.setProperty((String) key, (String) value));
return result;
}

Expand Down
6 changes: 3 additions & 3 deletions test/src/main/java/org/apache/accumulo/test/functional/AccumuloClientIT.java
View file
Open in desktop
Expand Up @@ -45,7 +45,7 @@

public class AccumuloClientIT extends AccumuloClusterHarness {

private static interface CloseCheck {
private interface CloseCheck {
void check() throws Exception;
}

Expand All @@ -69,11 +69,11 @@ public void testGetConnectorFromAccumuloClient() throws Exception {
// this should cause the connector to stop functioning
client.close();

expectClosed(() -> c.tableOperations());
expectClosed(c::tableOperations);
}

@Test
public void testclientectorBuilder() throws Exception {
public void testAccumuloClientBuilder() throws Exception {
AccumuloClient c = getAccumuloClient();
String instanceName = c.info().getInstanceName();
String zookeepers = c.info().getZooKeepers();
Expand Down