Update Java client to Search API v3 #30

xSAVIKx · 2017-05-12T21:10:39Z

I've updated the Java client to the latest v3 API.

What was done:

dependencies versions were bumped to the latest ones
gradle version was bumped to the latest one
added .gitattribute in order to be sure that files have correct line endings on commits
updated the overall client classes to work with v3 API - can close Update Java client to Search API v3 [$50] #29.
added support for compression - can close HTTP compression #28.

dentarg · 2017-05-15T08:27:19Z

This is not a proper review from me, I just have some questions. @roback will review, hopefully later this week.

disable openJDK build as it fails with sigfaults

Care to elaborate on this? Do you know the reason to as it sigfaults (did you mean "segfault"?)? Does the old version have the same problem with openjdk7?

Do we really need a .gitattributes that long for this repo?

Thanks!

dentarg · 2017-05-15T08:33:38Z

README.md

+5. Run `gradlew uploadArchives -Prelease` to release to the staging repository
+6. Log into [Nexus Repository Manager](https://oss.sonatype.org/) and find the archive
+7. Read [Releasing the Deployment](http://central.sonatype.org/pages/releasing-the-deployment.html)
+8. Release to Central Repository using [Nexus Repository Manager](https://oss.sonatype.org/)


Actually, when rendered, Markdown will do the numbering, not having the order in the Markdown source makes for easier "refactoring", so we would like to keep it like that. :-)

From https://guides.github.com/features/mastering-markdown/

Source:

1. Item 1 1. Item 2 1. Item 3 1. Item 3a 1. Item 3b

Rendered:

Item 1

Item 2

Item 3

Item 3a

Item 3b

Yeah, but I mostly read raw markdown, that's why I'm always trying to use correct numbering.

If you do think it should be reverted to serie of 1. - I'd do that.

Yes, please revert to 1.s

xSAVIKx · 2017-05-15T17:31:43Z

@dentarg Yes, misprint - segfault.
I do not know the reason and actually not willing to dig into the OpenJDK. It was introduced just as an option and I don't think anyone should invest time into making it work on Travis OpenJDK.

As for the .gitattributes - it's just a generated boilerplate for any possible project. If you do want - I can clean it up.

dentarg · 2017-05-15T18:36:17Z

Yes, please cleanup .gitattributes

reverted back Query and QueryBuilder classes and introduced deprecations for old stuff updated tests and examples

xSAVIKx · 2017-05-18T19:40:51Z

@roback @dentarg Please review updates.

I've made client more backward-compatible - almost same changes as for the Python client.

roback

Noticed some minor things, but I didn't have time to look everything over. I'll continue in the beginning of next week.

Did a few requests and that worked just fine 👍 (for the requests I did a least :P).

roback · 2017-05-19T12:38:57Z

README.md

-        // Create Query object using QueryBuilder for results in English published from startTime to endTime
-        Query query = QueryBuilder.create("spotify").documentLanguage(Language.English).startTime(startTime).endTime(endTime).build();
-        Result result = client.makeRequest(query);
+        Query q = "spotify";


I think it's supposed to be String q here :)

Yeah :-) Or realy Query object.

roback · 2017-05-19T12:46:10Z

src/main/java/com/twingly/search/Query.java

+    public String toStringRepresentation() {
+        StringBuilder result = new StringBuilder(searchQuery);
+        if (lang != null) {
+            result.append(" lang: ").append(lang);


You can remove the space after the colon lang: en should be lang:en. (same for start/end-date below)

roback · 2017-05-19T13:03:38Z

src/main/java/com/twingly/search/Constants.java

@@ -12,7 +12,12 @@
    /**
     * The constant DATE_FORMAT.
     */
-    public static final String DATE_FORMAT = "yyyy-MM-dd HH:mm:ss";
+    public static final String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ssX";


X -> Z?

@roback nope.

Z in Java is for RFC standard only. And X is for ISO standard.

roback · 2017-05-19T13:21:19Z

src/main/java/com/twingly/search/Query.java

+    public String toStringRepresentation() {
+        StringBuilder result = new StringBuilder(searchQuery);
+        if (lang != null) {
+            result.append(" lang: ").append(lang);


You can remove the space after the colon. (lang: en -> lang:en)

roback · 2017-05-22T13:21:02Z

src/main/java/com/twingly/search/Query.java

+     * @return language
+     * @since 3.0.0
+     */
+    public String getLang() {


There is no need for getLang and setLang. It's enough with deprecating get/setDocumentLanguage (as you have already done below).

Ignore my comment :) You can go ahead with get/setLang. I got a bit confused as the Java client uses a query builder which none of the other clients does.

roback · 2017-05-22T13:23:31Z

src/main/java/com/twingly/search/Query.java

+     * @return the location
+     * @since 3.0.0
+     */
+    public String getLocation() {


~~Remove get/setLocation (they aren't used anywhere). Location is part of the query string.~~

Edit: Changed my mind :) We should keep this, but the location should be added to the query string in toStringRepresentation(). A test that checks that the location gets added to the query string should be added as well.

A test that checks that the location gets added to the query string should be added as well.

Not just a test for location, but a test that all attributes gets added to the query correctly.

@roback done.

added additional test to ensure query string representation is OK removed additional date format - all dates now use same format

roback · 2017-05-23T13:44:06Z

src/main/java/com/twingly/search/Constants.java

@@ -15,10 +15,6 @@
    public static final String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ssX";

    /**
-     * The constant DATE_FORMAT.
-     */
-    public static final String SEARCH_QUERY_DATE_FORMAT = "yyyy-MM-dd HH:mm:ss";


Great! Thanks for taking the time to update this as well :)

yuri-sergiichuk · 2017-05-24T07:57:48Z

Hi @roback, how it is going?

roback · 2017-05-24T16:09:20Z

Hi @roback, how it is going?

Unfortunately we have a few other things that has higher priority that we need to finish first, so I won't be able to continue with this at the moment. Sorry for keeping you waiting.

roback · 2017-05-29T09:15:29Z

example/src/main/java/SearchPostsStream.java

-        query.setStartTime(lastPost.getPublished());
-        Result newResult = client.makeRequest(query);
+        Date lastPublishedAt = lastPost.getPublishedAt();
+        q = q.toBuilder().startTime(lastPublishedAt).build();


The type of q can be changed into a QueryBuilder instead of just a Query. Then there is no need to call toBuilder().

Here we can use QueryBuilder, but we will have to call build() anyway.

roback · 2017-05-29T09:22:36Z

example/src/main/java/SearchPostsStream.java

-        Result newResult = client.makeRequest(query);
+        Date lastPublishedAt = lastPost.getPublishedAt();
+        q = q.toBuilder().startTime(lastPublishedAt).build();
+        Result newResult = client.makeRequest(q);


If a public Result makeRequest(QueryBuilder queryBuilder) method was added to the client you wouldn't have to call .build() here either.

@roback It's actually not the best idea to allow passing QueryBuilder into the client.

The reason for the Query object existence is that it is immutable. While QueryBuilder is mutable and we should try not to pass mutable data to the client (as mutability could lead to unpredictable side effects, i.e. someone will change QueryBuilder state while client is still in process of query execution).

I'd prefer to leave it as is. If you do insist - I'll change the implementation.

👍 Now that I know there is a reason for this we can leave it as is :)

roback · 2017-05-29T12:08:33Z

src/main/java/com/twingly/search/domain/Post.java

+     *
+     * @return the coordinates
+     */
+    public String getCoordinates() {


Posts containing coordinates doesn't work, it looks like it's always an empty string.

In the XML the coordinates looks like:

<coordinates> <latitude>49.1</latitude> <longitude>10.75</longitude> </coordinates>

An example query for a post containing coordinates: https://api.twingly.com/blog/search/api/v3/search?apikey=KEY&q=id:7533026945919130930

In ruby we just put them in a hash: https://github.com/twingly/twingly-search-api-ruby/blob/dbf547f182d93137bb167eb99c9eda4a8197ac21/lib/twingly/search/parser.rb#L72-L75. I don't know what works best in Java though :)

@roback There was just no info about how coordinates are represented.
I'll add mapping for coordinate a bit later.

roback · 2017-05-30T07:06:06Z

Tested the coordinates and it worked great 👍

I will merge this now.

xSAVIKx added 10 commits May 11, 2017 22:04

bump dependencies and gradle version. added .gitattributes

07053cd

bump version in order to follow API version

fa3b154

migrate client to support twingly v3 API

18e601e

updated unit tests to follow new Twingly v3 API

a0dd7f7

updated functional tests to follow Twingly v3 API

96206ef

Updated examples with API v3 usage

53d5302

updated README to reflect API v3 changes

23e286f

disable openJDK build as it fails with sigfaults

c2afe8b

remove API KEY from code base

c64a058

added test for incomplete result

7ffe083

dentarg reviewed May 15, 2017

View reviewed changes

xSAVIKx added 2 commits May 16, 2017 20:10

cleanup .gitattributes and readme

faadcc5

refactored client to be more backward-compatible

b819085

reverted back Query and QueryBuilder classes and introduced deprecations for old stuff updated tests and examples

roback requested changes May 19, 2017

View reviewed changes

xSAVIKx added 2 commits May 20, 2017 20:13

fixed README example

5d592c1

removed extra space for lang and time parameters

d09188a

roback reviewed May 22, 2017

View reviewed changes

xSAVIKx added 4 commits May 22, 2017 21:12

added missing location to the Query#toStringRepresentation

43a1b23

added additional test to ensure query string representation is OK removed additional date format - all dates now use same format

fix unit tests on Travis

7f54398

ensure date format parser has time zone

d713b0f

fixed tests

1f3c129

roback reviewed May 23, 2017

View reviewed changes

roback reviewed May 29, 2017

View reviewed changes

xSAVIKx added 2 commits May 29, 2017 23:03

Use QueryBuilder inside SearchPostsStream instead of Query

3ab01f8

Added coordinates parsing capability

72243d2

roback approved these changes May 30, 2017

View reviewed changes

roback merged commit ad80047 into twingly:master May 30, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Java client to Search API v3 #30

Update Java client to Search API v3 #30

xSAVIKx commented May 12, 2017

dentarg commented May 15, 2017 •

edited

Loading

dentarg May 15, 2017 •

edited

Loading

xSAVIKx May 15, 2017

dentarg May 15, 2017

xSAVIKx commented May 15, 2017

dentarg commented May 15, 2017

xSAVIKx commented May 18, 2017

roback left a comment

roback May 19, 2017

xSAVIKx May 20, 2017

roback May 19, 2017

xSAVIKx May 20, 2017

roback May 19, 2017

xSAVIKx May 20, 2017

roback May 19, 2017

roback May 22, 2017

roback May 22, 2017

roback May 22, 2017 •

edited

Loading

roback May 22, 2017

xSAVIKx May 22, 2017

roback May 23, 2017

yuri-sergiichuk commented May 24, 2017

roback commented May 24, 2017

roback May 29, 2017

xSAVIKx May 29, 2017

roback May 29, 2017

xSAVIKx May 29, 2017 •

edited

Loading

roback May 29, 2017

roback May 29, 2017

xSAVIKx May 29, 2017

roback commented May 30, 2017

Update Java client to Search API v3 #30

Update Java client to Search API v3 #30

Conversation

xSAVIKx commented May 12, 2017

dentarg commented May 15, 2017 • edited Loading

dentarg May 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xSAVIKx commented May 15, 2017

dentarg commented May 15, 2017

xSAVIKx commented May 18, 2017

roback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roback May 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuri-sergiichuk commented May 24, 2017

roback commented May 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xSAVIKx May 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roback commented May 30, 2017

dentarg commented May 15, 2017 •

edited

Loading

dentarg May 15, 2017 •

edited

Loading

roback May 22, 2017 •

edited

Loading

xSAVIKx May 29, 2017 •

edited

Loading