Skip to content
This repository was archived by the owner on Dec 12, 2018. It is now read-only.

Don't thrown an exception if the JAR manifest does not specify the client version#1

Closed
celkins wants to merge 1 commit into
stormpath:masterfrom
celkins:unspecified-client-version
Closed

Don't thrown an exception if the JAR manifest does not specify the client version#1
celkins wants to merge 1 commit into
stormpath:masterfrom
celkins:unspecified-client-version

Conversation

@celkins

@celkins celkins commented Dec 7, 2012

Copy link
Copy Markdown

If the Stormpath client classes are embedded within another JAR (e.g., using the Maven shade plugin) and the original manifest file is not included, the client version lookup heuristic fails with an error. It seems that this version string is purely informational, given that lookupClientVersion() returns "NOT-FROM-JAR" under similar circumstances.

I have no opinion on the correct string to return in such cases, but throwing an exception seems too severe.

@lhazlewood

Copy link
Copy Markdown
Member

The NOT-FROM-JAR was originally only for testing purposes (it should be removed). The version number however is mostly required: we perform server logic based on the client versions for certain backwards compatibility concerns. It is also planned to use it in the near future for customizing media type versions served by the REST API.

Might there be another way to get this information with the Shade plugin? Perhaps create a 'Stormpath-Sdk-Implementation-Version' manifest entry that you can populate into your aggregate jar? Then we can look for that first and fall back to 'Implementation-Version' if it doesn't exist. Thoughts?

@lhazlewood

Copy link
Copy Markdown
Member

Also, we're open to other techniques for this as well - e.g. maybe populating a stormpath.sdk.version.properties file via Maven filtering at build time and then look that up at runtime. Its just that the Manifest is a particularly elegant way to retain .jar metadata (that is its purpose after all), which is why we went with that approach.

We're very much open to suggestions.

@celkins

celkins commented Dec 8, 2012

Copy link
Copy Markdown
Author

Yes, this change is certainly inappropriate if semantics are applied to the client version string. Since I'm not using my own manifest file for any purpose, I can work around the issue by adding the expected entry to my shaded JAR. However, support for an SDK-specific technique would be welcome, because declaring an incorrect implementation version for my own JAR is misleading.

How about using the standard manifest entry as the primary and falling back to a properties file in the resource path if necessary?

@celkins celkins closed this Dec 12, 2012
@lhazlewood

Copy link
Copy Markdown
Member

Hi Chris - do you not want us to try the version.properties file approach? (I was using this issue as a placeholder for that).

@celkins celkins reopened this Dec 12, 2012
@celkins

celkins commented Dec 12, 2012

Copy link
Copy Markdown
Author

Yes, please do. (Sorry. I meant to close the pull request and leave the issue open, but I guess their state is shared.)

FYI in case you haven't seen how I'm using the SDK: https://github.com/celkins/dropwizard-stormpath

@lhazlewood

Copy link
Copy Markdown
Member

Chris, this is super cool! Thanks so much for sharing - definitely let us know if you have any questions along the way.

@jarias

jarias commented Oct 16, 2013

Copy link
Copy Markdown
Contributor

This is also an issue if you bundle it in a spring-boot fat jar see: spring-projects/spring-boot#92

Doing something like:

String version = Version.class.getPackage().getImplementationVersion();

Works pretty good not sure if it would in jar created by the shade plugin since it flattens all jar deps contents into a single jar (if i'm not mistaken) but this code snippet covers all normal cases without assuming anything about the resources URL as pointed out by Dave Syer in the spring-boot ticket I created.

By the way @lhazlewood don't know if you remember me from Roundbox (trip to Ireland with Jeff and Chris) ;)

@lhazlewood

Copy link
Copy Markdown
Member

@jarias Hi Julio! Of COURSE I remember you! We had a ton of great times on that trip!

And thanks for the code snippet :) Are you using the Stormpath SDK for anything?

@lhazlewood

Copy link
Copy Markdown
Member

I got bit by this over the weekend when using Maven's 'jar-with-dependencies' artifact for an executable .jar. Definitely need the version.properties approach.

@lhazlewood

Copy link
Copy Markdown
Member

Closing - this is superseded by Issue 36

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants