From dc63a40ddaf6a223101c2af8b57ac12ad49428e6 Mon Sep 17 00:00:00 2001 From: zachh Date: Fri, 10 Nov 2017 11:42:47 -0800 Subject: Use parent message for CP2 info in phone_lookup_info.proto. PhoneLookupInfo should contain one message per PhoneLookup implementation for clarity. Instead of having a repeated Cp2Info field, we now have a single Cp2Info field which wraps a repeated Cp2ContactInfo field. Also added ApdlInfo to the message to make testing of CompositePhoneLookup more realistic, in that it tests merging across submessages now. (ApdlInfo is more or less a placeholder until ApdlPhoneLookup is implemented.) Test: yes PiperOrigin-RevId: 175316738 Change-Id: I196c2eaa885e1268ff80ebaad6d85840a9cc7a15 --- .../dialer/phonelookup/cp2/Cp2PhoneLookup.java | 68 +++++++++++----------- .../dialer/phonelookup/phone_lookup_info.proto | 44 ++++++++------ 2 files changed, 62 insertions(+), 50 deletions(-) diff --git a/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java b/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java index 2878e27c4..29aa909db 100644 --- a/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java +++ b/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java @@ -33,6 +33,7 @@ import com.android.dialer.inject.ApplicationContext; import com.android.dialer.phonelookup.PhoneLookup; import com.android.dialer.phonelookup.PhoneLookupInfo; import com.android.dialer.phonelookup.PhoneLookupInfo.Cp2Info; +import com.android.dialer.phonelookup.PhoneLookupInfo.Cp2Info.Cp2ContactInfo; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.ListenableFuture; @@ -182,9 +183,9 @@ public final class Cp2PhoneLookup implements PhoneLookup { getDeletedPhoneNumbers(existingInfoMap, lastModified); // For each DialerPhoneNumber that was associated with a contact or added to a contact, - // build a map of those DialerPhoneNumbers to a set Cp2Infos, where each Cp2Info represents a - // contact. - ImmutableMap> updatedContacts = + // build a map of those DialerPhoneNumbers to a set Cp2ContactInfos, where each Cp2ContactInfo + // represents a contact. + ImmutableMap> updatedContacts = buildMapForUpdatedOrAddedContacts(existingInfoMap, lastModified, deletedPhoneNumbers); // Start build a new map of updated info. This will replace existing info. @@ -196,16 +197,16 @@ public final class Cp2PhoneLookup implements PhoneLookup { // Build off the existing info PhoneLookupInfo.Builder infoBuilder = PhoneLookupInfo.newBuilder(entry.getValue()); - // If the contact was updated, replace the Cp2Info list + // If the contact was updated, replace the Cp2ContactInfo list if (updatedContacts.containsKey(entry.getKey())) { - infoBuilder.clearCp2Info(); - infoBuilder.addAllCp2Info(updatedContacts.get(entry.getKey())); + infoBuilder.setCp2Info( + Cp2Info.newBuilder().addAllCp2ContactInfo(updatedContacts.get(entry.getKey()))); - // If it was deleted and not added to a new contact, replace the Cp2Info list with - // the default instance of Cp2Info + // If it was deleted and not added to a new contact, replace the Cp2ContactInfo list with + // the default instance of Cp2ContactInfo } else if (deletedPhoneNumbers.contains(entry.getKey())) { - infoBuilder.clearCp2Info(); - infoBuilder.addCp2Info(Cp2Info.getDefaultInstance()); + infoBuilder.setCp2Info( + Cp2Info.newBuilder().addCp2ContactInfo(Cp2ContactInfo.getDefaultInstance())); } // If the DialerPhoneNumber didn't change, add the unchanged existing info. @@ -218,12 +219,12 @@ public final class Cp2PhoneLookup implements PhoneLookup { * 1. get all contact ids. if the id is unset, add the number to the list of contacts to look up. * 2. reduce our list of contact ids to those that were updated after lastModified. 3. Now we have * the smallest set of dialer phone numbers to query cp2 against. 4. build and return the map of - * dialerphonenumbers to their new cp2info + * dialerphonenumbers to their new Cp2ContactInfo * * @return Map of {@link DialerPhoneNumber} to {@link PhoneLookupInfo} with updated {@link - * Cp2Info}. + * Cp2ContactInfo}. */ - private ImmutableMap> buildMapForUpdatedOrAddedContacts( + private ImmutableMap> buildMapForUpdatedOrAddedContacts( ImmutableMap existingInfoMap, long lastModified, Set deletedPhoneNumbers) { @@ -239,20 +240,20 @@ public final class Cp2PhoneLookup implements PhoneLookup { continue; } - // For each Cp2Info for each existing DialerPhoneNumber... + // For each Cp2ContactInfo for each existing DialerPhoneNumber... // Store the contact id if it exist, else automatically add the DialerPhoneNumber to our // set of DialerPhoneNumbers we want to update. - for (Cp2Info cp2Info : entry.getValue().getCp2InfoList()) { - if (Objects.equals(cp2Info, Cp2Info.getDefaultInstance())) { - // If the number doesn't have any Cp2Info set to it, for various reasons, we need to look - // up the number to check if any exists. + for (Cp2ContactInfo cp2ContactInfo : entry.getValue().getCp2Info().getCp2ContactInfoList()) { + if (Objects.equals(cp2ContactInfo, Cp2ContactInfo.getDefaultInstance())) { + // If the number doesn't have any Cp2ContactInfo set to it, for various reasons, we need + // to look up the number to check if any exists. // The various reasons this might happen are: // - An existing contact that wasn't in the call log is now in the call log. // - A number was in the call log before but has now been added to a contact. // - A number is in the call log, but isn't associated with any contact. updatedNumbers.add(entry.getKey()); } else { - contactIds.add(cp2Info.getContactId()); + contactIds.add(cp2ContactInfo.getContactId()); } } } @@ -264,28 +265,29 @@ public final class Cp2PhoneLookup implements PhoneLookup { cursor.moveToPosition(-1); while (cursor.moveToNext()) { // Find the DialerPhoneNumber for each contact id and add it to our updated numbers set. - // These, along with our number not associated with any Cp2Info need to be updated. + // These, along with our number not associated with any Cp2ContactInfo need to be updated. long contactId = cursor.getLong(contactIdIndex); updatedNumbers.addAll(getDialerPhoneNumber(existingInfoMap, contactId)); } } - // Query the Phone table and build Cp2Info for each DialerPhoneNumber in our updatedNumbers set. - Map> map = new ArrayMap<>(); + // Query the Phone table and build Cp2ContactInfo for each DialerPhoneNumber in our + // updatedNumbers set. + Map> map = new ArrayMap<>(); try (Cursor cursor = getAllCp2Rows(updatedNumbers)) { cursor.moveToPosition(-1); while (cursor.moveToNext()) { // Map each dialer phone number to it's new cp2 info Set phoneNumbers = getDialerPhoneNumbers(updatedNumbers, cursor.getString(CP2_INFO_NUMBER_INDEX)); - Cp2Info info = buildCp2InfoFromUpdatedContactsCursor(cursor); + Cp2ContactInfo info = buildCp2ContactInfoFromUpdatedContactsCursor(cursor); for (DialerPhoneNumber phoneNumber : phoneNumbers) { if (map.containsKey(phoneNumber)) { map.get(phoneNumber).add(info); } else { - Set cp2Infos = new ArraySet<>(); - cp2Infos.add(info); - map.put(phoneNumber, cp2Infos); + Set cp2ContactInfos = new ArraySet<>(); + cp2ContactInfos.add(info); + map.put(phoneNumber, cp2ContactInfos); } } } @@ -312,14 +314,14 @@ public final class Cp2PhoneLookup implements PhoneLookup { /** * @param cursor with projection {@link #CP2_INFO_PROJECTION}. - * @return new {@link Cp2Info} based on current row of {@code cursor}. + * @return new {@link Cp2ContactInfo} based on current row of {@code cursor}. */ - private static Cp2Info buildCp2InfoFromUpdatedContactsCursor(Cursor cursor) { + private static Cp2ContactInfo buildCp2ContactInfoFromUpdatedContactsCursor(Cursor cursor) { String displayName = cursor.getString(CP2_INFO_NAME_INDEX); String photoUri = cursor.getString(CP2_INFO_PHOTO_URI_INDEX); String label = cursor.getString(CP2_INFO_LABEL_INDEX); - Cp2Info.Builder infoBuilder = Cp2Info.newBuilder(); + Cp2ContactInfo.Builder infoBuilder = Cp2ContactInfo.newBuilder(); if (!TextUtils.isEmpty(displayName)) { infoBuilder.setName(displayName); } @@ -351,8 +353,8 @@ public final class Cp2PhoneLookup implements PhoneLookup { private Set findContactIdsIn(ImmutableMap map) { Set contactIds = new ArraySet<>(); for (PhoneLookupInfo info : map.values()) { - for (Cp2Info cp2Info : info.getCp2InfoList()) { - contactIds.add(cp2Info.getContactId()); + for (Cp2ContactInfo cp2ContactInfo : info.getCp2Info().getCp2ContactInfoList()) { + contactIds.add(cp2ContactInfo.getContactId()); } } return contactIds; @@ -414,8 +416,8 @@ public final class Cp2PhoneLookup implements PhoneLookup { ImmutableMap existingInfoMap, long contactId) { Set matches = new ArraySet<>(); for (Entry entry : existingInfoMap.entrySet()) { - for (Cp2Info cp2Info : entry.getValue().getCp2InfoList()) { - if (cp2Info.getContactId() == contactId) { + for (Cp2ContactInfo cp2ContactInfo : entry.getValue().getCp2Info().getCp2ContactInfoList()) { + if (cp2ContactInfo.getContactId() == contactId) { matches.add(entry.getKey()); } } diff --git a/java/com/android/dialer/phonelookup/phone_lookup_info.proto b/java/com/android/dialer/phonelookup/phone_lookup_info.proto index cb89a64e3..93dd01e86 100644 --- a/java/com/android/dialer/phonelookup/phone_lookup_info.proto +++ b/java/com/android/dialer/phonelookup/phone_lookup_info.proto @@ -17,23 +17,33 @@ message PhoneLookupInfo { // Information about a PhoneNumber retrieved from CP2. Cp2PhoneLookup is // responsible for populating the data in this message. message Cp2Info { - // android.provider.ContactsContract.CommonDataKinds.Phone.DISPLAY_NAME_PRIMARY - optional string name = 1; - - // android.provider.ContactsContract.CommonDataKinds.Phone.PHOTO_THUMBNAIL_URI - optional string photo_uri = 2; - - // android.provider.ContactsContract.CommonDataKinds.Phone.PHOTO_ID - optional fixed64 photo_id = 3; - - // android.provider.ContactsContract.CommonDataKinds.Phone.LABEL - // "Home", "Mobile", ect. - optional string label = 4; + // Information about a single local contact. + message Cp2ContactInfo { + // android.provider.ContactsContract.CommonDataKinds.Phone.DISPLAY_NAME_PRIMARY + optional string name = 1; + + // android.provider.ContactsContract.CommonDataKinds.Phone.PHOTO_THUMBNAIL_URI + optional string photo_uri = 2; + + // android.provider.ContactsContract.CommonDataKinds.Phone.PHOTO_ID + optional fixed64 photo_id = 3; + + // android.provider.ContactsContract.CommonDataKinds.Phone.LABEL + // "Home", "Mobile", ect. + optional string label = 4; + + // android.provider.ContactsContract.CommonDataKinds.Phone.CONTACT_ID + optional fixed64 contact_id = 5; + } + // Repeated because one phone number can be associated with multiple CP2 + // contacts. + repeated Cp2ContactInfo cp2_contact_info = 1; + } + optional Cp2Info cp2_info = 1; - // android.provider.ContactsContract.CommonDataKinds.Phone.CONTACT_ID - optional fixed64 contact_id = 5; + // Message for APDL, a lookup for the proprietary Google dialer. + message ApdlInfo { + optional bool is_spam = 1; } - // Repeated because one phone number can be associated with multiple CP2 - // contacts. - repeated Cp2Info cp2_info = 1; + optional ApdlInfo apdl_info = 2; } \ No newline at end of file -- cgit v1.2.3 From e9f94c74ff76f9673d744ddc357b66def1f37f14 Mon Sep 17 00:00:00 2001 From: wangqi Date: Fri, 10 Nov 2017 11:51:50 -0800 Subject: Fix bug that "+" is dropped when performing actions in smart dial search. This is caused by normalizing numbers from dialpad search into search box, while "add new contacts" and "send sms" etc. wouldn't work properly without full number. This change also fix "send sms" in old search fragment. This change includes upstream change from: https://android-review.googlesource.com/#/c/platform/packages/apps/Dialer/+/530377/ Change address format when sending message from smart dial In Android O, send search key number as address to message app. In Android N, send real input number as address to message app. Fixed the issue by reverting to Android N implementation. Affected Area: Send SMS operation from Dialer smart dial search result list. Bug: 68962106 Test: manual PiperOrigin-RevId: 175317850 Change-Id: I96dd576144f93f7502977bfdb4b9e9d9c8f73526 --- java/com/android/dialer/app/DialtactsActivity.java | 9 +++++---- java/com/android/dialer/app/list/SearchFragment.java | 5 ++++- .../dialer/searchfragment/list/NewSearchFragment.java | 11 +++++++++-- .../android/dialer/searchfragment/list/SearchAdapter.java | 12 ++++++++++-- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/java/com/android/dialer/app/DialtactsActivity.java b/java/com/android/dialer/app/DialtactsActivity.java index 4c2634184..269e598e1 100644 --- a/java/com/android/dialer/app/DialtactsActivity.java +++ b/java/com/android/dialer/app/DialtactsActivity.java @@ -298,10 +298,8 @@ public class DialtactsActivity extends TransactionSafeActivity PerformanceReport.recordClick(UiAction.Type.TEXT_CHANGE_WITH_INPUT); } - if (DEBUG) { - LogUtil.v("DialtactsActivity.onTextChanged", "called with new query: " + newText); - LogUtil.v("DialtactsActivity.onTextChanged", "previous query: " + mSearchQuery); - } + LogUtil.v("DialtactsActivity.onTextChanged", "called with new query: " + newText); + LogUtil.v("DialtactsActivity.onTextChanged", "previous query: " + mSearchQuery); mSearchQuery = newText; // TODO(calderwoodra): show p13n when newText is empty. @@ -1381,6 +1379,9 @@ public class DialtactsActivity extends TransactionSafeActivity if (mSmartDialSearchFragment != null) { mSmartDialSearchFragment.setAddToContactNumber(query); } + if (mNewSearchFragment != null) { + mNewSearchFragment.setRawNumber(query); + } final String normalizedQuery = SmartDialNameMatcher.normalizeNumber(query, SmartDialNameMatcher.LATIN_SMART_DIAL_MAP); diff --git a/java/com/android/dialer/app/list/SearchFragment.java b/java/com/android/dialer/app/list/SearchFragment.java index e21e073bd..9330fc8c7 100644 --- a/java/com/android/dialer/app/list/SearchFragment.java +++ b/java/com/android/dialer/app/list/SearchFragment.java @@ -261,7 +261,10 @@ public class SearchFragment extends PhoneNumberPickerFragment { getActivity(), intent, R.string.add_contact_not_available); break; case DialerPhoneNumberListAdapter.SHORTCUT_SEND_SMS_MESSAGE: - number = adapter.getFormattedQueryString(); + number = + TextUtils.isEmpty(mAddToContactNumber) + ? adapter.getFormattedQueryString() + : mAddToContactNumber; intent = IntentUtil.getSendSmsIntent(number); DialerUtils.startActivityWithErrorToast(getActivity(), intent); break; diff --git a/java/com/android/dialer/searchfragment/list/NewSearchFragment.java b/java/com/android/dialer/searchfragment/list/NewSearchFragment.java index e797a0390..8306d37a6 100644 --- a/java/com/android/dialer/searchfragment/list/NewSearchFragment.java +++ b/java/com/android/dialer/searchfragment/list/NewSearchFragment.java @@ -112,6 +112,9 @@ public final class NewSearchFragment extends Fragment private RecyclerView recyclerView; private SearchAdapter adapter; private String query; + // Raw query number from dialpad, which may contain special character such as "+". This is used + // for actions to add contact or send sms. + private String rawNumber; private CallInitiationType.Type callInitiationType = CallInitiationType.Type.UNKNOWN_INITIATION; private boolean remoteDirectoriesDisabledForTesting; @@ -138,7 +141,7 @@ public final class NewSearchFragment extends Fragment LayoutInflater inflater, @Nullable ViewGroup parent, @Nullable Bundle savedInstanceState) { View view = inflater.inflate(R.layout.fragment_search, parent, false); adapter = new SearchAdapter(getContext(), new SearchCursorManager(), this); - adapter.setQuery(query); + adapter.setQuery(query, rawNumber); adapter.setSearchActions(getActions()); adapter.setZeroSuggestVisible(getArguments().getBoolean(KEY_SHOW_ZERO_SUGGEST)); emptyContentView = view.findViewById(R.id.empty_view); @@ -248,11 +251,15 @@ public final class NewSearchFragment extends Fragment } } + public void setRawNumber(String rawNumber) { + this.rawNumber = rawNumber; + } + public void setQuery(String query, CallInitiationType.Type callInitiationType) { this.query = query; this.callInitiationType = callInitiationType; if (adapter != null) { - adapter.setQuery(query); + adapter.setQuery(query, rawNumber); adapter.setSearchActions(getActions()); adapter.setZeroSuggestVisible(isRegularSearch()); loadNearbyPlacesCursor(); diff --git a/java/com/android/dialer/searchfragment/list/SearchAdapter.java b/java/com/android/dialer/searchfragment/list/SearchAdapter.java index cc090ac47..4254baece 100644 --- a/java/com/android/dialer/searchfragment/list/SearchAdapter.java +++ b/java/com/android/dialer/searchfragment/list/SearchAdapter.java @@ -18,6 +18,7 @@ package com.android.dialer.searchfragment.list; import android.content.Context; import android.database.Cursor; +import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; import android.support.v7.widget.RecyclerView; import android.support.v7.widget.RecyclerView.ViewHolder; @@ -47,6 +48,9 @@ public final class SearchAdapter extends RecyclerView.Adapter { private boolean showZeroSuggest; private String query; + // Raw query number from dialpad, which may contain special character such as "+". This is used + // for actions to add contact or send sms. + private String rawNumber; private OnClickListener allowClickListener; private OnClickListener dismissClickListener; private RowClickListener rowClickListener; @@ -127,7 +131,10 @@ public final class SearchAdapter extends RecyclerView.Adapter { ((HeaderViewHolder) holder).setHeader(header); } else if (holder instanceof SearchActionViewHolder) { ((SearchActionViewHolder) holder) - .setAction(searchCursorManager.getSearchAction(position), position, query); + .setAction( + searchCursorManager.getSearchAction(position), + position, + TextUtils.isEmpty(rawNumber) ? query : rawNumber); } else if (holder instanceof LocationPermissionViewHolder) { // No-op } else { @@ -179,8 +186,9 @@ public final class SearchAdapter extends RecyclerView.Adapter { showZeroSuggest = visible; } - public void setQuery(String query) { + public void setQuery(String query, @Nullable String rawNumber) { this.query = query; + this.rawNumber = rawNumber; if (searchCursorManager.setQuery(query)) { notifyDataSetChanged(); } -- cgit v1.2.3