Uploaded image for project: 'Apache Avro'
  1. Apache Avro
  2. AVRO-3582

Avro Maven plug in should fail if imports do not resolve to actual file/directory

    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Patch Available
    • Minor
    • Resolution: Unresolved
    • 1.10.2, 1.12.0
    • None
    • java, tools
    • None

    Description

      org.apache.avro.mojo.AbstractAvroMojo#getIncludedFiles contains the following code:

      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 } 

      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.:

      <imports>
        <!-- note the erroneous extra / at the beginning -->
        <import>/${basedir}/src/main/resources/schemas/</import>
      </imports>

      Then both checks at line no. 258 and 260 evaluate to false, meaning that in line no. 264 the code:

      fs.addExclude(null);
      

      is executed.

      This leads to the behaviour that the following condition in the org.apache.maven.shared.model.fileset.util.FileSetManager#scan code:

      656 if ( excludesArray.length > 0 )
      657 {
      658     scanner.setExcludes( excludesArray );
      659 } 

      evaluates to true, causing an exception to occur in  org.apache.maven.shared.utils.io.DirectoryScanner#setExcludes

      357 pattern = excludes[i].trim().replace( '/', File.separatorChar ).replace( '\\', File.separatorChar );

      with the Maven error:

      [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]
       

      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. Strictly speaking, throwing a FileNotFoundException would be more semantically correct, but I did not wish to add more checked exceptions. The current one is already rethrown by all calling methods.

      If the change seems OK, I can include a test case as well (should be very simple).

      Attachments

        Activity

          People

            Unassigned Unassigned
            fpanovski Filip
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated: