Skip to content
This repository has been archived by the owner on Jun 13, 2024. It is now read-only.

OkHttp for courier strategies #18

Merged
merged 13 commits into from
Jul 14, 2020
Merged

Conversation

norkator
Copy link
Owner

All of them are tested except GLS

Looks good and got response with requested test tracking code
Cainiao worked, tested with real package/tracking number.
Active tracking is broken because server returns You don't have permission to access ... on this server.
Yay! it started to work, url had changed and also needed header changes
No idea does it work but changed url to new one too, getting response but doubting it's working. Needs code for testing.
Postnord had changed their api url too
@norkator norkator linked an issue Jul 13, 2020 that may be closed by this pull request
DHLExpress needed proper headers to work.
@norkator norkator linked an issue Jul 13, 2020 that may be closed by this pull request
ristomatti
ristomatti previously approved these changes Jul 13, 2020
Copy link
Collaborator

@ristomatti ristomatti left a comment

Choose a reason for hiding this comment

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

I'm no Android developer but the changes look pretty straightforward to me. Added couple of comments for consideration but I'm not even sure myself if they'd be over engineering in this case.

app/build.gradle Show resolved Hide resolved
try {
SSLContext sc = SSLContext.getInstance("SSL");
sc.init(null, trustAllCerts, new java.security.SecureRandom());
HttpsURLConnection.setDefaultSSLSocketFactory(sc.getSocketFactory());
String url = "https://www.r-kioski.fi/wordpress/wp-admin/admin-ajax.php";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unrelated but maybe add this as a constant for tidyness?

Copy link
Owner Author

@norkator norkator Jul 13, 2020

Choose a reason for hiding this comment

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

Like this? c2c3f42
I left those ones intact which parts are constructed like url + tracking number + some latter part

OkHttpClient client = new OkHttpClient();
Request request = new Request.Builder()
.url(url)
.addHeader("User-Agent", "Mozilla/5.0")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a particular reason for this UA string to be different than the one for ArraPakettiStrategy? Could it be a constant somew

Copy link
Owner Author

@norkator norkator Jul 13, 2020

Choose a reason for hiding this comment

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

a42b595 how about now? same agent, tested to be working. As this kind of constant, is it okay?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Much cleaner!

}
// Close connection
in.close();
String url = "https://www.dhl.com/DatPublic/search.do?autoSearch=true&l=fi&directDownload=XML&statusHistory=true&search=consignmentId&a=" + parcelCode;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Constant here also?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Should I make part "https://www.dhl.com/DatPublic/search.do?autoSearch=true&l=fi&directDownload=XML&statusHistory=true&search=consignmentId&a=" private static final string? and then later append url + parcelCode? is that what you mean?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Didn't check so carefully but yep it'd more consistent.

@developerfromjokela
Copy link
Collaborator

At first glance looks good 😎.

@norkator norkator merged commit 8880478 into master Jul 14, 2020
@norkator norkator deleted the Feature/okhttp-implementation branch July 14, 2020 13:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
3 participants