raw4j should be Android compatible

corydissinger commented 11 years ago

Should work with all major versions of Android. This means not using Jersey (which relies on the javax.ws.rs package)

Current strategy will be to implement Apache Http Components for http communication with the Reddit API. Jackson will turn JSON into Java-consumable objects.

This issue will close once Android compatibility is verified.

abonander commented 11 years ago

Exclude jackson-core-lgpl from the jackson-mapper-lgpl dependency:

<dependency>
    <groupId>org.codehaus.jackson</groupId>
    <artifactId>jackson-mapper-lgpl</artifactId>
    <version>1.9.13</version>           
    <exclusions>
        <exclusion>
            <groupId>org.codehaus.jackson</groupId>
            <artifactId>jackson-core-lgpl</artifactId>
        </exclusion>
    </exclusions>
</dependency>

Otherwise there are duplicate class files in the JAR. While the JVM ignores them, the Android dex tool (a mandatory step in the build process) rejects them and fails the build.

corydissinger commented 11 years ago

Great find. It's a tad simpler then adding the exclusion - the entire jackson-core-lgpl dependency is not needed. It provides the jackson-mapper-lgpl functionality plus a lot more.

I've removed the jackson-core-lgpl dependency so running it through dex should work now.

abonander commented 11 years ago

That's what I did on my end originally, but you had jackson-core-asl and I didn't know if you had specifically chosen that distribution. I'm guessing that's the Apache licensed version, and the other is LGPL?

corydissinger commented 11 years ago

This licensing business it tricky but important. I would like to have the least prohibitive license that would not scare away potential users.

Based on this discussion it sounds like the Apache license is more permissive in the things you can do with it, whereas the LGPL license may force applications using it to go open source.

It's starting to sound like it may be smarter for raw4j in the long run to just make it Apache licensed, so that people can do what they want with it... right now we have GNU and it seems like all that accomplishes is complication of licesning...

abonander commented 11 years ago

Yeah, I cringe when I hear of the GPL. Its terms force adoption of open-source. It's called copyleft. The Apache license, a permissive license, gives the user a choice, and I think that's more beneficial to the advancement of the cause. Open source is all about choice.

The LGPL (Lesser GPL) has a linking exception where software that uses LGPL'd libraries isn't forced to adopt the LGPL, but it's a very fine line. I like the Apache license much better.

corydissinger commented 11 years ago

For this project, Apache 2 definitely makes more sense. This issue has become "Android compatibility + License change"! :)

I've updated all the files and README accordingly. Also using the Apache licensed version of Jackson now as well.

Let me know if you can include this in an Android project. As soon as you give the official greenlight, I'll close this issue.

I'd like to get the word out to /r/Android. More eyes on the project, the better.

abonander commented 11 years ago

Well, I feel like a fool. Apparently, the HttpComponents in Android is an old, buggy version and Google has effectively (but not actually) deprecated it in lieu of java.net.HttpURLConnection. I didn't realize this until I had created the test app, tried to login, and got a NoSuchFieldError, which means different versions.

Good news is, it looks like it's almost as simple to implement.

if you want me to, I can refactor the http package to use java.net.HttpURLConnection.

corydissinger commented 11 years ago

Learning new things is never bad. :)

Based on the SO post it looks like the issue is that Apache HttpComponents HttpClient is not thread safe.

Not really understanding how your NoSuchFieldError came about based on the SO post - the way the SO post reads to me is that HttpClient is not thread safe.

Yeah, if you don't mind refactoring it (and if it truly does need to use java.net.HttpURLConnection for Android compatibility) then it would be awesome if you make a pull request.

abonander commented 11 years ago

The problem is that the Android version of HttpComponents is missing some classes/fields that the version depended upon by RAW4J needs to operate.

The SO post just resulted from a search for the version that's in Android. I wanted to override the dependency in RAW4J to the one in Android, but I decided against it since the Android version is purportedly fraught with bugs.

At least with the java.net package we're falling back to the standard lib, but a package that's available on Android as well.

I'll get right on it.

abonander commented 11 years ago

https://github.com/cybergeek94/raw4j/tree/java-net

In RedditTest it only passes testLogin(). The other tests end in NullPointerException.

I'll need to spend some time debugging.

corydissinger commented 11 years ago

Interesting. You'll definitely need to debug a little bit more. There should'nt be any NullPointerException coming out of the JSON parsing code - if you are executing the same tests, the RedditRequestResponse should be identical to the one generated using Apache Http Components.

Thanks for the haste and enthusiasm. :)

abonander commented 11 years ago

Fixed! It was stupidly simple. Just missing path (the first "/") and query ("?") separators.

All tests pass now. Opening a pull request.

abonander commented 11 years ago

Pull request opened.

corydissinger / raw4j

raw4j should be Android compatible #2