mGerrit: fix Diff Dialogs across AOSP, AOKP & Gerrit-Review

Now using Apache Commons Codec v1.8 for Base64 conversions Updated Volley jar to latest codebase Renamed several Apache classes to avoid conflicts with with android.util.Base64 Cleaned up some commented code Bumped to version 2.02.001 Patch Set 2: Add Apache Commons Codec to Preferences Patch Set 3: Bump version to v2.02.002 Change-Id: I45f94e390e47c8d56681b692dec5807ed41c954c
JBirdVegas · May 29, 2013 · f415626 · f415626
1 parent 3da6d9e
commit f415626
Show file tree

Hide file tree

Showing 199 changed files with 22,674 additions and 106 deletions.
diff --git a/AndroidManifest.xml b/AndroidManifest.xml
@@ -18,8 +18,8 @@
 
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
           package="com.jbirdvegas.mgerrit"
-          android:versionCode="201001"
-          android:versionName="2.01.001">
+          android:versionCode="202002"
+          android:versionName="2.02.002">
 
     <uses-sdk android:minSdkVersion="16"
         android:targetSdkVersion="17"/>

diff --git a/build.xml b/build.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<project name="GerritControllerActivity" default="help">
+<project name="mGerrit" default="help">
 
     <!-- The local.properties file is created and updated by the 'android' tool.
          It contains the path to the SDK. It should *NOT* be checked into

diff --git a/libs/volley.jar b/libs/volley.jar
diff --git a/res/layout/diff_dialog.xml b/res/layout/diff_dialog.xml
@@ -1,22 +1,21 @@
 <?xml version="1.0" encoding="utf-8"?>
+<ScrollView xmlns:android="http://schemas.android.com/apk/res/android"
+            android:layout_width="match_parent"
+            android:layout_height="match_parent"
+            android:fillViewport="true"
+            android:id="@+id/scrollView">
 
-<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
-              android:orientation="vertical"
-              android:layout_width="match_parent"
-              android:layout_height="match_parent">
-
-    <ScrollView
+    <HorizontalScrollView
         android:layout_width="match_parent"
-        android:layout_height="match_parent"
-        android:fillViewport="true"
-        android:id="@+id/scrollView">
+        android:layout_height="wrap_content"
+        android:fillViewport="true">
 
         <TextView
             android:paddingTop="@dimen/diff_view_padding_top"
             android:padding="@dimen/default_left_card_start"
-            android:layout_width="match_parent"
+            android:layout_width="wrap_content"
             android:layout_height="match_parent"
             android:id="@+id/diff_view_diff"
             android:layout_gravity="left"/>
-    </ScrollView>
-</LinearLayout>
+    </HorizontalScrollView>
+</ScrollView>
diff --git a/res/layout/patchset_file_changed_list_item.xml b/res/layout/patchset_file_changed_list_item.xml
@@ -57,8 +57,7 @@
             android:paddingLeft="8dip"
             android:paddingRight="8dip"
             android:textAppearance="@style/CardText"
-            android:textSize="13sp"
-            android:textAlignment="viewEnd"/>
+            android:textSize="13sp"/>
     </LinearLayout>
     <LinearLayout
         android:orientation="horizontal"
@@ -84,7 +83,6 @@
             android:paddingLeft="8dip"
             android:paddingRight="8dip"
             android:textAppearance="@style/CardText"
-            android:textSize="13sp"
-            android:textAlignment="viewEnd"/>
+            android:textSize="13sp"/>
     </LinearLayout>
-</LinearLayout>
+</LinearLayout>
diff --git a/res/values/strings.xml b/res/values/strings.xml
@@ -93,4 +93,8 @@
     </string>
     <string name="please_select_update_for_range">Please select file to view changelog</string>
     <string name="download_rom_zip">Download ROM zip</string>
+    <string name="failed_to_decode_base64">Failed to decode the Base64 response.</string>
+    <string name="return_was_null">Responce was null... Please view in browser</string>
+    <string name="apache_commons_title">Apache Commons Codec v1.8</string>
+    <string name="apache_commons_website">http://commons.apache.org/proper/commons-codec/</string>
 </resources>
diff --git a/res/xml/prefs.xml b/res/xml/prefs.xml
@@ -41,5 +41,10 @@
             android:key="open_source_aosp_volley"
             android:title="@string/aosp_volley_title"
             android:summary="@string/aosp_volley_website"/>
+
+        <Preference
+            android:key="open_source_apache_commons"
+            android:title="@string/apache_commons_title"
+            android:summary="@string/apache_commons_website"/>
     </PreferenceCategory>
 </PreferenceScreen>
