laat / mvn-dl

Download maven artifacts
12 stars 11 forks source link

parseFileName fails with extension and classifier #20

Closed diabolicallabs closed 4 years ago

diabolicallabs commented 4 years ago

When passing an extension and classifier to mvn-artifact-name-parser.parseFileName it doesn't format the artifact properly. The version ends up with the classifier. The following code seems to fix the problem.

export default function parseFileName(name: string): Artifact {
  const parts = name.split(':');
  if (parts.length >= 3) {
    const artifact: Artifact = {
      groupId: parts[0],
      artifactId: parts[1],
      version: parts[2],
    };
    if (parts.length >= 4) {
      artifact.extension = parts[3];
    }
    if (parts.length >= 5) {
      artifact.classifier = parts[4];
    }
    if (artifact.version.endsWith('-SNAPSHOT')) {
      artifact.isSnapShot = true;
      artifact.version = artifact.version.substr(
        0,
        artifact.version.indexOf('-SNAPSHOT')
      );
    }
    return artifact;
  }
  throw new Error('not a maven package name. try <group>:<artifact>:<version>');
}
laat commented 4 years ago

Yes, the test for it looks wrong.

https://github.com/laat/mvn-dl/blob/73e4f8d16ae5451aa1ef076e4a0141e44a592d4c/packages/mvn-artifact-name-parser/test/test.js#L12-L18

Do you have examples that we can add as tests?

diabolicallabs commented 4 years ago

I think maybe the extension goes after the version based on the following mvn command.

// mvn org.apache.maven.plugins:maven-dependency-plugin:2.8:get -Dartifact=groupId:artifactId:version[:packaging[:classifier]]
let coordinates = mavenParser('junit:junit:4.12:jar:sources')
console.log(coordinates)
// {
//     groupId: 'junit',
//     artifactId: 'junit',
//     version: '4.12',
//     extension: 'jar',
//     classifier: 'sources'
// }
laat commented 4 years ago

It looks like there is no standard way to parse Maven coordinate strings. I do not know where I got the format from in the first place, but the current format is also used in the community:

Eclipse Aether Parser, Bazel rules_jvm_external, Apache Buildr, and maven-jaxb2-plugin are using the current syntax


As there is no standard way coordinate strings should be parsed, I'm not sure what to do. We could make a breaking change of the parser, and declare that the format used by "maven-dependency-plugin" is correct.

Or, we could document the current behaviour and define that as our pattern.

Or, implement both and let the consumers decide which pattern to use..

laat commented 4 years ago

I believe the current format is OK, since it is used by other tools in the community. And because the package is ~20 lines of code it is easy to implement others locally.

The other mvn-* packages can still be used with a custom parser.

diabolicallabs commented 4 years ago

Okay. Thanks for looking at it.