Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-19163] Android: Removal of Apache HTTP Client #7036

Merged
merged 11 commits into from Aug 25, 2015

Conversation

ashcoding
Copy link
Contributor

-Implemented HttpURLConnection for Android M to use

Ashraf added 2 commits August 6, 2015 15:35
… use

-In preparation of Apache's HTTPClient being removed in Android M
-GET and POST available
-SSL in use
-Changed replaced HTTPClientProxy and TiHTTPClient with
HttpURLConnectionProxy and TiHttpURLConnection
-Renamed the proxy back to HTTPClientProxy and TiHTTPClient
-Changed CookieProxy to use Java instead of Apache
-Progress listener added to outputstream
-Added Password and Username usage
-addAuthFactory method is not available for the new HttpClient
@ashcoding
Copy link
Contributor Author

Jira:- https://jira.appcelerator.org/browse/TIMOB-19163

Note:
Method addAuthFactory has been removed in this PR
http://appcelerator.github.io/appc-docs/latest/#!/api/Titanium.Network.HTTPClient-method-addAuthFactory

Refer to jira for method to test this.

setProperty(TiC.PROPERTY_DOMAIN, httpCookie.getDomain());
// PROPERTY_EXPIRY_DATE not used instead, PROPERTY_MAX_AGE is used
// See http://developer.android.com/reference/java/net/HttpCookie.html for more info
setProperty(TiC.PROPERTY_MAX_AGE, httpCookie.getMaxAge());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you modify the doc for this replacement property?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will modify the docs.

Ashraf added 2 commits August 18, 2015 12:51
…n to use HttpURLConnection

-Updated ImageView Doc due to changes in TiDrawableReference
@hieupham007
Copy link
Contributor

Ash, excellent work on this PR! Initial code review looks good, just one thing: I don't see any removal of Apache libraries (the jar itself). We should probably do that, along with updating the build scripts to remove any apache related libraries.

@hieupham007
Copy link
Contributor

I've ran your test app, and ran into a few problems:

  1. Test app have some layout issues when running on Galaxy Nexus (buttons overlap each other). Probably a good idea to put them in a ListView.
  2. Your test suite is quite basic, thus doesn't cover advanced cases, or regressions. Depends on what's changed, it's probably a good idea to find existing tickets that are solved in areas impacted by this PR and test them. For instance, regarding cookies, these two tickets could serve as additional test cases:
    https://jira.appcelerator.org/browse/TIMOB-14610
    https://jira.appcelerator.org/browse/TIMOB-15434
    A PR of this magnitude should have at least 10-15 tickets to test for regression.
  3. Individual test cases don't have expected results or description (i.e what should the tester expect when running the test case). Currently all test cases result in an Alert dialog, most with walls of texts. This may hinder QE from verifying expected results.

@ashcoding
Copy link
Contributor Author

Okay. I'll update the tests.

@hieupham007
Copy link
Contributor

Regression:

  1. https://jira.appcelerator.org/browse/TIMOB-15380: ticket works fine in 4.1.0.GA (location is redirected properly)
  2. https://jira.appcelerator.org/browse/TIMOB-17585: 4.1.0.GA works fine, crashes with this PR

@ashcoding
Copy link
Contributor Author

@hieupham007 Will work on those 2 working. Thanks for finding those!

@ashcoding
Copy link
Contributor Author

Fixed Regressions for:

  1. https://jira.appcelerator.org/browse/TIMOB-15380: ticket works fine in 4.1.0.GA (location is redirected properly)
  2. https://jira.appcelerator.org/browse/TIMOB-17585: 4.1.0.GA works fine, crashes with this PR

Check JIRA for updated test case for these 2.

@hieupham007
Copy link
Contributor

hieupham007 added a commit that referenced this pull request Aug 25, 2015
[TIMOB-19163] Android: Removal of Apache HTTP Client
@hieupham007 hieupham007 merged commit 9dfef4d into tidev:master Aug 25, 2015
@ashcoding
Copy link
Contributor Author

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants