Use Okhttp for jsoup connect and move jsoup class in to crawler #705
Conversation
|
My 2ct:
|
|
der MVHttpClient wird in den neuen Crawlern bisher nicht verwendet, deshalb wird durch den Einbau an dieser Stelle nichts gewonnen. Zumal OkHttp durch dem Umbau weiterhin nur für das Lesen von HTML-Seiten verwendet wird, beiREST-APIs wird |
|
Die Punkte 1 bis 4 habe ich hinzugefügt. |
| httpResponseCode = response.code(); | ||
| if (response.isSuccessful()) { | ||
| if (response.body() != null) { | ||
| responseString = response.body().string(); |
There was a problem hiding this comment.
Der responseString kann direkt return werden.
| if (response.body() != null) { | ||
| responseString = response.body().string(); | ||
| } | ||
| break; |
There was a problem hiding this comment.
Anstatt der beiden breaks lieber einen if(response.body() == null || httpRepsonseCode == 404 || httpResponseCode == 410) und darin den reponse string auf leer setzen. Im else fall den reponse string auf den body setzen wie in Zeile 46. Nach dem if den response string returnen. So werden die beiden breaks eingespart und die Komplexität gesenkt.
| int retry = 0; | ||
| int httpResponseCode = 0; | ||
| String responseString = ""; | ||
| while (retry < 3) { |
There was a problem hiding this comment.
Ich denke, eine do-while wäre hier verständlicher.
| } | ||
|
|
||
| public Document getDocument(String url) throws IOException { | ||
| return getConnection(url).get(); | ||
| public String getString(String url) throws IOException { |
There was a problem hiding this comment.
getString ist recht nichts sagend, vorallem wenn man es dann im Crawler liest: crawler.getConnection().getString(
Zusätzlich verwirrt das get. Man könnte denken es wird nur eine variable ausgelesen und nicht eine Verbindung aufgebaut. Wie wäre es mit requestBody oder readUrlContent?
There was a problem hiding this comment.
rename von getXYZ auf requestBodyAsXYZ
src/main/java/de/mediathekview/mserver/crawler/basic/AbstractCrawler.java
Show resolved
Hide resolved
|
@codingPF Der Build für den PR scheitert mit einem Compile-Fehler. Kannst du dir diesen anschauen? |
| final ResponseBody body = response.body()) { | ||
| httpResponseCode = response.code(); | ||
| if (response.isSuccessful()) { | ||
| if (response.body() != null) { |
There was a problem hiding this comment.
Die beiden ifs könnte man noch zu einem vereinen. ;)
src/main/java/de/mediathekview/mserver/base/webaccess/JsoupConnection.java
Show resolved
Hide resolved
Die Okttp Änderung ist in der JsoupConnection Klasse passiert.
Die Connection ist in den Crawler gewandert und wird dort mit den SenderConfig initializiert.
Testfälle sind angepasst.