Better group support

* change group methods to less specific values
* new account settings version: change group method to CATEGORIES for updated accounts
* change group method from CATEGORIES to GROUP_VCARDS automatically when a group VCard is received

GUI:
* AccountSettings: disable CalDAV/CardDAV options when the corresponding service is not available
* AccountSettings: new option to choose contact group method
* account setup: allow to choose contact group method at account creation

app/src/main/java/at/bitfire/davdroid/AccountSettings.java

@@ -50,7 +50,7 @@ import lombok.Cleanup;
 import okhttp3.HttpUrl;
 
 public class AccountSettings {
-    private final static int CURRENT_VERSION = 3;
+    private final static int CURRENT_VERSION = 4;
     private final static String
             KEY_SETTINGS_VERSION = "version",
 
@@ -80,10 +80,8 @@ public class AccountSettings {
     private final static String KEY_MANAGE_CALENDAR_COLORS = "manage_calendar_colors";
 
     /** Contact group method:
-        automatic  VCard4 if server supports VCard 4, VCard3 otherwise (default value)
+        value = null (not existing)     groups as separate VCards (default)
-        VCard3     adds a contact's groups to its CATEGORIES / interprets a contact's CATEGORIES as groups
+                "CATEGORIES"            groups are per-contact CATEGORIES
-        VCard4     uses groups as defined in VCard 4 (KIND/MEMBER properties)
-        Apple      uses Apple-proprietary X-ADDRESSBOOK-KIND/-MEMBER properties
     */
     private final static String KEY_CONTACT_GROUP_METHOD = "contact_group_method";
 
@@ -240,11 +238,11 @@ public class AccountSettings {
         final String name = accountManager.getUserData(account, KEY_CONTACT_GROUP_METHOD);
         return name != null ?
                 GroupMethod.valueOf(name) :
-                GroupMethod.AUTOMATIC;
+                GroupMethod.GROUP_VCARDS;
     }
 
     public void setGroupMethod(@NonNull GroupMethod method) {
-        final String name = GroupMethod.AUTOMATIC.equals(method) ? null : method.name();
+        final String name = method == GroupMethod.GROUP_VCARDS ? null : method.name();
         accountManager.setUserData(account, KEY_CONTACT_GROUP_METHOD, name);
     }
 
@@ -424,6 +422,11 @@ public class AccountSettings {
         accountManager.setUserData(account, KEY_SETTINGS_VERSION, "3");
     }
 
+    @SuppressWarnings({ "Recycle", "unused" })
+    private void update_3_4() {
+        setGroupMethod(GroupMethod.CATEGORIES);
+    }
+
 
     public static class AppUpdatedReceiver extends BroadcastReceiver {
 

app/src/main/java/at/bitfire/davdroid/resource/LocalAddressBook.java

@@ -164,6 +164,14 @@ public class LocalAddressBook extends AndroidAddressBook implements LocalCollect
                 group.delete();
     }
 
+    public void removeGroups() throws ContactsStorageException {
+        try {
+            provider.delete(syncAdapterURI(Groups.CONTENT_URI), null, null);
+        } catch(RemoteException e) {
+            throw new ContactsStorageException("Couldn't remove all groups", e);
+        }
+    }
+
 
     // SYNC STATE
 

app/src/main/java/at/bitfire/davdroid/resource/LocalGroup.java

@@ -19,6 +19,7 @@ import android.provider.ContactsContract.CommonDataKinds.GroupMembership;
 import android.provider.ContactsContract.Groups;
 import android.provider.ContactsContract.RawContacts;
 import android.provider.ContactsContract.RawContacts.Data;
+import android.provider.ContactsContract.RawContactsEntity;
 
 import org.apache.commons.lang3.ArrayUtils;
 

app/src/main/java/at/bitfire/davdroid/syncadapter/ContactsSyncManager.java

@@ -83,13 +83,13 @@ import okhttp3.ResponseBody;
  * <p></p>Group handling differs according to the {@link #groupMethod}. There are two basic methods to
  * handle/manage groups:</p>
  * <ul>
- *     <li>VCard3 {@code CATEGORIES}: groups memberships are attached to each contact and represented as
+ *     <li>{@code CATEGORIES}: groups memberships are attached to each contact and represented as
  *     "category". When a group is dirty or has been deleted, all its members have to be set to
  *     dirty, too (because they have to be uploaded without the respective category). This
  *     is done in {@link #prepareDirty()}. Empty groups can be deleted without further processing,
  *     which is done in {@link #postProcess()} because groups may become empty after downloading
  *     updated remoted contacts.</li>
- *     <li>VCard4-style: individual and group contacts (with a list of member UIDs) are
+ *     <li>Groups as separate VCards: individual and group contacts (with a list of member UIDs) are
  *     distinguished. When a local group is dirty, its members don't need to be set to dirty.
  *     <ol>
  *         <li>However, when a contact is dirty, it has
@@ -171,11 +171,9 @@ public class ContactsSyncManager extends SyncManager {
         App.log.info("Server advertises VCard/4 support: " + hasVCard4);
 
         groupMethod = settings.getGroupMethod();
-        if (GroupMethod.AUTOMATIC.equals(groupMethod))
-            groupMethod = hasVCard4 ? GroupMethod.VCARD4 : GroupMethod.VCARD3_CATEGORIES;
         App.log.info("Contact group method: " + groupMethod);
 
-        localAddressBook().includeGroups = !GroupMethod.VCARD3_CATEGORIES.equals(groupMethod);
+        localAddressBook().includeGroups = groupMethod == GroupMethod.GROUP_VCARDS;
     }
 
     @Override
@@ -184,13 +182,14 @@ public class ContactsSyncManager extends SyncManager {
 
         LocalAddressBook addressBook = localAddressBook();
 
-        if (GroupMethod.VCARD3_CATEGORIES.equals(groupMethod)) {
+        if (groupMethod == GroupMethod.CATEGORIES) {
-            /* VCard3 group handling: groups memberships are represented as contact CATEGORIES */
+            /* groups memberships are represented as contact CATEGORIES */
 
             // groups with DELETED=1: set all members to dirty, then remove group
             for (LocalGroup group : addressBook.getDeletedGroups()) {
-                App.log.fine("Removing group " + group + " and marking its members as dirty");
+                App.log.fine("Finally removing group " + group);
-                group.markMembersDirty();
+                // useless because Android deletes group memberships as soon as a group is set to DELETED:
+                // group.markMembersDirty();
                 group.delete();
             }
 
@@ -201,7 +200,7 @@ public class ContactsSyncManager extends SyncManager {
                 group.clearDirty(null);
             }
         } else {
-            /* VCard4 group handling: there are group contacts and individual contacts */
+            /* groups as separate VCards: there are group contacts and individual contacts */
 
             // mark groups with changed members as dirty
             BatchOperation batch = new BatchOperation(addressBook.provider);
@@ -231,8 +230,8 @@ public class ContactsSyncManager extends SyncManager {
             LocalContact local = ((LocalContact)resource);
             contact = local.getContact();
 
-            if (groupMethod == GroupMethod.VCARD3_CATEGORIES) {
+            if (groupMethod == GroupMethod.CATEGORIES) {
-                // VCard3: add groups as CATEGORIES
+                // add groups as CATEGORIES
                 for (long groupID : local.getGroupMemberships()) {
                     try {
                         @Cleanup Cursor c = provider.query(
@@ -362,7 +361,7 @@ public class ContactsSyncManager extends SyncManager {
 
     @Override
     protected void postProcess() throws CalendarStorageException, ContactsStorageException {
-        if (groupMethod == GroupMethod.VCARD3_CATEGORIES) {
+        if (groupMethod == GroupMethod.CATEGORIES) {
             /* VCard3 group handling: groups memberships are represented as contact CATEGORIES */
 
             // remove empty groups
@@ -399,6 +398,13 @@ public class ContactsSyncManager extends SyncManager {
 
         final Contact newData = contacts[0];
 
+        if (groupMethod == GroupMethod.CATEGORIES && newData.group) {
+            groupMethod = GroupMethod.GROUP_VCARDS;
+            App.log.warning("Received group VCard although group method is CATEGORIES. Deleting all groups; new group method: " + groupMethod);
+            localAddressBook().removeGroups();
+            settings.setGroupMethod(groupMethod);
+        }
+
         // update local contact, if it exists
         LocalResource local = localResources.get(fileName);
         if (local != null) {
@@ -446,7 +452,7 @@ public class ContactsSyncManager extends SyncManager {
             syncResult.stats.numInserts++;
         }
 
-        if (groupMethod == GroupMethod.VCARD3_CATEGORIES && local instanceof LocalContact) {
+        if (groupMethod == GroupMethod.CATEGORIES && local instanceof LocalContact) {
             // VCard3: update group memberships from CATEGORIES
             LocalContact contact = (LocalContact)local;
 

app/src/main/java/at/bitfire/davdroid/ui/AccountSettingsActivity.java

@@ -99,7 +99,8 @@ public class AccountSettingsActivity extends AppCompatActivity {
                 @Override
                 public boolean onPreferenceChange(Preference preference, Object newValue) {
                     settings.username((String)newValue);
-                    refresh(); return false;
+                    refresh();
+                    return false;
                 }
             });
 
@@ -108,7 +109,8 @@ public class AccountSettingsActivity extends AppCompatActivity {
                 @Override
                 public boolean onPreferenceChange(Preference preference, Object newValue) {
                     settings.password((String)newValue);
-                    refresh(); return false;
+                    refresh();
+                    return false;
                 }
             });
 
@@ -118,7 +120,8 @@ public class AccountSettingsActivity extends AppCompatActivity {
                 @Override
                 public boolean onPreferenceChange(Preference preference, Object newValue) {
                     settings.preemptiveAuth((Boolean)newValue);
-                    refresh(); return false;
+                    refresh();
+                    return false;
                 }
             });
 
@@ -135,7 +138,8 @@ public class AccountSettingsActivity extends AppCompatActivity {
                     @Override
                     public boolean onPreferenceChange(Preference preference, Object newValue) {
                         settings.setSyncInterval(ContactsContract.AUTHORITY, Long.parseLong((String)newValue));
-                        refresh(); return false;
+                        refresh();
+                        return false;
                     }
                 });
             } else {
@@ -155,7 +159,8 @@ public class AccountSettingsActivity extends AppCompatActivity {
                     @Override
                     public boolean onPreferenceChange(Preference preference, Object newValue) {
                         settings.setSyncInterval(CalendarContract.AUTHORITY, Long.parseLong((String)newValue));
-                        refresh(); return false;
+                        refresh();
+                        return false;
                     }
                 });
             } else {
@@ -175,7 +180,8 @@ public class AccountSettingsActivity extends AppCompatActivity {
                     @Override
                     public boolean onPreferenceChange(Preference preference, Object newValue) {
                         settings.setSyncInterval(TaskProvider.ProviderName.OpenTasks.authority, Long.parseLong((String)newValue));
-                        refresh(); return false;
+                        refresh();
+                        return false;
                     }
                 });
             } else {
@@ -189,7 +195,8 @@ public class AccountSettingsActivity extends AppCompatActivity {
                 @Override
                 public boolean onPreferenceChange(Preference preference, Object wifiOnly) {
                     settings.setSyncWiFiOnly((Boolean)wifiOnly);
-                    refresh(); return false;
+                    refresh();
+                    return false;
                 }
             });
 
@@ -205,23 +212,45 @@ public class AccountSettingsActivity extends AppCompatActivity {
                 public boolean onPreferenceChange(Preference preference, Object newValue) {
                     String ssid = (String)newValue;
                     settings.setSyncWifiOnlySSID(!TextUtils.isEmpty(ssid) ? ssid : null);
-                    refresh(); return false;
+                    refresh();
+                    return false;
                 }
             });
 
             // category: CardDAV
             final SwitchPreferenceCompat prefRFC6868 = (SwitchPreferenceCompat)findPreference("vcard_rfc6868");
+            if (syncIntervalContacts != null) {
                 prefRFC6868.setChecked(settings.getVCardRFC6868());
                 prefRFC6868.setOnPreferenceChangeListener(new Preference.OnPreferenceChangeListener() {
                     @Override
                     public boolean onPreferenceChange(Preference preference, Object o) {
                         settings.setVCardRFC6868((Boolean)o);
-                    refresh(); return false;
+                        refresh();
+                        return false;
                     }
                 });
+            } else
+                prefRFC6868.setEnabled(false);
+
+            final ListPreference prefGroupMethod = (ListPreference)findPreference("contact_group_method");
+            if (syncIntervalContacts != null) {
+                prefGroupMethod.setValue(settings.getGroupMethod().name());
+                prefGroupMethod.setSummary(prefGroupMethod.getEntry());
+                prefGroupMethod.setOnPreferenceChangeListener(new Preference.OnPreferenceChangeListener() {
+                    @Override
+                    public boolean onPreferenceChange(Preference preference, Object o) {
+                        String name = (String)o;
+                        settings.setGroupMethod(GroupMethod.valueOf(name));
+                        refresh();
+                        return false;
+                    }
+                });
+            } else
+                prefGroupMethod.setEnabled(false);
 
             // category: CalDAV
             final EditTextPreference prefTimeRangePastDays = (EditTextPreference)findPreference("time_range_past_days");
+            if (syncIntervalCalendars != null) {
                 Integer pastDays = settings.getTimeRangePastDays();
                 if (pastDays != null) {
                     prefTimeRangePastDays.setText(pastDays.toString());
@@ -240,33 +269,28 @@ public class AccountSettingsActivity extends AppCompatActivity {
                             days = -1;
                         }
                         settings.setTimeRangePastDays(days < 0 ? null : days);
-                    refresh(); return false;
+                        refresh();
+                        return false;
                     }
                 });
+            } else
+                prefTimeRangePastDays.setEnabled(false);
 
             final SwitchPreferenceCompat prefManageColors = (SwitchPreferenceCompat)findPreference("manage_calendar_colors");
+            if (syncIntervalCalendars != null || syncIntervalTasks != null) {
                 prefManageColors.setChecked(settings.getManageCalendarColors());
                 prefManageColors.setOnPreferenceChangeListener(new Preference.OnPreferenceChangeListener() {
                     @Override
                     public boolean onPreferenceChange(Preference preference, Object newValue) {
                         settings.setManageCalendarColors((Boolean)newValue);
-                    refresh(); return false;
+                        refresh();
+                        return false;
                     }
                 });
-
+            } else
-            // category: CardDAV
+                prefManageColors.setEnabled(false);
-            final ListPreference prefGroupMethod = (ListPreference)findPreference("contact_group_method");
-            prefGroupMethod.setValue(settings.getGroupMethod().name());
-            prefGroupMethod.setSummary(prefGroupMethod.getEntry());
-            prefGroupMethod.setOnPreferenceChangeListener(new Preference.OnPreferenceChangeListener() {
-                @Override
-                public boolean onPreferenceChange(Preference preference, Object o) {
-                    String name = (String)o;
-                    settings.setGroupMethod(GroupMethod.valueOf(name));
-                    refresh(); return false;
-                }
-            });
-        }
 
         }
     }
+
+}

app/src/main/java/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.java

@@ -25,6 +25,7 @@ import android.view.View;
 import android.view.ViewGroup;
 import android.widget.Button;
 import android.widget.EditText;
+import android.widget.Spinner;
 
 import java.net.URI;
 import java.util.logging.Level;
@@ -42,6 +43,7 @@ import at.bitfire.davdroid.model.ServiceDB.OpenHelper;
 import at.bitfire.davdroid.model.ServiceDB.Services;
 import at.bitfire.davdroid.resource.LocalTaskList;
 import at.bitfire.ical4android.TaskProvider;
+import at.bitfire.vcard4android.GroupMethod;
 import lombok.Cleanup;
 
 public class AccountDetailsFragment extends Fragment {
@@ -49,6 +51,9 @@ public class AccountDetailsFragment extends Fragment {
     private static final String KEY_CONFIG = "config";
     private static final int DEFAULT_SYNC_INTERVAL = 4 * 3600;  // 4 hours
 
+    Spinner spnrGroupMethod;
+
+
     public static AccountDetailsFragment newInstance(DavResourceFinder.Configuration config) {
         AccountDetailsFragment frag = new AccountDetailsFragment();
         Bundle args = new Bundle(1);
@@ -57,6 +62,7 @@ public class AccountDetailsFragment extends Fragment {
         return frag;
     }
 
+
     @Override
     public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
         final View v = inflater.inflate(R.layout.login_account_details, container, false);
@@ -74,6 +80,10 @@ public class AccountDetailsFragment extends Fragment {
         final EditText editName = (EditText)v.findViewById(R.id.account_name);
         editName.setText(config.userName);
 
+        // CardDAV-specific
+        v.findViewById(R.id.carddav).setVisibility(config.cardDAV != null ? View.VISIBLE : View.GONE);
+        spnrGroupMethod = (Spinner)v.findViewById(R.id.contact_group_method);
+
         Button btnCreate = (Button)v.findViewById(R.id.create_account);
         btnCreate.setOnClickListener(new View.OnClickListener() {
             @Override
@@ -114,25 +124,39 @@ public class AccountDetailsFragment extends Fragment {
             Intent refreshIntent = new Intent(getActivity(), DavService.class);
             refreshIntent.setAction(DavService.ACTION_REFRESH_COLLECTIONS);
 
-            db.beginTransactionNonExclusive();
             if (config.cardDAV != null) {
+                // insert CardDAV service
                 long id = insertService(db, accountName, Services.SERVICE_CARDDAV, config.cardDAV);
+
+                // start CardDAV service detection (refresh collections)
                 refreshIntent.putExtra(DavService.EXTRA_DAV_SERVICE_ID, id);
                 getActivity().startService(refreshIntent);
 
+                // initial CardDAV account settings
+                int idx = spnrGroupMethod.getSelectedItemPosition();
+                String groupMethodName = getResources().getStringArray(R.array.settings_contact_group_method_values)[idx];
+                settings.setGroupMethod(GroupMethod.valueOf(groupMethodName));
+
+                // enable contact sync
                 ContentResolver.setIsSyncable(account, ContactsContract.AUTHORITY, 1);
                 settings.setSyncInterval(ContactsContract.AUTHORITY, DEFAULT_SYNC_INTERVAL);
             } else
+                // disable contact sync when CardDAV is not available
                 ContentResolver.setIsSyncable(account, ContactsContract.AUTHORITY, 0);
 
             if (config.calDAV != null) {
+                // insert CalDAV service
                 long id = insertService(db, accountName, Services.SERVICE_CALDAV, config.calDAV);
+
+                // start CalDAV service detection (refresh collections)
                 refreshIntent.putExtra(DavService.EXTRA_DAV_SERVICE_ID, id);
                 getActivity().startService(refreshIntent);
 
+                // enable calendar sync
                 ContentResolver.setIsSyncable(account, CalendarContract.AUTHORITY, 1);
                 settings.setSyncInterval(CalendarContract.AUTHORITY, DEFAULT_SYNC_INTERVAL);
 
+                // enable task sync, if possible
                 if (Build.VERSION.SDK_INT >= 23 || LocalTaskList.tasksProviderAvailable(getContext())) {
                     ContentResolver.setIsSyncable(account, TaskProvider.ProviderName.OpenTasks.authority, 1);
                     settings.setSyncInterval(TaskProvider.ProviderName.OpenTasks.authority, DEFAULT_SYNC_INTERVAL);
@@ -141,15 +165,13 @@ public class AccountDetailsFragment extends Fragment {
                     // because otherwise, there will be a non-catchable SecurityException as soon as OpenTasks is installed
                     ContentResolver.setIsSyncable(account, TaskProvider.ProviderName.OpenTasks.authority, 0);
             } else {
+                // disable calendar and task sync when CalDAV is not available
                 ContentResolver.setIsSyncable(account, CalendarContract.AUTHORITY, 0);
                 ContentResolver.setIsSyncable(account, TaskProvider.ProviderName.OpenTasks.authority, 0);
             }
 
-            db.setTransactionSuccessful();
         } catch(InvalidAccountException e) {
             App.log.log(Level.SEVERE, "Couldn't access account settings", e);
-        } finally {
-            db.endTransaction();
         }
 
         return true;

app/src/main/java/at/bitfire/davdroid/ui/setup/LoginCredentialsFragment.java

@@ -87,6 +87,7 @@ public class LoginCredentialsFragment extends Fragment implements CompoundButton
             boolean loginByEmail = buttonView == radioUseEmail;
             emailDetails.setVisibility(loginByEmail ? View.VISIBLE : View.GONE);
             urlDetails.setVisibility(loginByEmail ? View.GONE : View.VISIBLE);
+            (loginByEmail ? editEmailAddress : editBaseURL).requestFocus();
         }
     }
 

app/src/main/res/layout/login_account_details.xml

@@ -38,11 +38,32 @@
                 android:inputType="textEmailAddress"/>
 
             <TextView
+                android:id="@+id/account_email_hint"
                 android:layout_width="match_parent"
                 android:layout_height="wrap_content"
                 android:layout_marginTop="8dp"
                 android:text="@string/login_account_name_info"/>
 
+            <LinearLayout
+                android:id="@+id/carddav"
+                android:layout_height="wrap_content"
+                android:layout_width="match_parent"
+                android:orientation="vertical">
+
+                <TextView
+                    android:layout_width="match_parent"
+                    android:layout_height="wrap_content"
+                    android:layout_marginTop="16dp"
+                    android:text="@string/login_account_contact_group_method"/>
+
+                <Spinner
+                    android:id="@+id/contact_group_method"
+                    android:layout_width="match_parent"
+                    android:layout_height="wrap_content"
+                    android:entries="@array/settings_contact_group_method_entries"/>
+
+            </LinearLayout>
+
         </LinearLayout>
     </ScrollView>
 

app/src/main/res/values/strings.xml

@@ -126,6 +126,7 @@
     <string name="login_create_account">Create account</string>
     <string name="login_account_name">Account name</string>
     <string name="login_account_name_info">Use your email address as account name because Android will use the account name as ORGANIZER field for events you create. You can\'t have two accounts with the same name.</string>
+    <string name="login_account_contact_group_method">Contact group method:</string>
     <string name="login_account_name_required">Account name required</string>
     <string name="login_account_not_created">Account could not be created</string>
 
@@ -182,16 +183,12 @@
     <string name="settings_carddav">CardDAV</string>
     <string name="settings_contact_group_method">Contact group method</string>
     <string-array name="settings_contact_group_method_values">
-        <item>AUTOMATIC</item>
+        <item>GROUP_VCARDS</item>
-        <item>VCARD3_CATEGORIES</item>
+        <item>CATEGORIES</item>
-        <item>VCARD4</item>
-        <item>X_ADDRESSBOOK_SERVER</item>
     </string-array>
     <string-array name="settings_contact_group_method_entries">
-        <item>Automatic (VCard3/VCard4)</item>
+        <item>Groups are separate VCards</item>
-        <item>VCard3 only (CATEGORIES)</item>
+        <item>Groups are per-contact categories</item>
-        <item>VCard4 only (KIND/MEMBER)</item>
-        <item>Apple (X-ADDRESSBOOK-SERVER)</item>
     </string-array>
     <string name="settings_rfc6868_for_vcards">Use RFC6868 for VCards</string>
     <string name="settings_rfc6868_for_vcards_on">Double quotes can be used in parameter values</string>

scripts/fetch-db.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+cd ~/tmp
+adb pull /data/data/com.android.providers.contacts/databases/contacts2.db

vcard4android

@@ -1 +1 @@
-Subproject commit 38e24e8a19df339618ec33afb294a3e4a61b3cbe
+Subproject commit 02eae2c067c8fca4a9cf9f0e324af9bb4b91d3d0