diff --git a/src/com/jbirdvegas/mgerrit/Prefs.java b/src/com/jbirdvegas/mgerrit/Prefs.java
@@ -32,6 +32,7 @@ public class Prefs extends PreferenceActivity implements Preference.OnPreference
     private static final CharSequence CARDS_UI_KEY = "open_source_lib_cards_ui";
     private static final CharSequence NINE_OLD_ANDROIDS_KEY = "open_source_lib_nine_old_androids";
     private static final CharSequence AOSP_VOLLEY = "open_source_aosp_volley";
+    private static final CharSequence APACHE_COMMONS_KEY = "open_source_apache_commons";
     private static final String GERRIT_KEY = "gerrit_instances_key";
 
     @Override
@@ -42,8 +43,10 @@ protected void onCreate(Bundle savedInstanceState) {
         findPreference(CARDS_UI_KEY).setOnPreferenceClickListener(this);
         // View NineOldAndroids website
         findPreference(NINE_OLD_ANDROIDS_KEY).setOnPreferenceClickListener(this);
-        // View Koush's website
+        // View AOSP's Volley website
         findPreference(AOSP_VOLLEY).setOnPreferenceClickListener(this);
+        // View Apache Commons Codec
+        findPreference(APACHE_COMMONS_KEY).setOnPreferenceClickListener(this);
         // select gerrit instance
         ListPreference gerritList = (ListPreference) findPreference(GERRIT_KEY);
         gerritList.setSummary(gerritList.getValue());

diff --git a/src/com/jbirdvegas/mgerrit/cards/PatchSetChangesCard.java b/src/com/jbirdvegas/mgerrit/cards/PatchSetChangesCard.java
@@ -120,17 +120,14 @@ private View generateChangedFileView(final ChangedFile changedFile, final Contex
             public void onClick(final View view) {
                 AlertDialog.Builder ad = new AlertDialog.Builder(mCardsActivity)
                         .setTitle(R.string.choose_diff_view);
-                // bug will be fixed don't abandon hope quite yet
-                //if (Prefs.getCurrentGerrit(context).equals(
-                //        context.getResources().getStringArray(R.array.gerrit_webaddresses)[0])) {
-
+                // TODO XXX ABANDONED till APIs are stable on Google's side :(
                 ad.setPositiveButton(R.string.context_menu_view_diff_dialog, new DialogInterface.OnClickListener() {
                     @Override
                     public void onClick(DialogInterface dialogInterface, int i) {
                         // http://gerrit.aokp.co/changes/AOKP%2Fexternal_jbirdvegas_mGerrit~master~I0d360472ee328c6cde0f8303b19a35f175869e68/revisions/current/patch
                         // or
                         // after v2.8 goes stable (returns Base64 encoded String)
-// curl https://gerrit-review.googlesource.com/changes/gerrit~master~Idc97af3d01999889d9b1a818fbd1bbe0b274dcf3/revisions/77e974c7070e274aaca3f2413a3fb53031d0f50e/files/ReleaseNotes%2fReleaseNotes-2.5.3.txt/content
+                        // curl https://gerrit-review.googlesource.com/changes/gerrit~master~Idc97af3d01999889d9b1a818fbd1bbe0b274dcf3/revisions/77e974c7070e274aaca3f2413a3fb53031d0f50e/files/ReleaseNotes%2fReleaseNotes-2.5.3.txt/content
                         ///changes/{change-id}/revisions/{revision-id}/files/{file-id}/content
                         String base = "%schanges/%s/revisions/current/patch";
                         String base64 = "%schanges/%s/revisions/%s/files/%s/content";
@@ -147,6 +144,7 @@ public void onClick(DialogInterface dialogInterface, int i) {
                         R.string.context_menu_diff_view_in_browser, new DialogInterface.OnClickListener() {
                     @Override
                     public void onClick(DialogInterface dialogInterface, int i) {
+                        // launch in webbrowser
                         String base = "%s#/c/%d/%d/%s";
                         Intent browserIntent = new Intent(
                                 Intent.ACTION_VIEW, Uri.parse(String.format(base,
@@ -163,6 +161,7 @@ public void onClick(DialogInterface dialogInterface, int i) {
         return innerRootView;
     }
 
+    // creates the Diff viewer dialog
     private void launchDiffDialog(String url, ChangedFile changedFile) {
         Log.d(TAG, "Attempting to contact: " + url);
         DiffDialog diffDialog = new DiffDialog(mCardsActivity, url, changedFile);

diff --git a/src/com/jbirdvegas/mgerrit/dialogs/DiffDialog.java b/src/com/jbirdvegas/mgerrit/dialogs/DiffDialog.java
@@ -4,6 +4,7 @@
 import android.content.Context;
 import android.graphics.Typeface;
 import android.text.SpannableString;
+import android.util.Base64;
 import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.View;
@@ -18,6 +19,7 @@
 import com.jbirdvegas.mgerrit.helpers.Base64Coder;
 import com.jbirdvegas.mgerrit.objects.ChangedFile;
 import com.jbirdvegas.mgerrit.objects.Diff;
+import org.apache.commons.codec.binary.ApacheBase64;
 
 import java.util.regex.Pattern;
 
@@ -45,18 +47,101 @@ public DiffDialog(Context context, String website, ChangedFile changedFile) {
         mRootView = mInflater.inflate(R.layout.diff_dialog, null);
         setView(mRootView);
         mDiffTextView = (TextView) mRootView.findViewById(R.id.diff_view_diff);
-        Log.d(TAG, "Calling url: " + mUrl); //+ "/b");
+        Log.d(TAG, "Calling url: " + mUrl);
         if (DIFF_DEBUG) {
             debugRestDiffApi(context, mUrl, mChangedFile);
         }
         // we can use volley here because we return
         // does not contain the magic number on the
         // first line. return is just the Base64 formatted
         // return
-        mRequestQueue.add(getBase64gRequest(mUrl, "/b"));
+        mRequestQueue.add(getBase64StringRequest(mUrl));
         mRequestQueue.start();
     }
 
+    private StringRequest getBase64StringRequest(final String weburl) {
+        return new StringRequest(weburl,
+                new Response.Listener<String>() {
+                    @Override
+                    public void onResponse(String base64) {
+                        String decoded = workAroundBadBase(base64);
+                        if (DIFF_DEBUG) {
+                            Log.d(TAG, "[DEBUG-MODE]\n"
+                                    + "url: " + weburl
+                                    + "\n==================================="
+                                    + decoded
+                                    + "====================================");
+                        }
+                        setTextView(decoded);
+                    }
+                },
+                new Response.ErrorListener() {
+                    @Override
+                    public void onErrorResponse(VolleyError volleyError) {
+                        Log.e(TAG, "Failed to download the diff", volleyError);
+                    }
+                }
+        );
+    }
+
+    private String workAroundBadBase(String baseString) {
+        if (baseString == null) {
+            return getContext().getString(R.string.return_was_null);
+        }
+        String failMessage = "Failed to decode Base64 using: ";
+        try {
+            return new String(ApacheBase64.decodeBase64(baseString));
+        } catch (IllegalArgumentException badBase) {
+            Log.e(TAG, failMessage + "org.apache.commons.codec.binary.ApacheBase64", badBase);
+        }
+        try {
+            return new String(Base64.decode(baseString.getBytes(), Base64.URL_SAFE | Base64.NO_PADDING));
+        } catch (IllegalArgumentException badBase) {
+            Log.e(TAG, failMessage + "android.util.Base64", badBase);
+        }
+        try {
+            return new String(Base64Coder.decode(baseString));
+        } catch (IllegalArgumentException badBase) {
+            Log.e(TAG, failMessage + "com.jbirdvegas.mgerrit.helpers.Base64Coder", badBase);
+        }
+        return getContext().getString(R.string.failed_to_decode_base64);
+    }
+
+    private void setTextView(String result) {
+        Pattern pattern = Pattern.compile("\\Qdiff --git \\E");
+        String[] filesChanged = pattern.split(result);
+        StringBuilder builder = new StringBuilder(0);
+        Diff currentDiff = null;
+        for (String change : filesChanged) {
+            String concat;
+            try {
+                concat = change.substring(2, change.lastIndexOf(mChangedFile.getPath())).trim();
+                concat = concat.split(" ")[0];
+            } catch (StringIndexOutOfBoundsException notFound) {
+                Log.d(TAG, notFound.getMessage());
+                continue;
+            }
+            if (concat.equals(mChangedFile.getPath())) {
+                builder.append(DIFF);
+                change.replaceAll("\n", mLineSplit);
+                currentDiff = new Diff(getContext(), change);
+                builder.append(change);
+            }
+        }
+        if (builder.length() == 0) {
+            builder.append("Diff not found!");
+        }
+        mDiffTextView.setTypeface(Typeface.MONOSPACE);
+        // rebuild text; required to respect the \n
+        SpannableString spannableString = currentDiff.getColorizedSpan();
+        if (spannableString != null) {
+            mDiffTextView.setText(currentDiff.getColorizedSpan(), TextView.BufferType.SPANNABLE);
+        } else {
+            mDiffTextView.setText("Failed to load diff :(");
+        }
+
+    }
+
     private void debugRestDiffApi(Context context, String mUrl, ChangedFile mChangedFile) {
         RequestQueue requestQueue = Volley.newRequestQueue(context);
         Log.d(TAG, "Targeting changed file: " + mChangedFile);
@@ -85,92 +170,18 @@ public void onResponse(String s) {
                                         + "\n"
                                         + "url: " + weburl
                                         + "\n==================================="
-                                        + Base64Coder.decodeString(s)
+                                        + new String(Base64.decode(s, Base64.URL_SAFE | Base64.NO_PADDING))
                                         + "===================================="
                                 );
                             }
                         },
                         new Response.ErrorListener() {
                             @Override
                             public void onErrorResponse(VolleyError volleyError) {
-                                Log.e(TAG, "[DEBUG-MODE]\n"
-                                        + "Volley Failed!!!", volleyError);
-                            }
-                        }
-                );
-        return debugRequest;
-    }
-
-    private Request getBase64gRequest(String url, String arg) {
-        // seems a bug prevents the args from being respected???
-        // See here:
-        // https://groups.google.com/forum/?fromgroups#!topic/repo-discuss/xmFCHbD4Z0Q
-        String limitContextLines = "&o=context:2";
-        final String args = arg + limitContextLines;
-        final String weburl = url + args;
-        Request debugRequest =
-                new StringRequest(weburl,
-                        new Response.Listener<String>() {
-                            @Override
-                            public void onResponse(String s) {
-                                if (DIFF_DEBUG) {
-                                    Log.d(TAG, "[DEBUG-MODE]\n" +
-                                            "Decoded Response for args {" + args + '}'
-                                            + "\n"
-                                            + "url: " + weburl
-                                            + "\n==================================="
-                                            + Base64Coder.decodeString(s)
-                                            + "===================================="
-                                    );
-                                }
-                                setTextView(Base64Coder.decodeString(s));
-                            }
-                        },
-                        new Response.ErrorListener() {
-                            @Override
-                            public void onErrorResponse(VolleyError volleyError) {
-                                Log.e(TAG, "Failed to download the diff", volleyError);
+                                Log.e(TAG, "Debuging Volley Failed!!!", volleyError);
                             }
                         }
                 );
         return debugRequest;
     }
-
-    private void setTextView(String result) {
-        //textView.setText(result);
-        // return;
-
-        Pattern pattern = Pattern.compile("\\Qdiff --git \\E");
-        String[] filesChanged = pattern.split(result);
-        StringBuilder builder = new StringBuilder(0);
-        Diff currentDiff = null;
-        for (String change : filesChanged) {
-            String concat;
-            try {
-                concat = change.substring(2, change.lastIndexOf(mChangedFile.getPath())).trim();
-                concat = concat.split(" ")[0];
-            } catch (StringIndexOutOfBoundsException notFound) {
-                Log.d(TAG, notFound.getMessage());
-                continue;
-            }
-            if (concat.equals(mChangedFile.getPath())) {
-                builder.append(DIFF);
-                change.replaceAll("\n", mLineSplit);
-                currentDiff = new Diff(getContext(), change);
-                builder.append(change);
-            }
-        }
-        if (builder.length() == 0) {
-            builder.append("Diff not found!");
-        }
-        mDiffTextView.setTypeface(Typeface.MONOSPACE);
-        // rebuild text; required to respect the \n
-        SpannableString spannableString = currentDiff.getColorizedSpan();
-        if (spannableString != null) {
-            mDiffTextView.setText(currentDiff.getColorizedSpan(), TextView.BufferType.SPANNABLE);
-        } else {
-            mDiffTextView.setText("Failed to load diff :(");
-        }
-
-    }
-}
+}
diff --git a/src/com/jbirdvegas/mgerrit/helpers/GravatarHelper.java b/src/com/jbirdvegas/mgerrit/helpers/GravatarHelper.java
@@ -73,7 +73,7 @@ public static void attachGravatarToTextView(final TextView textView, String emai
         imageRequest.add(new ImageRequest(url, new Response.Listener<Bitmap>() {
             @Override
             public void onResponse(Bitmap bitmap) {
-                textView.setCompoundDrawablesRelativeWithIntrinsicBounds(
+                textView.setCompoundDrawablesWithIntrinsicBounds(
                         new BitmapDrawable(bitmap),
                         null,
                         null,