You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "Filip (Jira)" <ji...@apache.org> on 2022/07/19 12:41:00 UTC

[jira] [Updated] (AVRO-3582) Avro Maven plug in should fail if imports do not resolve to actual file/directory

     [ https://issues.apache.org/jira/browse/AVRO-3582?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Filip updated AVRO-3582:
------------------------
    Attachment: abstractAvroMojo.patch
        Status: Patch Available  (was: Open)

> Avro Maven plug in should fail if imports do not resolve to actual file/directory
> ---------------------------------------------------------------------------------
>
>                 Key: AVRO-3582
>                 URL: https://issues.apache.org/jira/browse/AVRO-3582
>             Project: Apache Avro
>          Issue Type: Improvement
>          Components: java, tools
>    Affects Versions: 1.10.2, 1.12.0
>            Reporter: Filip
>            Priority: Minor
>         Attachments: abstractAvroMojo.patch
>
>
> {{org.apache.avro.mojo.AbstractAvroMojo#getIncludedFiles}} contains the following code:
> {code:java}
> 251 // exclude imports directory since it has already been compiled.
> 252 if (imports != null) {
> 253   String importExclude = null;
> 254
> 255   for (String importFile : this.imports) {
> 256     File file = new File(importFile);
> 257
> 258     if (file.isDirectory()) {
> 259       importExclude = file.getName() + "/**";
> 260     } else if (file.isFile()) {
> 261       importExclude = "**/" + file.getName();
> 262     }
> 263 
> 264     fs.addExclude(importExclude);
> 265   }
> 266 } {code}
> which is supposed to explicitly exclude any directories that have been added through an {{<import>...</import>}} configuration property.
> Unfortunately, when the configuration property does *not* resolve to an actual file or directory, e.g.:
> {code:java}
> <imports>
>   <import>/src/main/resources/schemas/</import> <!-- note the extra / at the beginning -->
> </imports>{code}
> Then both checks at line no. {{258}} and {{260}} evaluate to {{{}false{}}}, meaning that in line no. {{264}} the code:
> {code:java}
> fs.addExclude(null);
> {code}
> is executed.
> This leads to the behaviour that the following condition in the {{org.apache.maven.shared.model.fileset.util.FileSetManager#scan}} code:
>  
> {code:java}
> 656 if ( excludesArray.length > 0 )
> 657 {
> 658     scanner.setExcludes( excludesArray );
> 659 } {code}
> evaluates to {{{}true{}}}, causing an exception to occur in  {{org.apache.maven.shared.utils.io.DirectoryScanner#setExcludes}}
> {code:java}
> 357 pattern = excludes[i].trim().replace( '/', File.separatorChar ).replace( '\\', File.separatorChar );{code}
> with the Maven error:
> {code:java}
> [ERROR] Failed to execute goal org.apache.avro:avro-maven-plugin:1.11.0:schema (default) on project app: Execution default of goal org.apache.avro:avro-maven-plugin:1.11.0:schema failed: Cannot invoke "String.trim()" because "excludes[i]" is null -> [Help 1]
>  {code}
> which is highly unintuitive and has cost me almost a day of debugging now. The error is ultimately caused by the user (me!), but IMO {{org.apache.avro.mojo.AbstractAvroMojo#getIncludedFiles}} should simply throw an exception if the file path cannot be resolve to an actual directory or file.
> IMO, fail-fast behaviour when given incorrect configuration is a must for robust software, instead of getting a cryptic message from the underlying platform code.
> Proposed change attached. An alternative solution via {{file.exists()}} would also work.
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)