[android] Fixed review notes

This commit is contained in:
Dmitry Donskoy 2019-12-19 12:32:18 +03:00 committed by Aleksandr Zatsepin
parent 77a289d29b
commit 7b236a3283
12 changed files with 182 additions and 142 deletions

View file

@ -90,7 +90,8 @@ set(
com/mapswithme/util/GeoUtils.cpp
com/mapswithme/util/HttpClient.cpp
com/mapswithme/util/HttpUploader.cpp
com/mapswithme/util/HttpUploaderBackground.cpp
com/mapswithme/util/HttpBackgroundUploader.cpp
com/mapswithme/util/HttpUploaderUtils.cpp
com/mapswithme/util/Language.cpp
com/mapswithme/util/LoggerFactory.cpp
com/mapswithme/util/NetworkPolicy.cpp

View file

@ -27,7 +27,8 @@ jclass g_ratingClazz;
jclass g_loggerFactoryClazz;
jclass g_keyValueClazz;
jclass g_httpUploaderClazz;
jclass g_httpUploaderBackgroundClazz;
jclass g_httpPayloadClazz;
jclass g_httpBackgroundUploaderClazz;
jclass g_httpUploaderResultClazz;
jclass g_networkPolicyClazz;
jclass g_storageUtilsClazz;
@ -55,7 +56,9 @@ JNI_OnLoad(JavaVM * jvm, void *)
g_loggerFactoryClazz = jni::GetGlobalClassRef(env, "com/mapswithme/util/log/LoggerFactory");
g_keyValueClazz = jni::GetGlobalClassRef(env, "com/mapswithme/util/KeyValue");
g_httpUploaderClazz = jni::GetGlobalClassRef(env, "com/mapswithme/util/HttpUploader");
g_httpUploaderBackgroundClazz = jni::GetGlobalClassRef(env, "com/mapswithme/util/HttpBackgroundUploader");
g_httpPayloadClazz = jni::GetGlobalClassRef(env, "com/mapswithme/util/HttpPayload");
g_httpBackgroundUploaderClazz =
jni::GetGlobalClassRef(env, "com/mapswithme/util/HttpBackgroundUploader");
g_httpUploaderResultClazz = jni::GetGlobalClassRef(env, "com/mapswithme/util/HttpUploader$Result");
g_networkPolicyClazz = jni::GetGlobalClassRef(env, "com/mapswithme/util/NetworkPolicy");
g_storageUtilsClazz = jni::GetGlobalClassRef(env, "com/mapswithme/util/StorageUtils");
@ -81,7 +84,8 @@ JNI_OnUnload(JavaVM *, void *)
env->DeleteGlobalRef(g_loggerFactoryClazz);
env->DeleteGlobalRef(g_keyValueClazz);
env->DeleteGlobalRef(g_httpUploaderClazz);
env->DeleteGlobalRef(g_httpUploaderBackgroundClazz);
env->DeleteGlobalRef(g_httpPayloadClazz);
env->DeleteGlobalRef(g_httpBackgroundUploaderClazz);
env->DeleteGlobalRef(g_httpUploaderResultClazz);
env->DeleteGlobalRef(g_networkPolicyClazz);
env->DeleteGlobalRef(g_storageUtilsClazz);

View file

@ -28,7 +28,8 @@ extern jclass g_ratingClazz;
extern jclass g_loggerFactoryClazz;
extern jclass g_keyValueClazz;
extern jclass g_httpUploaderClazz;
extern jclass g_httpUploaderBackgroundClazz;
extern jclass g_httpPayloadClazz;
extern jclass g_httpBackgroundUploaderClazz;
extern jclass g_httpUploaderResultClazz;
extern jclass g_networkPolicyClazz;
extern jclass g_storageUtilsClazz;

View file

@ -0,0 +1,34 @@
#include <jni.h>
#include "com/mapswithme/core/ScopedEnv.hpp"
#include "com/mapswithme/core/ScopedLocalRef.hpp"
#include "com/mapswithme/core/jni_helper.hpp"
#include "platform/http_uploader.hpp"
#include "platform/http_uploader_background.hpp"
#include "base/assert.hpp"
#include <cstdint>
#include <functional>
#include "HttpUploaderUtils.hpp"
#include "private.h"
namespace platform
{
void HttpUploaderBackground::Upload() const
{
JNIEnv * env = jni::GetEnv();
CHECK(env, ());
HttpPayload const payload = GetPayload();
jobject uploader = platform::MakeHttpUploader(env, payload, g_httpBackgroundUploaderClazz);
jni::TScopedLocalRef uploaderRef(env, uploader);
static jmethodID const uploadId = jni::GetMethodID(env, uploaderRef.get(), "upload", "()V");
env->CallVoidMethod(uploaderRef.get(), uploadId);
jni::HandleJavaException(env);
}
} // namespace platform

View file

@ -11,6 +11,7 @@
#include <cstdint>
#include <functional>
#include "HttpUploaderUtils.hpp"
#include "private.h"
namespace
@ -42,31 +43,15 @@ HttpUploader::Result HttpUploader::Upload() const
CHECK(env, ());
static jmethodID const httpUploaderConstructor =
jni::GetConstructorID(env, g_httpUploaderClazz,
"(Ljava/lang/String;Ljava/lang/String;"
"[Lcom/mapswithme/util/KeyValue;"
"[Lcom/mapswithme/util/KeyValue;"
"Ljava/lang/String;Ljava/lang/String;Z)V");
HttpPayload const payload = GetPayload();
jni::ScopedLocalRef<jstring> const method(env, jni::ToJavaString(env, payload.m_method));
jni::ScopedLocalRef<jstring> const url(env, jni::ToJavaString(env, payload.m_url));
jni::ScopedLocalRef<jobjectArray> const params(env, jni::ToKeyValueArray(env, payload.m_params));
jni::ScopedLocalRef<jobjectArray> const headers(env,
jni::ToKeyValueArray(env, payload.m_headers));
jni::ScopedLocalRef<jstring> const fileKey(env, jni::ToJavaString(env, payload.m_fileKey));
jni::ScopedLocalRef<jstring> const filePath(env, jni::ToJavaString(env, payload.m_filePath));
jobject uploader = platform::MakeHttpUploader(env, payload, g_httpUploaderClazz);
jni::ScopedLocalRef<jobject> const uploaderRef(env, uploader);
jni::ScopedLocalRef<jobject> const httpUploaderObject(
env, env->NewObject(g_httpUploaderClazz, httpUploaderConstructor, method.get(), url.get(),
params.get(), headers.get(), fileKey.get(), filePath.get(),
static_cast<jboolean>(payload.m_needClientAuth)));
static jmethodID const uploadId = jni::GetMethodID(env, httpUploaderObject, "upload",
static jmethodID const uploadId = jni::GetMethodID(env, uploaderRef.get(), "upload",
"()Lcom/mapswithme/util/HttpUploader$Result;");
jni::ScopedLocalRef<jobject> const result(env,
env->CallObjectMethod(httpUploaderObject, uploadId));
env->CallObjectMethod(uploaderRef.get(), uploadId));
if (jni::HandleJavaException(env))
{

View file

@ -4,30 +4,19 @@
#include "com/mapswithme/core/ScopedLocalRef.hpp"
#include "com/mapswithme/core/jni_helper.hpp"
#include "platform/http_uploader_background.hpp"
#include "base/assert.hpp"
#include <cstdint>
#include <functional>
#include "private.h"
#include "platform/http_uploader.hpp"
namespace platform
{
void HttpUploaderBackground::Upload() const
jobject ToJavaHttpPayload(JNIEnv *env, const platform::HttpPayload payload)
{
JNIEnv* env = jni::GetEnv();
CHECK(env, ());
static jmethodID const httpUploaderConstructor =
jni::GetConstructorID(env, g_httpUploaderBackgroundClazz,
static jmethodID const constructor =
jni::GetConstructorID(env, g_httpPayloadClazz,
"(Ljava/lang/String;Ljava/lang/String;"
"[Lcom/mapswithme/util/KeyValue;"
"[Lcom/mapswithme/util/KeyValue;"
"Ljava/lang/String;Ljava/lang/String;Z)V");
HttpPayload const payload = GetPayload();
jni::ScopedLocalRef<jstring> const method(env, jni::ToJavaString(env, payload.m_method));
jni::ScopedLocalRef<jstring> const url(env, jni::ToJavaString(env, payload.m_url));
jni::ScopedLocalRef<jobjectArray> const params(env, jni::ToKeyValueArray(env, payload.m_params));
@ -36,16 +25,18 @@ void HttpUploaderBackground::Upload() const
jni::ScopedLocalRef<jstring> const fileKey(env, jni::ToJavaString(env, payload.m_fileKey));
jni::ScopedLocalRef<jstring> const filePath(env, jni::ToJavaString(env, payload.m_filePath));
jni::ScopedLocalRef<jobject> const httpUploaderObject(
env, env->NewObject(g_httpUploaderBackgroundClazz, httpUploaderConstructor, method.get(), url.get(),
params.get(), headers.get(), fileKey.get(), filePath.get(),
static_cast<jboolean>(payload.m_needClientAuth)));
return env->NewObject(g_httpPayloadClazz, constructor, method.get(), url.get(), params.get(),
headers.get(), fileKey.get(), filePath.get(),
static_cast<jboolean>(payload.m_needClientAuth));
}
static jmethodID const uploadId = jni::GetMethodID(env, httpUploaderObject, "upload", "()V");
env->CallVoidMethod(httpUploaderObject, uploadId);
jobject MakeHttpUploader(JNIEnv * env, const platform::HttpPayload payload, jclass uploaderClass)
{
static jmethodID const httpUploaderConstructor =
jni::GetConstructorID(env, uploaderClass, "(Lcom/mapswithme/util/HttpPayload;)V");
jni::TScopedLocalRef javaPayloadRef(env, ToJavaHttpPayload(env, payload));
return env->NewObject(uploaderClass, httpUploaderConstructor, javaPayloadRef.get());
}
} // namespace platform
extern "C"
{
}

View file

@ -0,0 +1,8 @@
#pragma once
#include "jni.h"
#include "platform/http_payload.hpp"
namespace platform
{
jobject MakeHttpUploader(JNIEnv * env, const platform::HttpPayload payload, jclass uploaderClass);
}

View file

@ -10,6 +10,7 @@ com/mapswithme/util/HttpClient$Params.class
com/mapswithme/util/HttpClient.class
com/mapswithme/util/HttpUploader$Result.class
com/mapswithme/util/HttpUploader.class
com/mapswithme/util/HttpPayload.class
com/mapswithme/util/HttpBackgroundUploader.class
com/mapswithme/util/KeyValue.class
com/mapswithme/util/log/LoggerFactory.class

View file

@ -2,74 +2,19 @@ package com.mapswithme.util;
import androidx.annotation.NonNull;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
public abstract class AbstractHttpUploader
{
@NonNull
private final String mMethod;
@NonNull
private final String mUrl;
@NonNull
private final List<KeyValue> mParams;
@NonNull
private final List<KeyValue> mHeaders;
@NonNull
private final String mFileKey;
@NonNull
private final String mFilePath;
private final HttpPayload mPayload;
private final boolean mNeedClientAuth;
AbstractHttpUploader(@NonNull String method, @NonNull String url,
@NonNull KeyValue[] params,
@NonNull KeyValue[] headers, @NonNull String fileKey,
@NonNull String filePath,
boolean needClientAuth)
AbstractHttpUploader(@NonNull HttpPayload payload)
{
mMethod = method;
mUrl = url;
mFileKey = fileKey;
mFilePath = filePath;
mParams = new ArrayList<>(Arrays.asList(params));
mHeaders = new ArrayList<>(Arrays.asList(headers));
mNeedClientAuth = needClientAuth;
mPayload = payload;
}
@NonNull
protected String getMethod() {
return mMethod;
public HttpPayload getPayload()
{
return mPayload;
}
@NonNull
protected String getUrl() {
return mUrl;
}
@NonNull
protected List<KeyValue> getParams() {
return mParams;
}
@NonNull
protected List<KeyValue> getHeaders() {
return mHeaders;
}
@NonNull
protected String getFileKey() {
return mFileKey;
}
@NonNull
protected String getFilePath() {
return mFilePath;
}
protected boolean needClientAuth() {
return mNeedClientAuth;
}
}

View file

@ -4,14 +4,9 @@ import androidx.annotation.NonNull;
public class HttpBackgroundUploader extends AbstractHttpUploader
{
public HttpBackgroundUploader(@NonNull String method, @NonNull String url,
@NonNull KeyValue[] params,
@NonNull KeyValue[] headers, @NonNull String fileKey,
@NonNull String filePath,
boolean needClientAuth)
public HttpBackgroundUploader(@NonNull HttpPayload payload)
{
super(method, url, params, headers, fileKey, filePath, needClientAuth);
super(payload);
}
public void upload()

View file

@ -0,0 +1,78 @@
package com.mapswithme.util;
import androidx.annotation.NonNull;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
public class HttpPayload
{
@NonNull
private final String mMethod;
@NonNull
private final String mUrl;
@NonNull
private final List<KeyValue> mParams;
@NonNull
private final List<KeyValue> mHeaders;
@NonNull
private final String mFileKey;
@NonNull
private final String mFilePath;
private final boolean mNeedClientAuth;
public HttpPayload(@NonNull String method, @NonNull String url, @NonNull KeyValue[] params,
@NonNull KeyValue[] headers, @NonNull String fileKey, @NonNull String filePath,
boolean needClientAuth)
{
mMethod = method;
mUrl = url;
mParams = Collections.unmodifiableList(Arrays.asList(params));
mHeaders = Collections.unmodifiableList(Arrays.asList(headers));
mFileKey = fileKey;
mFilePath = filePath;
mNeedClientAuth = needClientAuth;
}
@NonNull
public String getMethod()
{
return mMethod;
}
@NonNull
public String getUrl()
{
return mUrl;
}
@NonNull
public List<KeyValue> getParams()
{
return mParams;
}
@NonNull
public List<KeyValue> getHeaders()
{
return mHeaders;
}
@NonNull
public String getFileKey()
{
return mFileKey;
}
@NonNull
public String getFilePath()
{
return mFilePath;
}
public boolean needClientAuth()
{
return mNeedClientAuth;
}
}

View file

@ -3,15 +3,12 @@ package com.mapswithme.util;
import android.os.Build;
import android.text.TextUtils;
import android.util.Base64;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import com.mapswithme.maps.BuildConfig;
import com.mapswithme.maps.Framework;
import com.mapswithme.util.log.Logger;
import com.mapswithme.util.log.LoggerFactory;
import java.io.BufferedReader;
import java.io.File;
import java.io.FileInputStream;
@ -24,7 +21,7 @@ import java.io.PrintWriter;
import java.net.HttpURLConnection;
import java.net.URL;
import java.net.URLConnection;
import java.util.List;
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLSocketFactory;
@ -42,11 +39,9 @@ public final class HttpUploader extends AbstractHttpUploader
@NonNull
private final String mEndPart;
public HttpUploader(@NonNull String method, @NonNull String url, @NonNull KeyValue[] params,
@NonNull KeyValue[] headers, @NonNull String fileKey, @NonNull String filePath,
boolean needClientAuth)
public HttpUploader(@NonNull HttpPayload payload)
{
super(method, url, params, headers, fileKey, filePath, needClientAuth);
super(payload);
mBoundary = "----" + System.currentTimeMillis();
mEndPart = LINE_FEED + "--" + mBoundary + "--" + LINE_FEED;
}
@ -60,14 +55,14 @@ public final class HttpUploader extends AbstractHttpUploader
HttpURLConnection connection = null;
try
{
URL url = new URL(getUrl());
URL url = new URL(getPayload().getUrl());
connection = (HttpURLConnection) url.openConnection();
connection.setConnectTimeout(Constants.CONNECTION_TIMEOUT_MS);
connection.setReadTimeout(Constants.READ_TIMEOUT_MS);
connection.setUseCaches(false);
connection.setRequestMethod(getMethod());
connection.setDoOutput(getMethod().equals("POST"));
if ("https".equals(connection.getURL().getProtocol()) && needClientAuth())
connection.setRequestMethod(getPayload().getMethod());
connection.setDoOutput(getPayload().getMethod().equals("POST"));
if ("https".equals(connection.getURL().getProtocol()) && getPayload().needClientAuth())
{
HttpsURLConnection httpsConnection = (HttpsURLConnection) connection;
String cert = HttpUploader.nativeUserBindingCertificate();
@ -77,18 +72,19 @@ public final class HttpUploader extends AbstractHttpUploader
httpsConnection.setSSLSocketFactory(socketFactory);
}
long fileSize = StorageUtils.getFileSize(getFilePath());
long fileSize = StorageUtils.getFileSize(getPayload().getFilePath());
StringBuilder paramsBuilder = new StringBuilder();
fillBodyParams(paramsBuilder);
File file = new File(getFilePath());
fillFileParams(paramsBuilder, getFileKey(), file);
File file = new File(getPayload().getFilePath());
fillFileParams(paramsBuilder, getPayload().getFileKey(), file);
int endPartSize = mEndPart.getBytes().length;
int paramsSize = paramsBuilder.toString().getBytes().length;
long bodyLength = paramsSize + fileSize + endPartSize;
setStreamingMode(connection, bodyLength);
setHeaders(connection, bodyLength);
long startTime = System.currentTimeMillis();
LOGGER.d(TAG, "Start bookmarks upload on url: '" + Utils.makeUrlSafe(getUrl()) + "'");
LOGGER.d(
TAG, "Start bookmarks upload on url: '" + Utils.makeUrlSafe(getPayload().getUrl()) + "'");
OutputStream outputStream = connection.getOutputStream();
writer = new PrintWriter(new OutputStreamWriter(outputStream, CHARSET));
writeParams(writer, paramsBuilder);
@ -105,7 +101,7 @@ public final class HttpUploader extends AbstractHttpUploader
}
catch (IOException e)
{
message = "I/O exception '" + Utils.makeUrlSafe(getUrl()) + "'";
message = "I/O exception '" + Utils.makeUrlSafe(getPayload().getUrl()) + "'";
if (connection != null)
{
String errMsg = readErrorResponse(connection);
@ -182,17 +178,18 @@ public final class HttpUploader extends AbstractHttpUploader
private void setHeaders(@NonNull URLConnection connection, long bodyLength)
{
getHeaders().add(new KeyValue(HttpClient.HEADER_USER_AGENT, Framework.nativeGetUserAgent()));
getHeaders().add(new KeyValue("App-Version", BuildConfig.VERSION_NAME));
getHeaders().add(new KeyValue("Content-Type", "multipart/form-data; boundary=" + mBoundary));
getHeaders().add(new KeyValue("Content-Length", String.valueOf(bodyLength)));
for (KeyValue header : getHeaders())
List<KeyValue> headers = getPayload().getHeaders();
headers.add(new KeyValue(HttpClient.HEADER_USER_AGENT, Framework.nativeGetUserAgent()));
headers.add(new KeyValue("App-Version", BuildConfig.VERSION_NAME));
headers.add(new KeyValue("Content-Type", "multipart/form-data; boundary=" + mBoundary));
headers.add(new KeyValue("Content-Length", String.valueOf(bodyLength)));
for (KeyValue header : headers)
connection.setRequestProperty(header.mKey, header.mValue);
}
private void fillBodyParams(@NonNull StringBuilder builder)
{
for (KeyValue field : getParams())
for (KeyValue field : getPayload().getParams())
addParam(builder, field.mKey, field.mValue);
}