From 5e09e31e63ab8f759e03f5b1b90dab88d317ba4c Mon Sep 17 00:00:00 2001 From: Dmitry Donskoy Date: Mon, 24 Dec 2018 15:59:45 +0300 Subject: [PATCH] [android] Fixed review notes --- android/AndroidManifest.xml | 2 +- .../com/mapswithme/maps/LightFramework.cpp | 21 +++-- .../com/mapswithme/maps/MwmApplication.java | 2 +- .../maps/bookmarks/data/FeatureId.java | 7 -- .../com/mapswithme/maps/geofence/Factory.java | 16 ++++ .../maps/geofence/GeoFenceFeature.java | 80 ++++++------------- .../GeofenceTransitionsIntentService.java | 7 +- 7 files changed, 61 insertions(+), 74 deletions(-) create mode 100644 android/src/com/mapswithme/maps/geofence/Factory.java diff --git a/android/AndroidManifest.xml b/android/AndroidManifest.xml index 1e8e66a673..3a0fc57b57 100644 --- a/android/AndroidManifest.xml +++ b/android/AndroidManifest.xml @@ -569,7 +569,7 @@ + android:exported="false"/> (framework.GetNumberOfUnsentUGC()); } +jobject CreateFeatureId(JNIEnv * env, CampaignFeature const & data) +{ + static jmethodID const featureCtorId = + jni::GetConstructorID(env, g_featureIdClazz, "(Ljava/lang/String;JI)V"); + + jni::TScopedLocalRef const countryId(env, jni::ToJavaString(env, data.m_countryId)); + return env->NewObject(g_featureIdClazz, featureCtorId, countryId.get(), + static_cast(data.m_mwmVersion), + static_cast(data.m_featureIndex)); +} + JNIEXPORT jobjectArray JNICALL Java_com_mapswithme_maps_LightFramework_nativeGetLocalAdsFeatures(JNIEnv * env, jclass clazz, jdouble lat, jdouble lon, @@ -34,19 +45,17 @@ Java_com_mapswithme_maps_LightFramework_nativeGetLocalAdsFeatures(JNIEnv * env, static jclass const geoFenceFeatureClazz = jni::GetGlobalClassRef(env, "com/mapswithme/maps/geofence/GeoFenceFeature"); - // Java signature : GeoFenceFeature(long mwmVersion, String countryId, int featureIndex, + // Java signature : GeoFenceFeature(FeatureId featureId, // double latitude, double longitude) static jmethodID const geoFenceFeatureConstructor = - jni::GetConstructorID(env, geoFenceFeatureClazz, "(JLjava/lang/String;IDD)V"); + jni::GetConstructorID(env, geoFenceFeatureClazz, "(Lcom/mapswithme/maps/bookmarks/data/FeatureId;DD)V"); return jni::ToJavaArray(env, geoFenceFeatureClazz, features, [&](JNIEnv * jEnv, CampaignFeature const & data) { - jni::TScopedLocalRef const countryId(env, jni::ToJavaString(env, data.m_countryId)); + jni::TScopedLocalRef const featureId(env, CreateFeatureId(env, data)); return env->NewObject(geoFenceFeatureClazz, geoFenceFeatureConstructor, - static_cast(data.m_mwmVersion), - countryId.get(), - static_cast(data.m_featureIndex), + featureId.get(), static_cast(data.m_lat), static_cast(data.m_lon)); }); diff --git a/android/src/com/mapswithme/maps/MwmApplication.java b/android/src/com/mapswithme/maps/MwmApplication.java index 079c5fd236..a7b61d12dc 100644 --- a/android/src/com/mapswithme/maps/MwmApplication.java +++ b/android/src/com/mapswithme/maps/MwmApplication.java @@ -154,9 +154,9 @@ public class MwmApplication extends Application public void onCreate() { super.onCreate(); - mBackgroundListener = new AppBaseTransitionListener(this); LoggerFactory.INSTANCE.initialize(this); mLogger = LoggerFactory.INSTANCE.getLogger(LoggerFactory.Type.MISC); + mBackgroundListener = new AppBaseTransitionListener(this); getLogger().d(TAG, "Application is created"); mMainLoopHandler = new Handler(getMainLooper()); mMediator = new ExternalLibrariesMediator(this); diff --git a/android/src/com/mapswithme/maps/bookmarks/data/FeatureId.java b/android/src/com/mapswithme/maps/bookmarks/data/FeatureId.java index ad9ffe96de..b283ff4436 100644 --- a/android/src/com/mapswithme/maps/bookmarks/data/FeatureId.java +++ b/android/src/com/mapswithme/maps/bookmarks/data/FeatureId.java @@ -116,11 +116,4 @@ public class FeatureId implements Parcelable { return mMwmName + ":" + mMwmVersion + ":" + mFeatureIndex; } - - @NonNull - public static FeatureId from(@NonNull Geofence geofence) - { - String requestId = geofence.getRequestId(); - return fromString(requestId); - } } diff --git a/android/src/com/mapswithme/maps/geofence/Factory.java b/android/src/com/mapswithme/maps/geofence/Factory.java new file mode 100644 index 0000000000..fc0753c7d8 --- /dev/null +++ b/android/src/com/mapswithme/maps/geofence/Factory.java @@ -0,0 +1,16 @@ +package com.mapswithme.maps.geofence; + +import android.support.annotation.NonNull; + +import com.google.android.gms.location.Geofence; +import com.mapswithme.maps.bookmarks.data.FeatureId; + +class Factory +{ + @NonNull + public static FeatureId from(@NonNull Geofence geofence) + { + String requestId = geofence.getRequestId(); + return FeatureId.fromString(requestId); + } +} diff --git a/android/src/com/mapswithme/maps/geofence/GeoFenceFeature.java b/android/src/com/mapswithme/maps/geofence/GeoFenceFeature.java index 0bb85d6754..f838761ea6 100644 --- a/android/src/com/mapswithme/maps/geofence/GeoFenceFeature.java +++ b/android/src/com/mapswithme/maps/geofence/GeoFenceFeature.java @@ -11,39 +11,18 @@ import com.mapswithme.maps.bookmarks.data.FeatureId; */ public class GeoFenceFeature implements Parcelable { - private final long mwmVersion; @NonNull - private final String countryId; - private final int featureIndex; + private final FeatureId mFeatureId; private final double latitude; private final double longitude; - public GeoFenceFeature(long mwmVersion, @NonNull String countryId, int featureIndex, - double latitude, double longitude) + public GeoFenceFeature(@NonNull FeatureId featureId, double latitude, double longitude) { - this.mwmVersion = mwmVersion; - this.countryId = countryId; - this.featureIndex = featureIndex; + mFeatureId = featureId; this.latitude = latitude; this.longitude = longitude; } - public long getMwmVersion() - { - return mwmVersion; - } - - @NonNull - public String getCountryId() - { - return countryId; - } - - public int getFeatureIndex() - { - return featureIndex; - } - public double getLatitude() { return latitude; @@ -54,32 +33,10 @@ public class GeoFenceFeature implements Parcelable return longitude; } - @Override - public boolean equals(Object o) - { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - GeoFenceFeature that = (GeoFenceFeature) o; - - if (mwmVersion != that.mwmVersion) return false; - if (featureIndex != that.featureIndex) return false; - return countryId.equals(that.countryId); - } - - @Override - public int hashCode() - { - int result = (int) (mwmVersion ^ (mwmVersion >>> 32)); - result = 31 * result + countryId.hashCode(); - result = 31 * result + featureIndex; - return result; - } - @NonNull public FeatureId getId() { - return new FeatureId(countryId, mwmVersion, featureIndex); + return mFeatureId; } @Override @@ -91,18 +48,14 @@ public class GeoFenceFeature implements Parcelable @Override public void writeToParcel(Parcel dest, int flags) { - dest.writeLong(this.mwmVersion); - dest.writeString(this.countryId); - dest.writeInt(this.featureIndex); + dest.writeParcelable(this.mFeatureId, flags); dest.writeDouble(this.latitude); dest.writeDouble(this.longitude); } protected GeoFenceFeature(Parcel in) { - this.mwmVersion = in.readLong(); - this.countryId = in.readString(); - this.featureIndex = in.readInt(); + this.mFeatureId = in.readParcelable(FeatureId.class.getClassLoader()); this.latitude = in.readDouble(); this.longitude = in.readDouble(); } @@ -126,12 +79,27 @@ public class GeoFenceFeature implements Parcelable public String toString() { final StringBuilder sb = new StringBuilder("GeoFenceFeature{"); - sb.append("mwmVersion=").append(mwmVersion); - sb.append(", countryId='").append(countryId).append('\''); - sb.append(", featureIndex=").append(featureIndex); + sb.append("mFeatureId=").append(mFeatureId); sb.append(", latitude=").append(latitude); sb.append(", longitude=").append(longitude); sb.append('}'); return sb.toString(); } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + GeoFenceFeature that = (GeoFenceFeature) o; + + return mFeatureId.equals(that.mFeatureId); + } + + @Override + public int hashCode() + { + return mFeatureId.hashCode(); + } } diff --git a/android/src/com/mapswithme/maps/geofence/GeofenceTransitionsIntentService.java b/android/src/com/mapswithme/maps/geofence/GeofenceTransitionsIntentService.java index 39ce4d32e0..9d74eca88d 100644 --- a/android/src/com/mapswithme/maps/geofence/GeofenceTransitionsIntentService.java +++ b/android/src/com/mapswithme/maps/geofence/GeofenceTransitionsIntentService.java @@ -38,7 +38,7 @@ public class GeofenceTransitionsIntentService extends JobIntentService @Override protected void onHandleWork(@NonNull Intent intent) { - LOG.d(TAG, "onHandleWork"); + LOG.d(TAG, "onHandleWork. Intent = " + intent); GeofencingEvent geofencingEvent = GeofencingEvent.fromIntent(intent);; if (geofencingEvent.hasError()) onError(geofencingEvent); @@ -76,7 +76,7 @@ public class GeofenceTransitionsIntentService extends JobIntentService } catch (InterruptedException e) { - LOG.e(TAG, "error", e); + LOG.e(TAG, "Failed to make location probe for '" + geofencingEvent + "\'", e); } } @@ -112,6 +112,7 @@ public class GeofenceTransitionsIntentService extends JobIntentService { int id = JobIdMap.getId(GeofenceTransitionsIntentService.class); enqueueWork(context, GeofenceTransitionsIntentService.class, id, intent); + LOG.d(TAG, "Service was enqueued"); } private static class CheckLocationTask extends AbstractGeofenceTask @@ -139,7 +140,7 @@ public class GeofenceTransitionsIntentService extends JobIntentService GeofenceLocation geofenceLocation = getGeofenceLocation(); for (Geofence each : mGeofences) { - FeatureId feature = FeatureId.from(each); + FeatureId feature = Factory.from(each); LightFramework.logLocalAdsEvent(geofenceLocation, feature); } }