fix review issues:

1. publish and remove return void 2. remove -> removeServices 3. import for detail packages 4. naming consistent 5. remove DeleteLocalRef
project-chip · Oct 27, 2021 · 7f2bc85 · 7f2bc85
1 parent b2ec7a9
commit 7f2bc85
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 47 deletions.
diff --git a/src/platform/android/DnssdImpl.cpp b/src/platform/android/DnssdImpl.cpp
@@ -37,11 +37,11 @@ namespace Dnssd {
 using namespace chip::Platform;
 
 namespace {
-jobject sResolverObject     = nullptr;
-jobject sMdnsCallbackObject = nullptr;
-jmethodID sResolveMethod    = nullptr;
-jmethodID sPublishMethod    = nullptr;
-jmethodID sRemoveMethod     = nullptr;
+jobject sResolverObject         = nullptr;
+jobject sMdnsCallbackObject     = nullptr;
+jmethodID sResolveMethod        = nullptr;
+jmethodID sPublishMethod        = nullptr;
+jmethodID sRemoveServicesMethod = nullptr;
 } // namespace
 
 // Implemention of functions declared in lib/dnssd/platform/Dnssd.h
@@ -62,10 +62,10 @@ CHIP_ERROR ChipDnssdShutdown()
 
 CHIP_ERROR ChipDnssdRemoveServices()
 {
-    VerifyOrReturnError(sResolverObject != nullptr && sRemoveMethod != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturnError(sResolverObject != nullptr && sRemoveServicesMethod != nullptr, CHIP_ERROR_INCORRECT_STATE);
     JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
 
-    jboolean jret = env->CallBooleanMethod(sResolverObject, sRemoveMethod);
+    env->CallVoidMethod(sResolverObject, sRemoveServicesMethod);
 
     if (env->ExceptionCheck())
     {
@@ -75,14 +75,7 @@ CHIP_ERROR ChipDnssdRemoveServices()
         return CHIP_JNI_ERROR_EXCEPTION_THROWN;
     }
 
-    if (jret)
-    {
-        return CHIP_NO_ERROR;
-    }
-    else
-    {
-        return CHIP_JNI_ERROR_JAVA_ERROR;
-    }
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR ChipDnssdPublishService(const DnssdService * service)
@@ -121,12 +114,8 @@ CHIP_ERROR ChipDnssdPublishService(const DnssdService * service)
         env->SetObjectArrayElement(subTypes, i, jniSubType.jniValue());
     }
 
-    jboolean jret = env->CallBooleanMethod(sResolverObject, sPublishMethod, jniName.jniValue(), jniHostName.jniValue(),
-                                           jniServiceType.jniValue(), service->mPort, keys, datas, subTypes);
-
-    env->DeleteLocalRef(keys);
-    env->DeleteLocalRef(datas);
-    env->DeleteLocalRef(subTypes);
+    env->CallVoidMethod(sResolverObject, sPublishMethod, jniName.jniValue(), jniHostName.jniValue(), jniServiceType.jniValue(),
+                        service->mPort, keys, datas, subTypes);
 
     if (env->ExceptionCheck())
     {
@@ -136,14 +125,7 @@ CHIP_ERROR ChipDnssdPublishService(const DnssdService * service)
         return CHIP_JNI_ERROR_EXCEPTION_THROWN;
     }
 
-    if (jret)
-    {
-        return CHIP_NO_ERROR;
-    }
-    else
-    {
-        return CHIP_JNI_ERROR_JAVA_ERROR;
-    }
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR ChipDnssdFinalizeServiceUpdate()
@@ -207,17 +189,17 @@ void InitializeWithObjects(jobject resolverObject, jobject mdnsCallbackObject)
 
     sPublishMethod =
         env->GetMethodID(resolverClass, "publish",
-                         "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;I[Ljava/lang/String;[[B[Ljava/lang/String;)Z");
+                         "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;I[Ljava/lang/String;[[B[Ljava/lang/String;)V");
     if (sPublishMethod == nullptr)
     {
         ChipLogError(Discovery, "Failed to access Resolver 'publish' method");
         env->ExceptionClear();
     }
 
-    sRemoveMethod = env->GetMethodID(resolverClass, "remove", "()Z");
-    if (sRemoveMethod == nullptr)
+    sRemoveServicesMethod = env->GetMethodID(resolverClass, "removeServices", "()V");
+    if (sRemoveServicesMethod == nullptr)
     {
-        ChipLogError(Discovery, "Failed to access Resolver 'remove' method");
+        ChipLogError(Discovery, "Failed to access Resolver 'removeServices' method");
         env->ExceptionClear();
     }
 }

diff --git a/src/platform/android/java/chip/platform/NsdManagerServiceResolver.java b/src/platform/android/java/chip/platform/NsdManagerServiceResolver.java
@@ -25,15 +25,16 @@
 import android.os.Handler;
 import android.os.Looper;
 import android.util.Log;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.List;
 
 public class NsdManagerServiceResolver implements ServiceResolver {
   private static final String TAG = NsdManagerServiceResolver.class.getSimpleName();
   private static final long RESOLVE_SERVICE_TIMEOUT = 30000;
   private final NsdManager nsdManager;
   private MulticastLock multicastLock;
   private Handler mainThreadHandler;
-  private List<NsdManager.RegistrationListener> mRegistrationListeners = new ArrayList<>();
+  private List<NsdManager.RegistrationListener> registrationListeners = new ArrayList<>();
 
   public NsdManagerServiceResolver(Context context) {
     this.nsdManager = (NsdManager) context.getSystemService(Context.NSD_SERVICE);
@@ -116,7 +117,7 @@ public void onServiceResolved(NsdServiceInfo serviceInfo) {
   }
 
   @Override
-  public boolean publish(
+  public void publish(
       String serviceName,
       String hostName,
       String type,
@@ -156,19 +157,15 @@ public void onServiceUnregistered(NsdServiceInfo serviceInfo) {
             Log.i(TAG, "service " + serviceInfo.getServiceName() + " onServiceRegistered:");
           }
         };
-    mRegistrationListeners.add(registrationListener);
+    registrationListeners.add(registrationListener);
 
     nsdManager.registerService(serviceInfo, NsdManager.PROTOCOL_DNS_SD, registrationListener);
-
-    return true;
   }
 
   @Override
-  public boolean remove() {
-    for (NsdManager.RegistrationListener l : mRegistrationListeners) {
+  public void removeServices() {
+    for (NsdManager.RegistrationListener l : registrationListeners) {
       nsdManager.unregisterService(l);
     }
-
-    return true;
   }
 }
diff --git a/src/platform/android/java/chip/platform/ServiceResolver.java b/src/platform/android/java/chip/platform/ServiceResolver.java
@@ -35,10 +35,9 @@ void resolve(
    * Publishes a service via DNS-SD.
    *
    * <p>Calling the function again with the same service name, type, protocol, interface and port
-   * but different text will update the text published. This function will NOT take the ownership of
-   * service->mTextEntries memory.
+   * but different text will update the text published.
    */
-  boolean publish(
+  void publish(
       String serviceName,
       String hostName,
       String type,
@@ -48,5 +47,5 @@ boolean publish(
       String[] subTypes);
 
   /** Removes or marks all services being advertised for removal. */
-  boolean remove();
+  void removeServices();
 }