Author: billie Date: Thu Dec 1 22:05:58 2011 New Revision: 1209272 URL: http://svn.apache.org/viewvc?rev=1209272&view=rev Log: ACCUMULO-200 added checks that throw errors on conflicts
Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java?rev=1209272&r1=1209271&r2=1209272&view=diff ============================================================================== --- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java (original) +++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java Thu Dec 1 22:05:58 2011 @@ -36,7 +36,8 @@ public interface ScannerBase extends Ite * @param cfg * fully specified scan-time iterator, including all options for the iterator. Any changes to the iterator setting after this call are not propagated * to the stored iterator. - * + * @throws IllegalArgumentException + * if the setting conflicts with existing iterators */ public void addScanIterator(IteratorSetting cfg); Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java?rev=1209272&r1=1209271&r2=1209272&view=diff ============================================================================== --- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java (original) +++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java Thu Dec 1 22:05:58 2011 @@ -471,6 +471,8 @@ public interface TableOperations { * @throws AccumuloException * @throws TableNotFoundException * throw if the table no longer exists + * @throws IllegalArgumentException + * if the setting conflicts with any existing iterators */ public void attachIterator(String tableName, IteratorSetting setting) throws AccumuloSecurityException, AccumuloException, TableNotFoundException; @@ -522,4 +524,19 @@ public interface TableOperations { * @throws TableNotFoundException */ public Set<String> listIterators(String tableName) throws AccumuloSecurityException, AccumuloException, TableNotFoundException; + + /** + * Check whether a given iterator configuration conflicts with existing configuration; in particular, determine if the name or priority are already in use for + * the specified scopes. + * + * @param tableName + * the name of the table + * @param setting + * object specifying the properties of the iterator + * @throws AccumuloException + * @throws TableNotFoundException + * @throws IllegalStateException + * if the setting conflicts with any existing iterators + */ + public void checkIteratorConflicts(String tableName, IteratorSetting setting) throws AccumuloException, TableNotFoundException; } Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java?rev=1209272&r1=1209271&r2=1209272&view=diff ============================================================================== --- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java (original) +++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java Thu Dec 1 22:05:58 2011 @@ -22,6 +22,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import java.util.TreeMap; import org.apache.accumulo.core.client.AccumuloException; import org.apache.accumulo.core.client.AccumuloSecurityException; @@ -34,7 +35,7 @@ public abstract class TableOperationsHel @Override public void attachIterator(String tableName, IteratorSetting setting) throws AccumuloSecurityException, AccumuloException, TableNotFoundException { - removeIterator(tableName, setting.getName(), setting.getScopes()); + checkIteratorConflicts(tableName, setting); for (IteratorScope scope : setting.getScopes()) { String root = String.format("%s%s.%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase(), setting.getName()); this.setProperty(tableName, root, setting.getPriority() + "," + setting.getIteratorClass()); @@ -103,4 +104,35 @@ public abstract class TableOperationsHel } return result; } + + @Override + public void checkIteratorConflicts(String tableName, IteratorSetting setting) throws AccumuloException, TableNotFoundException { + for (IteratorScope scope : setting.getScopes()) { + String scopeStr = String.format("%s%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase()); + String nameStr = String.format("%s.%s", scopeStr, setting.getName()); + String optStr = String.format("%s.opt.", nameStr); + Map<String,String> optionConflicts = new TreeMap<String,String>(); + for (Entry<String,String> property : this.getProperties(tableName)) { + if (property.getKey().startsWith(scopeStr)) { + if (property.getKey().equals(nameStr)) + throw new IllegalArgumentException("iterator name conflict for " + setting.getName() + ": " + property.getKey() + "=" + property.getValue()); + if (property.getKey().startsWith(optStr)) + optionConflicts.put(property.getKey(), property.getValue()); + if (property.getKey().contains(".opt.")) + continue; + String parts[] = property.getValue().split(","); + if (parts.length != 2) + throw new AccumuloException("Bad value for existing iterator setting: " + property.getKey() + "=" + property.getValue()); + try { + if (Integer.parseInt(parts[0]) == setting.getPriority()) + throw new IllegalArgumentException("iterator priority conflict: " + property.getKey() + "=" + property.getValue()); + } catch (NumberFormatException e) { + throw new AccumuloException("Bad value for existing iterator setting: " + property.getKey() + "=" + property.getValue()); + } + } + } + if (optionConflicts.size() > 0) + throw new IllegalArgumentException("iterator options conflict for " + setting.getName() + ": " + optionConflicts); + } + } } Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java?rev=1209272&r1=1209271&r2=1209272&view=diff ============================================================================== --- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java (original) +++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java Thu Dec 1 22:05:58 2011 @@ -64,9 +64,12 @@ public class ScannerOptions implements S if (serverSideIteratorList.size() == 0) serverSideIteratorList = new ArrayList<IterInfo>(); - for (IterInfo ii : serverSideIteratorList) + for (IterInfo ii : serverSideIteratorList) { if (ii.iterName.equals(si.getName())) throw new IllegalArgumentException("Iterator name is already in use " + si.getName()); + if (ii.getPriority() == si.getPriority()) + throw new IllegalArgumentException("Iterator priority is already in use " + si.getPriority()); + } serverSideIteratorList.add(new IterInfo(si.getPriority(), si.getIteratorClass(), si.getName())); Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java?rev=1209272&r1=1209271&r2=1209272&view=diff ============================================================================== --- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java (original) +++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java Thu Dec 1 22:05:58 2011 @@ -58,6 +58,8 @@ public class DeleteIterCommand extends C scopes.add(IteratorScope.majc); if (cl.hasOption(scanScopeOpt.getOpt())) scopes.add(IteratorScope.scan); + if (scopes.isEmpty()) + throw new IllegalArgumentException("You must select at least one scope to configure"); shellState.getConnector().tableOperations().removeIterator(tableName, name, scopes); return 0; } @@ -95,4 +97,4 @@ public class DeleteIterCommand extends C public int numArgs() { return 0; } -} \ No newline at end of file +} Modified: incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java?rev=1209272&r1=1209271&r2=1209272&view=diff ============================================================================== --- incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java (original) +++ incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java Thu Dec 1 22:05:58 2011 @@ -204,7 +204,7 @@ public class TableOperationsHelperTest { t.check("table", new String[] {"table.iterator.scan.someName=10,foo.bar",}); t.removeIterator("table", "someName", EnumSet.of(IteratorScope.scan)); t.check("table", new String[] {}); - + IteratorSetting setting = new IteratorSetting(10, "someName", "foo.bar"); setting.setScopes(EnumSet.of(IteratorScope.majc)); setting.addOptions(Collections.singletonMap("key", "value")); @@ -213,7 +213,7 @@ public class TableOperationsHelperTest { t.attachIterator("table", setting); t.check("table", new String[] {"table.iterator.majc.someName=10,foo.bar", "table.iterator.majc.someName.opt.key=value", "table.iterator.scan.someName=10,foo.bar",}); - + setting = new IteratorSetting(20, "otherName", "some.classname"); setting.setScopes(EnumSet.of(IteratorScope.majc)); setting.addOptions(Collections.singletonMap("key", "value")); @@ -227,7 +227,7 @@ public class TableOperationsHelperTest { t.removeIterator("table", "someName", EnumSet.allOf(IteratorScope.class)); t.check("table", new String[] {"table.iterator.majc.otherName=20,some.classname", "table.iterator.majc.otherName.opt.key=value", "table.iterator.scan.otherName=20,some.classname",}); - + setting = t.getIteratorSetting("table", "otherName", IteratorScope.scan); Assert.assertEquals(20, setting.getPriority()); Assert.assertEquals("some.classname", setting.getIteratorClass()); @@ -241,6 +241,18 @@ public class TableOperationsHelperTest { t.attachIterator("table", setting); t.check("table", new String[] {"table.iterator.majc.otherName=20,some.classname", "table.iterator.majc.otherName.opt.key=value", "table.iterator.minc.otherName=20,some.classname", "table.iterator.minc.otherName.opt.key=value", "table.iterator.scan.otherName=20,some.classname",}); + + try { + t.attachIterator("table", setting); + Assert.fail(); + } catch (IllegalArgumentException e) {} + setting.setName("thirdName"); + try { + t.attachIterator("table", setting); + Assert.fail(); + } catch (IllegalArgumentException e) {} + setting.setPriority(10); + t.attachIterator("table", setting); } }