ctubbsii commented on code in PR #41:
URL: https://github.com/apache/accumulo-proxy/pull/41#discussion_r1021956444
##########
src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java:
##########
@@ -974,320 +975,232 @@ public void testNamespaceClassLoadLoginFailure() {
}
@Test
- public void tableNotFound() throws Exception {
+ public void tableNotFound() throws IOException {
final String doesNotExist = "doesNotExists";
- try {
- client.addConstraint(creds, doesNotExist,
NumericValueConstraint.class.getName());
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.addSplits(creds, doesNotExist, Collections.<ByteBuffer>
emptySet());
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.addConstraint(creds, doesNotExist,
NumericValueConstraint.class.getName()));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.addSplits(creds, doesNotExist, Collections.emptySet()));
+
final IteratorSetting setting = new IteratorSetting(100, "slow",
SlowIterator.class.getName(),
Collections.singletonMap("sleepTime", "200"));
- try {
- client.attachIterator(creds, doesNotExist, setting,
EnumSet.allOf(IteratorScope.class));
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.cancelCompaction(creds, doesNotExist);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.checkIteratorConflicts(creds, doesNotExist, setting,
- EnumSet.allOf(IteratorScope.class));
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.clearLocatorCache(creds, doesNotExist);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- final String TABLE_TEST = getUniqueNameArray(1)[0];
- client.cloneTable(creds, doesNotExist, TABLE_TEST, false, null, null);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.compactTable(creds, doesNotExist, null, null, null, true, false,
null);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.createBatchScanner(creds, doesNotExist, new BatchScanOptions());
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.createScanner(creds, doesNotExist, new ScanOptions());
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.createWriter(creds, doesNotExist, new WriterOptions());
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.deleteRows(creds, doesNotExist, null, null);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.deleteTable(creds, doesNotExist);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.exportTable(creds, doesNotExist, "/tmp");
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.flushTable(creds, doesNotExist, null, null, false);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.getIteratorSetting(creds, doesNotExist, "foo",
IteratorScope.SCAN);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.getLocalityGroups(creds, doesNotExist);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.getMaxRow(creds, doesNotExist, Collections.<ByteBuffer>
emptySet(), null, false, null,
- false);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.getTableProperties(creds, doesNotExist);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.grantTablePermission(creds, "root", doesNotExist,
TablePermission.WRITE);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.hasTablePermission(creds, "root", doesNotExist,
TablePermission.WRITE);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- MiniAccumuloClusterImpl cluster = SharedMiniClusterBase.getCluster();
- Path base = cluster.getTemporaryPath();
- Path importDir = new Path(base, "importDir");
- Path failuresDir = new Path(base, "failuresDir");
- assertTrue(cluster.getFileSystem().mkdirs(importDir));
- assertTrue(cluster.getFileSystem().mkdirs(failuresDir));
- client.importDirectory(creds, doesNotExist, importDir.toString(),
failuresDir.toString(),
- true);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.listConstraints(creds, doesNotExist);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.listSplits(creds, doesNotExist, 10000);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.mergeTablets(creds, doesNotExist, null, null);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.offlineTable(creds, doesNotExist, false);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.onlineTable(creds, doesNotExist, false);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.removeConstraint(creds, doesNotExist, 0);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.removeIterator(creds, doesNotExist, "name",
EnumSet.allOf(IteratorScope.class));
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.removeTableProperty(creds, doesNotExist,
Property.TABLE_FILE_MAX.getKey());
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.renameTable(creds, doesNotExist, "someTableName");
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.revokeTablePermission(creds, "root", doesNotExist,
TablePermission.ALTER_TABLE);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.setTableProperty(creds, doesNotExist,
Property.TABLE_FILE_MAX.getKey(), "0");
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.splitRangeByTablets(creds, doesNotExist,
- client.getRowRange(ByteBuffer.wrap("row".getBytes(UTF_8))), 10);
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.updateAndFlush(creds, doesNotExist, new
HashMap<ByteBuffer,List<ColumnUpdate>>());
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.getDiskUsage(creds, Collections.singleton(doesNotExist));
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.testTableClassLoad(creds, doesNotExist,
VersioningIterator.class.getName(),
- SortedKeyValueIterator.class.getName());
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
- try {
- client.createConditionalWriter(creds, doesNotExist, new
ConditionalWriterOptions());
- fail("exception not thrown");
- } catch (TableNotFoundException ex) {}
+
+ assertThrows(TableNotFoundException.class, () ->
client.attachIterator(creds, doesNotExist,
+ setting, EnumSet.allOf(IteratorScope.class)));
+
+ assertThrows(TableNotFoundException.class, () ->
client.cancelCompaction(creds, doesNotExist));
+
+ assertThrows(TableNotFoundException.class, () ->
client.checkIteratorConflicts(creds,
+ doesNotExist, setting, EnumSet.allOf(IteratorScope.class)));
+
+ assertThrows(TableNotFoundException.class, () ->
client.clearLocatorCache(creds, doesNotExist));
+
+ final String TABLE_TEST = getUniqueNameArray(1)[0];
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.cloneTable(creds, doesNotExist, TABLE_TEST, false, null,
null));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.compactTable(creds, doesNotExist, null, null, null, true,
false, null));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.createBatchScanner(creds, doesNotExist, new
BatchScanOptions()));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.createScanner(creds, doesNotExist, new ScanOptions()));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.createWriter(creds, doesNotExist, new WriterOptions()));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.deleteRows(creds, doesNotExist, null, null));
+
+ assertThrows(TableNotFoundException.class, () -> client.deleteTable(creds,
doesNotExist));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.exportTable(creds, doesNotExist, "/tmp"));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.flushTable(creds, doesNotExist, null, null, false));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.getIteratorSetting(creds, doesNotExist, "foo",
IteratorScope.SCAN));
+
+ assertThrows(TableNotFoundException.class, () ->
client.getLocalityGroups(creds, doesNotExist));
+
+ assertThrows(TableNotFoundException.class, () -> client.getMaxRow(creds,
doesNotExist,
+ Collections.emptySet(), null, false, null, false));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.getTableProperties(creds, doesNotExist));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.grantTablePermission(creds, "root", doesNotExist,
TablePermission.WRITE));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.hasTablePermission(creds, "root", doesNotExist,
TablePermission.WRITE));
+
+ MiniAccumuloClusterImpl cluster = SharedMiniClusterBase.getCluster();
+ Path base = cluster.getTemporaryPath();
+ Path importDir = new Path(base, "importDir");
+ Path failuresDir = new Path(base, "failuresDir");
+ assertTrue(cluster.getFileSystem().mkdirs(importDir));
+ assertTrue(cluster.getFileSystem().mkdirs(failuresDir));
+
+ assertThrows(TableNotFoundException.class, () ->
client.importDirectory(creds, doesNotExist,
+ importDir.toString(), failuresDir.toString(), true));
+
+ assertThrows(TableNotFoundException.class, () ->
client.listConstraints(creds, doesNotExist));
+
+ assertThrows(TableNotFoundException.class, () -> client.listSplits(creds,
doesNotExist, 10000));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.mergeTablets(creds, doesNotExist, null, null));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.offlineTable(creds, doesNotExist, false));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.onlineTable(creds, doesNotExist, false));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.removeConstraint(creds, doesNotExist, 0));
+
+ assertThrows(TableNotFoundException.class, () ->
client.removeIterator(creds, doesNotExist,
+ "name", EnumSet.allOf(IteratorScope.class)));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.removeTableProperty(creds, doesNotExist,
Property.TABLE_FILE_MAX.getKey()));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.renameTable(creds, doesNotExist, "someTableName"));
+
+ assertThrows(TableNotFoundException.class, () ->
client.revokeTablePermission(creds, "root",
+ doesNotExist, TablePermission.ALTER_TABLE));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.setTableProperty(creds, doesNotExist,
Property.TABLE_FILE_MAX.getKey(), "0"));
+
+ assertThrows(TableNotFoundException.class, () ->
client.splitRangeByTablets(creds, doesNotExist,
+ client.getRowRange(ByteBuffer.wrap("row".getBytes(UTF_8))), 10));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.updateAndFlush(creds, doesNotExist, new HashMap<>()));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.getDiskUsage(creds, Collections.singleton(doesNotExist)));
+
+ assertThrows(TableNotFoundException.class, () ->
client.testTableClassLoad(creds, doesNotExist,
+ VersioningIterator.class.getName(),
SortedKeyValueIterator.class.getName()));
+
+ assertThrows(TableNotFoundException.class,
+ () -> client.createConditionalWriter(creds, doesNotExist, new
ConditionalWriterOptions()));
}
@Test
- public void namespaceNotFound() throws Exception {
+ public void namespaceNotFound() {
final String doesNotExist = "doesNotExists";
- try {
- client.deleteNamespace(creds, doesNotExist);
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- client.renameNamespace(creds, doesNotExist, "abcdefg");
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- client.setNamespaceProperty(creds, doesNotExist,
"table.compaction.major.ratio", "4");
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- client.removeNamespaceProperty(creds, doesNotExist,
"table.compaction.major.ratio");
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- client.getNamespaceProperties(creds, doesNotExist);
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- IteratorSetting setting = new IteratorSetting(100, "DebugTheThings",
- DebugIterator.class.getName(), Collections.emptyMap());
- client.attachNamespaceIterator(creds, doesNotExist, setting,
- EnumSet.allOf(IteratorScope.class));
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- client.removeNamespaceIterator(creds, doesNotExist, "DebugTheThings",
- EnumSet.allOf(IteratorScope.class));
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- client.getNamespaceIteratorSetting(creds, doesNotExist,
"DebugTheThings", IteratorScope.SCAN);
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- client.listNamespaceIterators(creds, doesNotExist);
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- IteratorSetting setting = new IteratorSetting(100, "DebugTheThings",
- DebugIterator.class.getName(), Collections.emptyMap());
- client.checkNamespaceIteratorConflicts(creds, doesNotExist, setting,
- EnumSet.allOf(IteratorScope.class));
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- client.addNamespaceConstraint(creds, doesNotExist,
MaxMutationSize.class.getName());
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- client.removeNamespaceConstraint(creds, doesNotExist, 1);
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- client.listNamespaceConstraints(creds, doesNotExist);
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
- try {
- client.testNamespaceClassLoad(creds, doesNotExist,
DebugIterator.class.getName(),
- SortedKeyValueIterator.class.getName());
- fail("exception not thrown");
- } catch (NamespaceNotFoundException ex) {}
+ IteratorSetting iteratorSetting = new IteratorSetting(100,
"DebugTheThings",
+ DebugIterator.class.getName(), Collections.emptyMap());
+
+ assertThrows(NamespaceNotFoundException.class,
+ () -> client.deleteNamespace(creds, doesNotExist));
+
+ assertThrows(NamespaceNotFoundException.class,
+ () -> client.renameNamespace(creds, doesNotExist, "abcdefg"));
+
+ assertThrows(NamespaceNotFoundException.class, () ->
client.setNamespaceProperty(creds,
+ doesNotExist, "table.compaction.major.ratio", "4"));
Review Comment:
Since a lot of these are checking for the same exception, I was thinking you
could just loop over all these lambda's, something like:
```java
Stream.<Executable>of(() -> client.deleteNamespace(creds, doesNotExist),
() -> client.renameNamespace(creds, doesNotExist, "abcdefg"),
...
.forEach(executable -> assertThrows(NamespaceNotFoundException.class,
executable));
```
This removes a lot of boilerplate, since we're testing the same thing over
and over again. This is great if everything is passing, but it might be hard to
troubleshoot which one had a problem when there's a failure. That might be
easier to troubleshoot by adding a loop counter to track which executable
failed to throw the expected exception and broke out of the loop.
In any case, it's fine the way you have it now.
##########
src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java:
##########
@@ -1738,17 +1639,15 @@ public void userPermissions() throws Exception {
assertFalse(client.listTables(creds).contains("fail"));
}
// denied!
- try {
- if (isKerberosEnabled()) {
- // Switch back to the extra user
- UserGroupInformation.loginUserFromKeytab(otherClient.getPrincipal(),
- otherClient.getKeytab().getAbsolutePath());
- client = userClient;
- }
- String scanner = client.createScanner(user, tableName, null);
- client.nextK(scanner, 100);
- fail("stooge should not read table test");
- } catch (AccumuloSecurityException ex) {}
+ if (isKerberosEnabled()) {
+ // Switch back to the extra user
+ UserGroupInformation.loginUserFromKeytab(otherClient.getPrincipal(),
+ otherClient.getKeytab().getAbsolutePath());
+ client = userClient;
+ }
+ String scanner = client.createScanner(user, tableName, null);
+ assertThrows(AccumuloSecurityException.class, () -> client.nextK(scanner,
100),
+ "stooge should not read table test");
Review Comment:
The block was removed, causing variable name collisions for some variables,
like `scanner`. If you prefer to reuse the variable names instead of renaming
them, you can put them inside an anonymous block (keep the block, but drop the
`try` and `catch`), as in:
```java
{
String scanner = client.createScanner(user, tableName, null);
assertThrows(AccumuloSecurityException.class, () ->
client.nextK(scanner, 100),
"stooge should not read table test");
}
```
Or you can just use a different variable name, like you did. It's up to you.
I just wanted to make sure you were aware. I sometimes use anonymous blocks to
more strictly control the scope of commonly named temporary variables, so they
don't get accidentally referenced later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
