From 8f65cc365f0010473e3273d7906c7e1738c448ab Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Wed, 25 Apr 2018 11:54:08 -0700 Subject: [PATCH] Use PrinterHashMap in MDNSFilteredDiscovery Before MDNSFilteredDiscovery used the ip-addresses to remove printers, but this did not work as once the nsd service is lost, you cannot resolve it anymore. Hence do the same thing as the other plugins, and remove a lost service if the name matches. Also do not try back when the callbacks are already gone. Fixes: 70622095 Test: Removed Wifi when print service recommendations were shown and saw all recommendations (including samsung) to go away. Without this change the samsung service never realized when printers went away. Change-Id: I7371ef00d626738741d39a2f3669497bfc9fd080 --- .../recommendation/RecommendationService.java | 18 ++-- .../plugin/hp/PrinterHashMap.java | 33 ------- .../plugin/hp/ServiceListener.java | 5 +- .../plugin/xerox/PrinterHashMap.java | 34 ------- .../plugin/xerox/ServiceResolver.java | 8 +- .../util/MDNSFilteredDiscovery.java | 55 +++++------ .../recommendation/util/PrinterHashMap.java | 91 +++++++++++++++++++ 7 files changed, 128 insertions(+), 116 deletions(-) delete mode 100644 packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/hp/PrinterHashMap.java delete mode 100755 packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/xerox/PrinterHashMap.java create mode 100644 packages/PrintRecommendationService/src/com/android/printservice/recommendation/util/PrinterHashMap.java diff --git a/core/java/android/printservice/recommendation/RecommendationService.java b/core/java/android/printservice/recommendation/RecommendationService.java index 733629aa35641..968a62585d8ea 100644 --- a/core/java/android/printservice/recommendation/RecommendationService.java +++ b/core/java/android/printservice/recommendation/RecommendationService.java @@ -119,14 +119,16 @@ public abstract class RecommendationService extends Service { mCallbacks = null; break; case MSG_UPDATE: - // Note that there might be a connection change in progress. In this case the - // message is handled as before the change. This is acceptable as the caller of - // the connection change has not guarantee when the connection change binder - // transaction is actually processed. - try { - mCallbacks.onRecommendationsUpdated((List) msg.obj); - } catch (RemoteException | NullPointerException e) { - Log.e(LOG_TAG, "Could not update recommended services", e); + if (mCallbacks != null) { + // Note that there might be a connection change in progress. In this case + // the message is handled as before the change. This is acceptable as the + // caller of the connection change has not guarantee when the connection + // change binder transaction is actually processed. + try { + mCallbacks.onRecommendationsUpdated((List) msg.obj); + } catch (RemoteException | NullPointerException e) { + Log.e(LOG_TAG, "Could not update recommended services", e); + } } break; } diff --git a/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/hp/PrinterHashMap.java b/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/hp/PrinterHashMap.java deleted file mode 100644 index 61956f6942451..0000000000000 --- a/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/hp/PrinterHashMap.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright (C) 2016 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.printservice.recommendation.plugin.hp; - -import android.net.nsd.NsdServiceInfo; - -import java.util.HashMap; - -final class PrinterHashMap extends HashMap { - public static String getKey(NsdServiceInfo serviceInfo) { - return serviceInfo.getServiceName(); - } - public NsdServiceInfo addPrinter(NsdServiceInfo device) { - return put(getKey(device), device); - } - public NsdServiceInfo removePrinter(NsdServiceInfo device) { - return remove(getKey(device)); - } -} diff --git a/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/hp/ServiceListener.java b/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/hp/ServiceListener.java index 600af1ff6da4d..9535ef06f888b 100644 --- a/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/hp/ServiceListener.java +++ b/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/hp/ServiceListener.java @@ -24,6 +24,7 @@ import android.text.TextUtils; import com.android.printservice.recommendation.R; import com.android.printservice.recommendation.util.DiscoveryListenerMultiplexer; +import com.android.printservice.recommendation.util.PrinterHashMap; import java.net.InetAddress; import java.util.ArrayList; @@ -183,9 +184,7 @@ public class ServiceListener implements ServiceResolveQueue.ResolveCallback { ArrayList printerAddressess = new ArrayList<>(); for (PrinterHashMap oneVendorPrinters : mVendorHashMap.values()) { - for (NsdServiceInfo printer : oneVendorPrinters.values()) { - printerAddressess.add(printer.getHost()); - } + printerAddressess.addAll(oneVendorPrinters.getPrinterAddresses()); } return printerAddressess; diff --git a/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/xerox/PrinterHashMap.java b/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/xerox/PrinterHashMap.java deleted file mode 100755 index b88c7c7253499..0000000000000 --- a/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/xerox/PrinterHashMap.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (C) 2016 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.android.printservice.recommendation.plugin.xerox; - -import android.net.nsd.NsdServiceInfo; - -import java.util.HashMap; - -final class PrinterHashMap extends HashMap { - public static String getKey(NsdServiceInfo serviceInfo) { - return serviceInfo.getServiceName(); - } - - public NsdServiceInfo addPrinter(NsdServiceInfo device) { - return put(getKey(device), device); - } - - public NsdServiceInfo removePrinter(NsdServiceInfo device) { - return remove(getKey(device)); - } -} diff --git a/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/xerox/ServiceResolver.java b/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/xerox/ServiceResolver.java index 4d0efd8be23d1..9ada969c84c9c 100755 --- a/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/xerox/ServiceResolver.java +++ b/packages/PrintRecommendationService/src/com/android/printservice/recommendation/plugin/xerox/ServiceResolver.java @@ -22,6 +22,7 @@ import android.text.TextUtils; import com.android.printservice.recommendation.util.DiscoveryListenerMultiplexer; import com.android.printservice.recommendation.util.NsdResolveQueue; +import com.android.printservice.recommendation.util.PrinterHashMap; import java.net.InetAddress; import java.util.ArrayList; @@ -195,12 +196,7 @@ class ServiceResolver { } public ArrayList getPrinters() { - ArrayList printerAddresses = new ArrayList<>(); - for (NsdServiceInfo printer : mPrinterHashMap.values()) { - printerAddresses.add(printer.getHost()); - } - - return printerAddresses; + return mPrinterHashMap.getPrinterAddresses(); } } diff --git a/packages/PrintRecommendationService/src/com/android/printservice/recommendation/util/MDNSFilteredDiscovery.java b/packages/PrintRecommendationService/src/com/android/printservice/recommendation/util/MDNSFilteredDiscovery.java index c08ca6ef591fb..65cef9441fdc7 100644 --- a/packages/PrintRecommendationService/src/com/android/printservice/recommendation/util/MDNSFilteredDiscovery.java +++ b/packages/PrintRecommendationService/src/com/android/printservice/recommendation/util/MDNSFilteredDiscovery.java @@ -18,6 +18,7 @@ package com.android.printservice.recommendation.util; import android.content.Context; import android.net.nsd.NsdManager; import android.net.nsd.NsdServiceInfo; +import android.text.TextUtils; import android.util.Log; import androidx.annotation.GuardedBy; @@ -27,9 +28,8 @@ import androidx.core.util.Preconditions; import com.android.printservice.recommendation.PrintServicePlugin; -import java.net.InetAddress; -import java.util.ArrayList; import java.util.HashSet; +import java.util.Objects; import java.util.Set; /** @@ -55,9 +55,9 @@ public class MDNSFilteredDiscovery implements NsdManager.DiscoveryListener { boolean matchesCriteria(NsdServiceInfo nsdServiceInfo); } - /** Printer identifiers of the mPrinters found. */ + /** Printers found. */ @GuardedBy("mLock") - private final @NonNull HashSet mPrinters; + private final @NonNull PrinterHashMap mPrinters; /** Service types discovered by this plugin */ private final @NonNull HashSet mServiceTypes; @@ -97,7 +97,7 @@ public class MDNSFilteredDiscovery implements NsdManager.DiscoveryListener { mPrinterFilter = Preconditions.checkNotNull(printerFilter, "printerFilter"); mResolveQueue = NsdResolveQueue.getInstance(); - mPrinters = new HashSet<>(); + mPrinters = new PrinterHashMap(); } /** @@ -107,6 +107,12 @@ public class MDNSFilteredDiscovery implements NsdManager.DiscoveryListener { return (NsdManager) mContext.getSystemService(Context.NSD_SERVICE); } + private void onChanged() { + if (mCallback != null) { + mCallback.onChanged(mPrinters.getPrinterAddresses()); + } + } + /** * Start the discovery. * @@ -114,7 +120,8 @@ public class MDNSFilteredDiscovery implements NsdManager.DiscoveryListener { */ public void start(@NonNull PrintServicePlugin.PrinterDiscoveryCallback callback) { mCallback = callback; - mCallback.onChanged(new ArrayList<>(mPrinters)); + + onChanged(); for (String serviceType : mServiceTypes) { DiscoveryListenerMultiplexer.addListener(getNDSManager(), serviceType, this); @@ -167,11 +174,12 @@ public class MDNSFilteredDiscovery implements NsdManager.DiscoveryListener { @Override public void onServiceResolved(NsdServiceInfo serviceInfo) { - if (mPrinterFilter.matchesCriteria(serviceInfo)) { + if (!TextUtils.isEmpty(PrinterHashMap.getKey(serviceInfo)) + && mPrinterFilter.matchesCriteria(serviceInfo)) { if (mCallback != null) { - boolean added = mPrinters.add(serviceInfo.getHost()); - if (added) { - mCallback.onChanged(new ArrayList<>(mPrinters)); + NsdServiceInfo old = mPrinters.addPrinter(serviceInfo); + if (!Objects.equals(old, serviceInfo)) { + onChanged(); } } } @@ -181,26 +189,9 @@ public class MDNSFilteredDiscovery implements NsdManager.DiscoveryListener { @Override public void onServiceLost(NsdServiceInfo serviceInfo) { - mResolveQueue.resolve(getNDSManager(), serviceInfo, - new NsdManager.ResolveListener() { - @Override - public void onResolveFailed(NsdServiceInfo serviceInfo, int errorCode) { - Log.w(LOG_TAG, "Service lost: Could not resolve " + serviceInfo + ": " - + errorCode); - } - - @Override - public void onServiceResolved(NsdServiceInfo serviceInfo) { - if (mPrinterFilter.matchesCriteria(serviceInfo)) { - if (mCallback != null) { - boolean removed = mPrinters.remove(serviceInfo.getHost()); - - if (removed) { - mCallback.onChanged(new ArrayList<>(mPrinters)); - } - } - } - } - }); + NsdServiceInfo oldAddress = mPrinters.removePrinter(serviceInfo); + if (oldAddress != null) { + onChanged(); + } } -} \ No newline at end of file +} diff --git a/packages/PrintRecommendationService/src/com/android/printservice/recommendation/util/PrinterHashMap.java b/packages/PrintRecommendationService/src/com/android/printservice/recommendation/util/PrinterHashMap.java new file mode 100644 index 0000000000000..ee35edb3956d0 --- /dev/null +++ b/packages/PrintRecommendationService/src/com/android/printservice/recommendation/util/PrinterHashMap.java @@ -0,0 +1,91 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.printservice.recommendation.util; + +import android.net.nsd.NsdServiceInfo; +import android.util.ArrayMap; + +import java.net.InetAddress; +import java.util.ArrayList; + +/** + * Map to store {@link NsdServiceInfo} belonging to printers. If two infos have the same + * {@link PrinterHashMap#getKey(NsdServiceInfo) key} they are considered the same. + */ +public class PrinterHashMap { + private final ArrayMap mPrinters = new ArrayMap<>(); + + /** + * Key uniquely identifying a printer. + * + * @param serviceInfo The service info describing the printer + * + * @return The key + */ + public static String getKey(NsdServiceInfo serviceInfo) { + return serviceInfo.getServiceName(); + } + + /** + * Add a printer. + * + * @param device The service info of the printer + * + * @return The service info of the printer that was previously registered for the same key + */ + public NsdServiceInfo addPrinter(NsdServiceInfo device) { + return mPrinters.put(getKey(device), device); + } + + /** + * Remove a printer. + * + * @param device The service info of the printer + * + * @return The service info of the printer that was previously registered for the same key + */ + public NsdServiceInfo removePrinter(NsdServiceInfo device) { + return mPrinters.remove(getKey(device)); + } + + /** + * @return the addresses of printers + */ + public ArrayList getPrinterAddresses() { + int numPrinters = mPrinters.size(); + ArrayList printerAddressess = new ArrayList<>(numPrinters); + for (int i = 0; i < numPrinters; i++) { + printerAddressess.add(mPrinters.valueAt(i).getHost()); + } + + return printerAddressess; + } + + /** + * Remove all printers + */ + public void clear() { + mPrinters.clear(); + } + + /** + * @return {@code} true iff the map is empty + */ + public boolean isEmpty() { + return mPrinters.isEmpty(); + } +}