Scrooge maven plugin is overly restrictive

[fzakaria]

I believe the maven plugin for scrooge is too restrictive, especially for cases where not all thrift model dependencies have the idl classifier

Here is the relevant code from AbstractMavenScroogeMojo

    for (Artifact artifact : deps) {
      // This artifact has an idl classifier.
      if (isIdlCalssifier(artifact, classifier)) {
        thriftDependencies.add(artifact);
      } else {
        if (isDepOfIdlArtifact(artifact, depsMap)) {
          // Fetch idl artifact for dependency of an idl artifact.
          try {
            Artifact idlArtifact = MavenScroogeCompilerUtil.getIdlArtifact(
              artifact,
              artifactFactory,
              artifactResolver,
              localRepository,
              remoteArtifactRepositories,
              classifier);
            thriftDependencies.add(idlArtifact);
          } catch (MojoExecutionException e) {
            /* Do nothing as this artifact is not an idl artifact
             binary jars may have dependency on thrift lib etc.
             */
            getLog().debug("Could not fetch idl jar for " + artifact);
          }
        }
      }
    }
    return thriftDependencies;
  }

Specifically consider the use-case where someone has a Maven artfiact ThriftDemoModel with the classifier idl.
This artifact depends on finatra-thrift_2.11 accordingly so that it can make use of finatra-thrift/finatra_thrift_exceptions.thrift

The code above will hit the MavenScroogeCompilerUtil however it will attempt to use the classifier idl and in this particular case, finatra-thrift is not published with any classifier.

The solution to have the classifier as empty or null won't work either because it doesn't allow mixing & matching of dependencies that may /may not use the classifier.

Solution

A better solution would be once the code has determined isIdlCalssifier, it should get all transitive dependencies and simply add them to thriftDependencies

Additional bug

There is an additional bug with the current solution. When performing isDepOfIdlArtifact it uses the getDependencyTrail method. The problem is, if a dependency of an IDL package AND the current root module, the dependencyTrail will be the shortest path and not even include the IDL dependency.