Issue 5081 - Make libadblockplus-android users supply the application and applicationVersion parame…

libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 61 matching lines @@
   private volatile LogSystem logSystem;
   private volatile WebRequest webRequest;
   private volatile UpdateAvailableCallback updateAvailableCallback;
   private volatile UpdateCheckDoneCallback updateCheckDoneCallback;
   private volatile FilterChangeCallback filterChangeCallback;
   private volatile ShowNotificationCallback showNotificationCallback;
   private volatile boolean elemhideEnabled;
   private volatile boolean enabled = true;
   private volatile List<String> whitelistedDomains;
 
-  public static AppInfo generateAppInfo(final Context context, boolean developmentBuild)
+  public static AppInfo generateAppInfo(final Context context, boolean developmentBuild,
+                                        String application, String applicationVersion)
   {
-    String version = "0";
-    try
-    {
-      final PackageInfo info = context.getPackageManager().getPackageInfo(context.getPackageName(), 0);
-      version = info.versionName;
-      if (developmentBuild)
-        version += "." + info.versionCode;

[anton, 2017/03/31 13:30:27]

addonVersion is generated, but we need hardcoded value

[sergei, 2017/03/31 13:49:09]

I'm not sure that we should use a hard coded value instead of a generated one.
Could we update version values in some manifest, where versionName and
versionCode are defined?

[anton, 2017/03/31 13:52:26]

it's not clear from the task. i think the intention was to get fixed expected
number. if we don't remove like i did we will get generated (unexpected)
version. 

[sergei, 2017/03/31 14:01:52]

I thought we control this version because it's our packages, don't we?

[anton, 2017/03/31 14:05:27]

no, it's app package, not library

-    }
-    catch (final NameNotFoundException e)
-    {
-      Log.e(TAG, "Failed to get the application version number", e);
-    }
     final String sdkVersion = String.valueOf(VERSION.SDK_INT);
     final String locale = Locale.getDefault().toString().replace('_', '-');
 
-    return AppInfo.builder()
-        .setVersion(version)
+    AppInfo.Builder builder =
+      AppInfo
+        .builder()
         .setApplicationVersion(sdkVersion)
         .setLocale(locale)
-        .setDevelopmentBuild(developmentBuild)
-        .build();
+        .setDevelopmentBuild(developmentBuild);
+
+    if (application != null)
+    {
+      builder.setApplication(application);
+    }
+
+    if (applicationVersion != null)
+    {
+      builder.setApplicationVersion(applicationVersion);
+    }
+
+    return builder.build();
+  }
+
+  public static AppInfo generateAppInfo(final Context context, boolean developmentBuild)

[sergei, 2017/03/31 13:49:09]

So, currently application and applicationVersion are optional, should not this
method be removed to cause a compilation error which should be a notification
for others to pass some values?

[anton, 2017/03/31 13:52:26]

there will be no compilation errors here

[sergei, 2017/03/31 14:01:52]

Then I think it should be removed.

[anton, 2017/03/31 14:05:27]

not clear what you've meant.
if the user(app developer) isn't using helper it can still use this method
without application and applicationVersion. 

[sergei, 2017/03/31 14:08:33]

That was the idea to don't allow to use this method and encourage the user to
pass values.

+  {
+    return generateAppInfo(context, developmentBuild, null, null);
   }
 
   /**
    * Builds Adblock engine
    */
   public static class Builder
   {
     private Context context;
     private Map<String, Integer> urlToResourceIdMap;
     private AndroidWebRequestResourceWrapper.Storage resourceStorage;
@@ skipping 606 matching lines @@
   public void setWhitelistedDomains(List<String> domains)
   {
     this.whitelistedDomains = domains;
   }
 
   public List<String> getWhitelistedDomains()
   {
     return whitelistedDomains;
   }
 }