[
https://issues.apache.org/jira/browse/MENFORCER-390?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17911098#comment-17911098
]
ASF GitHub Bot commented on MENFORCER-390:
------------------------------------------
elharo commented on code in PR #297:
URL: https://github.com/apache/maven-enforcer/pull/297#discussion_r1907238846
##########
enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/files/TestRequireFilesDontExist.java:
##########
@@ -107,15 +106,58 @@ void testEmptyFileListAllowNull() {
@Test
void testFileDoesNotExist() throws EnforcerRuleException, IOException {
File f = File.createTempFile("junit", null, temporaryFolder);
+ rule.setFilesList(Collections.singletonList(f));
+
+ EnforcerRuleException e = assertThrows(EnforcerRuleException.class,
rule::execute);
+ assertNotNull(e.getMessage());
+
f.delete();
assertFalse(f.exists());
- rule.setFilesList(Collections.singletonList(f));
-
rule.execute();
}
+ @Test
+ void testSymbolicLinkDoesNotExist() throws Exception {
+ File canonicalFile = File.createTempFile("canonical_", null,
temporaryFolder);
+ File linkFile = Files.createSymbolicLink(
+ Paths.get(temporaryFolder.getAbsolutePath(),
"symbolic.link"),
+ Paths.get(canonicalFile.getAbsolutePath()))
+ .toFile();
+
+ try {
+ rule.setFilesList(Collections.singletonList(linkFile));
+ EnforcerRuleException e =
assertThrows(EnforcerRuleException.class, rule::execute);
+ assertNotNull(e.getMessage());
+
+ linkFile.delete();
+ rule.execute();
+ } finally {
Review Comment:
can we use try with resources for this?
##########
enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/files/TestRequireFilesDontExist.java:
##########
@@ -107,15 +106,58 @@ void testEmptyFileListAllowNull() {
@Test
void testFileDoesNotExist() throws EnforcerRuleException, IOException {
File f = File.createTempFile("junit", null, temporaryFolder);
+ rule.setFilesList(Collections.singletonList(f));
+
+ EnforcerRuleException e = assertThrows(EnforcerRuleException.class,
rule::execute);
+ assertNotNull(e.getMessage());
+
f.delete();
assertFalse(f.exists());
- rule.setFilesList(Collections.singletonList(f));
-
rule.execute();
}
+ @Test
+ void testSymbolicLinkDoesNotExist() throws Exception {
+ File canonicalFile = File.createTempFile("canonical_", null,
temporaryFolder);
+ File linkFile = Files.createSymbolicLink(
+ Paths.get(temporaryFolder.getAbsolutePath(),
"symbolic.link"),
+ Paths.get(canonicalFile.getAbsolutePath()))
+ .toFile();
+
+ try {
+ rule.setFilesList(Collections.singletonList(linkFile));
+ EnforcerRuleException e =
assertThrows(EnforcerRuleException.class, rule::execute);
+ assertNotNull(e.getMessage());
+
+ linkFile.delete();
+ rule.execute();
+ } finally {
+ if (linkFile.exists()) {
+ linkFile.delete();
+ }
+ canonicalFile.delete();
+ }
+ }
+
+ @Test
+ void testSymbolicLinkTargetDoesNotExist() throws Exception {
+ File canonicalFile = File.createTempFile("canonical_", null,
temporaryFolder);
+ File linkFile = Files.createSymbolicLink(
+ Paths.get(temporaryFolder.getAbsolutePath(),
"symbolic.link"),
+ Paths.get(canonicalFile.getAbsolutePath()))
+ .toFile();
+ canonicalFile.delete();
+ rule.setFilesList(Collections.singletonList(linkFile));
+
+ try {
+ rule.execute();
Review Comment:
no exception is a pass?
##########
enforcer-rules/src/site/apt/requireFilesDontExist.apt.vm:
##########
@@ -26,7 +26,10 @@
Require Files Don't Exist
This rule checks that the specified list of files do not exist.
-
+
+ The mounted filesystem(s) will dictate the case-sensitive rules when
checking files. If you require checks that your
Review Comment:
case-sensitivity
##########
enforcer-rules/src/site/apt/requireFilesDontExist.apt.vm:
##########
@@ -26,7 +26,10 @@
Require Files Don't Exist
This rule checks that the specified list of files do not exist.
-
+
+ The mounted filesystem(s) will dictate the case-sensitive rules when
checking files. If you require checks that your
+ filesystem(s) do not support (e.g. a case-sensitive check on a
case-insensitive filesystem) then you should consider
Review Comment:
delete "then you should"
##########
enforcer-rules/src/site/apt/requireFilesDontExist.apt.vm:
##########
@@ -26,7 +26,10 @@
Require Files Don't Exist
This rule checks that the specified list of files do not exist.
-
+
+ The mounted filesystem(s) will dictate the case-sensitive rules when
checking files. If you require checks that your
+ filesystem(s) do not support (e.g. a case-sensitive check on a
case-insensitive filesystem) then you should consider
+ adding your own custom plugin that meets your specific needs.
Review Comment:
your own --> a
##########
enforcer-rules/src/site/apt/requireFilesExist.apt.vm:
##########
@@ -26,7 +26,10 @@
Require Files Exist
This rule checks that the specified list of files exist.
-
+
+ The mounted filesystem(s) will dictate the case-sensitive rules when
checking files. If you require checks that your
Review Comment:
as above
##########
enforcer-rules/src/site/apt/requireFilesSize.apt.vm:
##########
@@ -26,7 +26,10 @@
Require File Size
This rule checks that the specified list of files exist and are within the
specified size range.
-
+
+ The mounted filesystem(s) will dictate the case-sensitive rules when
checking files. If you require checks that your
Review Comment:
ditto
##########
enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/files/TestRequireFilesDontExist.java:
##########
@@ -107,15 +106,58 @@ void testEmptyFileListAllowNull() {
@Test
void testFileDoesNotExist() throws EnforcerRuleException, IOException {
Review Comment:
I don't understand this test. It checks that we create a file, delete it,
and then that the file doesn't exist? Consider renamingi
##########
enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/files/TestRequireFilesDontExist.java:
##########
@@ -20,17 +20,16 @@
import java.io.File;
import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Collections;
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
+import static org.junit.jupiter.api.Assertions.*;
Review Comment:
project style does not use wildcard imports
> "requireFilesExist" no longer handles non-canonical paths
> ---------------------------------------------------------
>
> Key: MENFORCER-390
> URL: https://issues.apache.org/jira/browse/MENFORCER-390
> Project: Maven Enforcer Plugin
> Issue Type: Bug
> Components: requireFilesExist, Standard Rules
> Affects Versions: 3.0.0
> Reporter: Gene Smith
> Priority: Major
>
> With the commit to resolve MENFORCER-364, the rule "requireFileExists" checks
> that the canonical path of a file is the same as the absolute path.
> But not all absolute paths are canonical.
> * absolute paths can involve symbolic links
> * and they are allowed to have parts which are relative
> ** {{/../}}
> ** {{/./}}
> And when it fails to handle a path, it can report that a file does not exist,
> even though the local system will resolve the path.
> A blunt solution might be three separate rules:
> * requireFileExists
> ** the provided path must resolve to a file (which may be a directory or
> link)
> * requireCanonicalFileExists
> ** the provided path must exist as a canonical file
> * requireCasesenstiveFileExists
> ** the provided path must file a file
> ** the file name must have the same case (upper//lower) as the
> ** the parts of the path from the file up must have the same case until they
> go through a symbolic link
> I have used the "nio" package to handle some of stuff before. I will add a
> comment with some java code I would start with. Since the outcome here is
> very dependent on the use case you pick, the java will be "meta code" with
> ??? where you have to know the use case to know the outcome.
> but basically, with "nio" you can march up a path checking for symbolic links
> and such.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
