[
https://issues.apache.org/jira/browse/HADOOP-10579?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18085049#comment-18085049
]
ASF GitHub Bot commented on HADOOP-10579:
-----------------------------------------
aajisaka commented on code in PR #8259:
URL: https://github.com/apache/hadoop/pull/8259#discussion_r3331459870
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/find/TestSize.java:
##########
@@ -0,0 +1,151 @@
+/**
+ * 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.hadoop.fs.shell.find;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.*;
+import static org.apache.hadoop.fs.shell.find.TestHelper.*;
+
+import java.io.IOException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.shell.PathData;
+
+public class TestSize {
+ private FileSystem mockFs;
+
+ private PathData fiveBlocks;
+ private PathData fourBlocks;
+ private PathData sixBlocks;
+ private PathData fiveBlocksMinus;
+ private PathData fiveBlocksPlus;
+
+ @BeforeEach
+ public void resetMock() throws IOException {
+ mockFs = MockFileSystem.setup();
+
+ fiveBlocks = createPathData("fiveBlocks", 5L * 512L);
+ sixBlocks = createPathData("sixBlocks", 6L * 512L);
+ fourBlocks = createPathData("fourBlocks", 4L * 512L);
+ fiveBlocksPlus = createPathData("fiveBlocksPlus", (5L * 512L) + 511L);
+ fiveBlocksMinus = createPathData("fiveBlocksMinus", (5L * 512L) - 1L);
+ }
+
+ private PathData createPathData(String name, long length) throws IOException
{
+ Path path = new Path(name);
+ FileStatus fileStatus = mock(FileStatus.class);
+ when(fileStatus.getLen()).thenReturn(length);
+ when(mockFs.getFileStatus(eq(path))).thenReturn(fileStatus);
+ return new PathData(name, mockFs.getConf());
+ }
+
+ // test exact match in blocks
+ @Test
+ public void applyEqualsBlock() throws IOException {
+ Size size = new Size();
+ addArgument(size, "5");
+ size.setOptions(new FindOptions());
+ size.prepare();
+
+ assertEquals(Result.PASS, size.apply(fiveBlocks, -1));
+ assertEquals(Result.FAIL, size.apply(sixBlocks, -1));
+ assertEquals(Result.FAIL, size.apply(fourBlocks, -1));
+ assertEquals(Result.PASS, size.apply(fiveBlocksPlus, -1));
+ assertEquals(Result.FAIL, size.apply(fiveBlocksMinus, -1));
+ }
+
+ // test greater than match in blocks
+ @Test
+ public void applyGreaterThanBlock() throws IOException {
+ Size size = new Size();
+ addArgument(size, "+5");
+ size.setOptions(new FindOptions());
+ size.prepare();
+
+ assertEquals(Result.FAIL, size.apply(fiveBlocks, -1));
+ assertEquals(Result.PASS, size.apply(sixBlocks, -1));
+ assertEquals(Result.FAIL, size.apply(fourBlocks, -1));
+ assertEquals(Result.FAIL, size.apply(fiveBlocksPlus, -1));
Review Comment:
fiveBlocksPlus should pass
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/find/NumberExpression.java:
##########
@@ -0,0 +1,109 @@
+/**
+ * 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.hadoop.fs.shell.find;
+
+import java.io.IOException;
+import java.util.Deque;
+
+/**
+ * Base class for numeric {@link Expression}s.
+ * Implements helper methods for processing numeric arguments.
+ */
+public abstract class NumberExpression extends BaseExpression {
+ /**
+ * Number of milliseconds in a day.
+ */
+ protected static final long DAY_IN_MILLISECONDS = 86400000L;
+
+ /**
+ * Number of milliseconds in a minute.
+ */
+ protected static final long MINUTE_IN_MILLISECONDS = 60000L;
+
+ private long max = -1L;
+ private long min = -1L;
+ private long units = 1L;
+
+ /**
+ * Constructor specifying for size of units to used as the expression
+ * argument.
+ *
+ * @param units size of the expression argument compared to the item being
+ * processed.
+ */
+ protected NumberExpression(long units) {
+ setUnits(units);
+ }
+
+ protected NumberExpression() {
+ this(1L);
+ }
+
+ protected void setUnits(long units) {
+ this.units = units;
+ }
+
+ @Override
+ public void prepare() throws IOException {
+ parseArgument(getArgument(1));
+ }
+
+ /**
+ * Parse the argument string to extract the numeric argument.
+ *
+ * @param arg String to be parsed
+ * @throws IllegalArgumentException if there is a problem parsing the
argument
+ */
+ protected void parseArgument(String arg) throws IllegalArgumentException {
+ if (arg == null) {
+ throw new IllegalArgumentException("Invalid null argument");
+ } else if (arg.isEmpty()) {
+ throw new IllegalArgumentException("Invalid empty argument");
+ }
+ if (arg.startsWith("+")) {
+ min = (Long.parseLong(arg.substring(1)) * units) + units;
+ } else if (arg.startsWith("-")) {
+ max = (Long.parseLong(arg.substring(1)) * units) - 1L;
Review Comment:
It sets `max = -1` for arguments like `-size -0` / `-mtime -0`. Then
`applyNumber` treats `-1` as unset and the upper bound is ignored. This makes
`-0` pass all entries.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/find/TestSize.java:
##########
@@ -0,0 +1,151 @@
+/**
+ * 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.hadoop.fs.shell.find;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.*;
+import static org.apache.hadoop.fs.shell.find.TestHelper.*;
+
+import java.io.IOException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.shell.PathData;
+
+public class TestSize {
+ private FileSystem mockFs;
+
+ private PathData fiveBlocks;
+ private PathData fourBlocks;
+ private PathData sixBlocks;
+ private PathData fiveBlocksMinus;
+ private PathData fiveBlocksPlus;
+
+ @BeforeEach
+ public void resetMock() throws IOException {
+ mockFs = MockFileSystem.setup();
+
+ fiveBlocks = createPathData("fiveBlocks", 5L * 512L);
+ sixBlocks = createPathData("sixBlocks", 6L * 512L);
+ fourBlocks = createPathData("fourBlocks", 4L * 512L);
+ fiveBlocksPlus = createPathData("fiveBlocksPlus", (5L * 512L) + 511L);
+ fiveBlocksMinus = createPathData("fiveBlocksMinus", (5L * 512L) - 1L);
+ }
+
+ private PathData createPathData(String name, long length) throws IOException
{
+ Path path = new Path(name);
+ FileStatus fileStatus = mock(FileStatus.class);
+ when(fileStatus.getLen()).thenReturn(length);
+ when(mockFs.getFileStatus(eq(path))).thenReturn(fileStatus);
+ return new PathData(name, mockFs.getConf());
+ }
+
+ // test exact match in blocks
+ @Test
+ public void applyEqualsBlock() throws IOException {
+ Size size = new Size();
+ addArgument(size, "5");
+ size.setOptions(new FindOptions());
+ size.prepare();
+
+ assertEquals(Result.PASS, size.apply(fiveBlocks, -1));
+ assertEquals(Result.FAIL, size.apply(sixBlocks, -1));
+ assertEquals(Result.FAIL, size.apply(fourBlocks, -1));
+ assertEquals(Result.PASS, size.apply(fiveBlocksPlus, -1));
+ assertEquals(Result.FAIL, size.apply(fiveBlocksMinus, -1));
+ }
+
+ // test greater than match in blocks
+ @Test
+ public void applyGreaterThanBlock() throws IOException {
+ Size size = new Size();
+ addArgument(size, "+5");
+ size.setOptions(new FindOptions());
+ size.prepare();
+
+ assertEquals(Result.FAIL, size.apply(fiveBlocks, -1));
+ assertEquals(Result.PASS, size.apply(sixBlocks, -1));
+ assertEquals(Result.FAIL, size.apply(fourBlocks, -1));
+ assertEquals(Result.FAIL, size.apply(fiveBlocksPlus, -1));
+ assertEquals(Result.FAIL, size.apply(fiveBlocksMinus, -1));
+ }
+
+ // test less than match in blocks
+ @Test
+ public void applyLessThanBlock() throws IOException {
+ Size size = new Size();
+ addArgument(size, "-5");
+ size.setOptions(new FindOptions());
+ size.prepare();
+
+ assertEquals(Result.FAIL, size.apply(fiveBlocks, -1));
+ assertEquals(Result.FAIL, size.apply(sixBlocks, -1));
+ assertEquals(Result.PASS, size.apply(fourBlocks, -1));
+ assertEquals(Result.FAIL, size.apply(fiveBlocksPlus, -1));
+ assertEquals(Result.PASS, size.apply(fiveBlocksMinus, -1));
Review Comment:
fiveBlocksMinus should fail, because it's rounded up to 5 blocks.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/find/TestSize.java:
##########
@@ -0,0 +1,151 @@
+/**
+ * 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.hadoop.fs.shell.find;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.*;
+import static org.apache.hadoop.fs.shell.find.TestHelper.*;
+
+import java.io.IOException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.shell.PathData;
+
+public class TestSize {
+ private FileSystem mockFs;
+
+ private PathData fiveBlocks;
+ private PathData fourBlocks;
+ private PathData sixBlocks;
+ private PathData fiveBlocksMinus;
+ private PathData fiveBlocksPlus;
+
+ @BeforeEach
+ public void resetMock() throws IOException {
+ mockFs = MockFileSystem.setup();
+
+ fiveBlocks = createPathData("fiveBlocks", 5L * 512L);
+ sixBlocks = createPathData("sixBlocks", 6L * 512L);
+ fourBlocks = createPathData("fourBlocks", 4L * 512L);
+ fiveBlocksPlus = createPathData("fiveBlocksPlus", (5L * 512L) + 511L);
+ fiveBlocksMinus = createPathData("fiveBlocksMinus", (5L * 512L) - 1L);
+ }
+
+ private PathData createPathData(String name, long length) throws IOException
{
+ Path path = new Path(name);
+ FileStatus fileStatus = mock(FileStatus.class);
+ when(fileStatus.getLen()).thenReturn(length);
+ when(mockFs.getFileStatus(eq(path))).thenReturn(fileStatus);
+ return new PathData(name, mockFs.getConf());
+ }
+
+ // test exact match in blocks
+ @Test
+ public void applyEqualsBlock() throws IOException {
+ Size size = new Size();
+ addArgument(size, "5");
+ size.setOptions(new FindOptions());
+ size.prepare();
+
+ assertEquals(Result.PASS, size.apply(fiveBlocks, -1));
+ assertEquals(Result.FAIL, size.apply(sixBlocks, -1));
+ assertEquals(Result.FAIL, size.apply(fourBlocks, -1));
+ assertEquals(Result.PASS, size.apply(fiveBlocksPlus, -1));
+ assertEquals(Result.FAIL, size.apply(fiveBlocksMinus, -1));
Review Comment:
In this case, fiveBocksMinus should pass and fiveBlocksPlus should fail.
This is because fiveBlocksMinus fits in 5 blocks and fiveBLocksPlus exceeds 5
blocks.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/find/Perm.java:
##########
@@ -0,0 +1,209 @@
+/**
+ * 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.hadoop.fs.shell.find;
+
+import java.io.IOException;
+import java.util.Deque;
+
+import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.fs.shell.PathData;
+
+/**
+ * Implements the -perm expression for the {@link
org.apache.hadoop.fs.shell.find.Find} command.
+ */
+final class Perm extends BaseExpression {
+ /**
+ * Registers this expression with the specified factory.
+ */
+ public static void registerExpression(ExpressionFactory factory) throws
IOException {
+ factory.addClass(Perm.class, "-perm");
+ }
+
+ private static final String[] USAGE = {"-perm [-]mode", "-perm [-]onum"};
+ private static final String[] HELP = {"Evaluates as true if the file
permissions match that",
+ "specified. If the hyphen is specified then the expression",
+ "shall evaluate as true if at least the bits specified",
+ "match, otherwise an exact match is required.",
+ "The mode may be specified using either symbolic notation,",
+ "eg 'u=rwx,g+x+w' or as an octal number."};
Review Comment:
Does `g+x+w` pass? Looks like it's rejected in L132.
> Find command - add match expressions to find command
> ----------------------------------------------------
>
> Key: HADOOP-10579
> URL: https://issues.apache.org/jira/browse/HADOOP-10579
> Project: Hadoop Common
> Issue Type: Sub-task
> Reporter: Jonathan Allen
> Assignee: Hiroki Egawa
> Priority: Minor
> Labels: pull-request-available
> Attachments: HADOOP-10579.patch
>
>
> Add match expressions to the find command created under HADOOP-8989, e.g.:
> - atime
> - empty
> - group
> - mtime
> - newer
> - nogroup
> - nouser
> - perm
> - regex
> - size
> - user
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
