ACCUMULO-4138 Fix the description of -b options It was wrong for FlushCommand and CompactCommand. In the process of refactoring, the following was accomplished:
- Change the description in the OptUtil.startRowOpt to be (NOT) inclusive. - Use OptUtil.startRowOpt in CompactCommand, MergeCommand and FlushCommand. - Refactor the DeleteRowCommand to use the OptUtil.startRowOpt method. - Refactor ScanCommand to override the description to say it is inclusive. Also updated the javadoc from a potential cut and paste on TableOperations. A little mixed on the usefulness of the Unit Tests for these. They may be brittle, but perhaps they would alert if someone changed the way one command worked without looking at the others. Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/50db442b Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/50db442b Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/50db442b Branch: refs/heads/1.7 Commit: 50db442b04f615ce698d0f63545917be6d4fb13c Parents: a75c685 Author: Michael Wall <mjw...@gmail.com> Authored: Thu Feb 18 15:22:09 2016 -0500 Committer: Michael Wall <mjw...@gmail.com> Committed: Thu Feb 18 15:22:09 2016 -0500 ---------------------------------------------------------------------- .../core/client/admin/TableOperations.java | 4 +-- .../util/shell/commands/DeleteRowsCommand.java | 4 +-- .../core/util/shell/commands/MergeCommand.java | 4 +-- .../core/util/shell/commands/OptUtil.java | 2 +- .../core/util/shell/commands/ScanCommand.java | 5 +++- .../util/shell/commands/CompactCommandTest.java | 29 ++++++++++++++++++++ .../shell/commands/DeleteManyCommandTest.java | 29 ++++++++++++++++++++ .../shell/commands/DeleteRowsCommandTest.java | 29 ++++++++++++++++++++ .../util/shell/commands/FlushCommandTest.java | 29 ++++++++++++++++++++ .../util/shell/commands/MergeCommandTest.java | 29 ++++++++++++++++++++ .../util/shell/commands/ScanCommandTest.java | 29 ++++++++++++++++++++ 11 files changed, 183 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/50db442b/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java b/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java index 536d6e6..bcad3a3 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java +++ b/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java @@ -260,7 +260,7 @@ public interface TableOperations { * @param start * first tablet to be compacted contains the row after this row, null means the first tablet in table * @param end - * last tablet to be merged contains this row, null means the last tablet in table + * last tablet to be compacted contains this row, null means the last tablet in table * @param flush * when true, table memory is flushed before compaction starts * @param wait @@ -276,7 +276,7 @@ public interface TableOperations { * @param start * first tablet to be compacted contains the row after this row, null means the first tablet in table * @param end - * last tablet to be merged contains this row, null means the last tablet in table + * last tablet to be compacted contains this row, null means the last tablet in table * @param iterators * A set of iterators that will be applied to each tablet compacted * @param flush http://git-wip-us.apache.org/repos/asf/accumulo/blob/50db442b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteRowsCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteRowsCommand.java b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteRowsCommand.java index 64968f0..6ffa3f4 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteRowsCommand.java +++ b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteRowsCommand.java @@ -54,9 +54,7 @@ public class DeleteRowsCommand extends Command { public Options getOptions() { final Options o = new Options(); forceOpt = new Option("f", "force", false, "delete data even if start or end are not specified"); - startRowOptExclusive = new Option(OptUtil.START_ROW_OPT, "begin-row", true, "begin row (exclusive)"); - startRowOptExclusive.setArgName("begin-row"); - o.addOption(startRowOptExclusive); + o.addOption(OptUtil.startRowOpt()); o.addOption(OptUtil.endRowOpt()); o.addOption(OptUtil.tableOpt("table to delete a row range from")); o.addOption(forceOpt); http://git-wip-us.apache.org/repos/asf/accumulo/blob/50db442b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/MergeCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/util/shell/commands/MergeCommand.java b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/MergeCommand.java index 9213a06..18d519d 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/shell/commands/MergeCommand.java +++ b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/MergeCommand.java @@ -96,9 +96,7 @@ public class MergeCommand extends Command { sizeOpt = new Option("s", "size", true, "merge tablets to the given size over the entire table"); forceOpt = new Option("f", "force", false, "merge small tablets to large tablets, even if it goes over the given size"); allOpt = new Option("", "all", false, "allow an entire table to be merged into one tablet without prompting the user for confirmation"); - Option startRowOpt = OptUtil.startRowOpt(); - startRowOpt.setDescription("begin row (NOT inclusive)"); - o.addOption(startRowOpt); + o.addOption(OptUtil.startRowOpt()); o.addOption(OptUtil.endRowOpt()); o.addOption(OptUtil.tableOpt("table to be merged")); o.addOption(verboseOpt); http://git-wip-us.apache.org/repos/asf/accumulo/blob/50db442b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/OptUtil.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/util/shell/commands/OptUtil.java b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/OptUtil.java index 9915bdf..432f17a 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/shell/commands/OptUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/OptUtil.java @@ -117,7 +117,7 @@ public abstract class OptUtil { } public static Option startRowOpt() { - final Option o = new Option(START_ROW_OPT, "begin-row", true, "begin row (inclusive)"); + final Option o = new Option(START_ROW_OPT, "begin-row", true, "begin row (NOT) inclusive"); o.setArgName("begin-row"); return o; } http://git-wip-us.apache.org/repos/asf/accumulo/blob/50db442b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/ScanCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/util/shell/commands/ScanCommand.java b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/ScanCommand.java index 9a0026a..60ae0a7 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/shell/commands/ScanCommand.java +++ b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/ScanCommand.java @@ -57,6 +57,7 @@ public class ScanCommand extends Command { protected Option timestampOpt; private Option optStartRowExclusive; + private Option optStartRowInclusive; private Option optEndRowExclusive; private Option timeoutOption; private Option profileOpt; @@ -318,7 +319,9 @@ public class ScanCommand extends Command { o.addOption(scanOptAuths); o.addOption(scanOptRow); - o.addOption(OptUtil.startRowOpt()); + optStartRowInclusive = new Option(OptUtil.START_ROW_OPT, "begin-row", true, "begin row (inclusive)"); + optStartRowInclusive.setArgName("begin-row"); + o.addOption(optStartRowInclusive); o.addOption(OptUtil.endRowOpt()); o.addOption(optStartRowExclusive); o.addOption(optEndRowExclusive); http://git-wip-us.apache.org/repos/asf/accumulo/blob/50db442b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/CompactCommandTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/accumulo/core/util/shell/commands/CompactCommandTest.java b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/CompactCommandTest.java new file mode 100644 index 0000000..9d553da --- /dev/null +++ b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/CompactCommandTest.java @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.core.util.shell.commands; + +import static org.junit.Assert.assertTrue; +import org.junit.Test; + +public class CompactCommandTest { + + @Test + public void testBeginRowHelp() { + assertTrue("-b should say it is NOT inclusive", new CompactCommand().getOptions().getOption("b").getDescription().contains("(NOT) inclusive")); + } + +} http://git-wip-us.apache.org/repos/asf/accumulo/blob/50db442b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/DeleteManyCommandTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/accumulo/core/util/shell/commands/DeleteManyCommandTest.java b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/DeleteManyCommandTest.java new file mode 100644 index 0000000..97de5d3 --- /dev/null +++ b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/DeleteManyCommandTest.java @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.core.util.shell.commands; + +import static org.junit.Assert.assertTrue; +import org.junit.Test; + +public class DeleteManyCommandTest { + + @Test + public void testBeginRowHelp() { + assertTrue("-b should say it is inclusive", new DeleteManyCommand().getOptions().getOption("b").getDescription().contains("row (inclusive)")); + } + +} http://git-wip-us.apache.org/repos/asf/accumulo/blob/50db442b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/DeleteRowsCommandTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/accumulo/core/util/shell/commands/DeleteRowsCommandTest.java b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/DeleteRowsCommandTest.java new file mode 100644 index 0000000..c601fbd --- /dev/null +++ b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/DeleteRowsCommandTest.java @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.core.util.shell.commands; + +import static org.junit.Assert.assertTrue; +import org.junit.Test; + +public class DeleteRowsCommandTest { + + @Test + public void testBeginRowHelp() { + assertTrue("-b should say it is NOT inclusive", new DeleteRowsCommand().getOptions().getOption("b").getDescription().contains("(NOT) inclusive")); + } + +} http://git-wip-us.apache.org/repos/asf/accumulo/blob/50db442b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/FlushCommandTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/accumulo/core/util/shell/commands/FlushCommandTest.java b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/FlushCommandTest.java new file mode 100644 index 0000000..7a775eb --- /dev/null +++ b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/FlushCommandTest.java @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.core.util.shell.commands; + +import static org.junit.Assert.assertTrue; +import org.junit.Test; + +public class FlushCommandTest { + + @Test + public void testBeginRowHelp() { + assertTrue("-b should say it is NOT inclusive", new FlushCommand().getOptions().getOption("b").getDescription().contains("(NOT) inclusive")); + } + +} http://git-wip-us.apache.org/repos/asf/accumulo/blob/50db442b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/MergeCommandTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/accumulo/core/util/shell/commands/MergeCommandTest.java b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/MergeCommandTest.java new file mode 100644 index 0000000..3826b28 --- /dev/null +++ b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/MergeCommandTest.java @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.core.util.shell.commands; + +import static org.junit.Assert.assertTrue; +import org.junit.Test; + +public class MergeCommandTest { + + @Test + public void testBeginRowHelp() { + assertTrue("-b should say it is NOT inclusive", new MergeCommand().getOptions().getOption("b").getDescription().contains("(NOT) inclusive")); + } + +} http://git-wip-us.apache.org/repos/asf/accumulo/blob/50db442b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/ScanCommandTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/accumulo/core/util/shell/commands/ScanCommandTest.java b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/ScanCommandTest.java new file mode 100644 index 0000000..a356fe8 --- /dev/null +++ b/core/src/test/java/org/apache/accumulo/core/util/shell/commands/ScanCommandTest.java @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.core.util.shell.commands; + +import static org.junit.Assert.assertTrue; +import org.junit.Test; + +public class ScanCommandTest { + + @Test + public void testBeginRowHelp() { + assertTrue("-b should say it is inclusive", new ScanCommand().getOptions().getOption("b").getDescription().contains("row (inclusive)")); + } + +}