[ntcore] Various fixes (#4469)

* Fix C++ Publisher and Subscriber move assignment 
* Fix Publisher comment typo.
* Publish: check that properties is an object.
Print a warning, but still publish, just with empty properties.
Add error print for unassigned type publish.
* Return boolean from SetProperties
* Document exception-throw in Java for invalid JSON.

ntcore/src/main/native/cpp/LocalStorage.cpp

@@ -759,6 +759,9 @@ void LSImpl::SetRetained(TopicData* topic, bool value) {
 
 void LSImpl::SetProperties(TopicData* topic, const wpi::json& update,
                            bool sendNetwork) {
+  if (!update.is_object()) {
+    return;
+  }
   DEBUG4("SetProperties({},{})", topic->name, sendNetwork);
   for (auto&& change : update.items()) {
     if (change.value().is_null()) {
@@ -924,8 +927,13 @@ PublisherData* LSImpl::AddLocalPublisher(TopicData* topic,
 
     if (properties.is_null()) {
       topic->properties = wpi::json::object();
-    } else {
+    } else if (properties.is_object()) {
       topic->properties = properties;
+    } else {
+      WPI_WARNING(m_logger,
+                  "ignoring non-object properties when publishing '{}'",
+                  topic->name);
+      topic->properties = wpi::json::object();
     }
 
     if (topic->properties.empty()) {
@@ -1812,14 +1820,17 @@ wpi::json LocalStorage::GetTopicProperties(NT_Topic topicHandle) {
   }
 }
 
-void LocalStorage::SetTopicProperties(NT_Topic topicHandle,
+bool LocalStorage::SetTopicProperties(NT_Topic topicHandle,
                                       const wpi::json& update) {
   if (!update.is_object()) {
-    return;
+    return false;
   }
   std::scoped_lock lock{m_mutex};
   if (auto topic = m_impl->m_topics.Get(topicHandle)) {
     m_impl->SetProperties(topic, update, true);
+    return true;
+  } else {
+    return {};
   }
 }
 
@@ -1871,15 +1882,21 @@ NT_Publisher LocalStorage::Publish(NT_Topic topicHandle, NT_Type type,
                                    std::string_view typeStr,
                                    const wpi::json& properties,
                                    wpi::span<const PubSubOption> options) {
-  if (type == NT_UNASSIGNED || typeStr.empty()) {
-    return 0;
-  }
-
   std::scoped_lock lock{m_mutex};
 
   // Get the topic
   auto* topic = m_impl->m_topics.Get(topicHandle);
   if (!topic) {
+    WPI_ERROR(m_impl->m_logger, "trying to publish invalid topic handle ({})",
+              topicHandle);
+    return 0;
+  }
+
+  if (type == NT_UNASSIGNED || typeStr.empty()) {
+    WPI_ERROR(
+        m_impl->m_logger,
+        "cannot publish '{}' with an unassigned type or empty type string",
+        topic->name);
     return 0;
   }
 

ntcore/src/main/native/cpp/LocalStorage.h

@@ -84,7 +84,7 @@ class LocalStorage final : public net::ILocalStorage {
 
   wpi::json GetTopicProperties(NT_Topic topic);
 
-  void SetTopicProperties(NT_Topic topic, const wpi::json& update);
+  bool SetTopicProperties(NT_Topic topic, const wpi::json& update);
 
   TopicInfo GetTopicInfo(NT_Topic topic);
 

ntcore/src/main/native/cpp/ntcore_c.cpp

@@ -298,8 +298,7 @@ NT_Bool NT_SetTopicProperties(NT_Topic topic, const char* properties) {
   } catch (wpi::json::parse_error&) {
     return false;
   }
-  nt::SetTopicProperties(topic, j);
-  return true;
+  return nt::SetTopicProperties(topic, j);
 }
 
 NT_Subscriber NT_Subscribe(NT_Topic topic, NT_Type type, const char* typeStr,

ntcore/src/main/native/cpp/ntcore_cpp.cpp

@@ -293,9 +293,11 @@ wpi::json GetTopicProperties(NT_Topic topic) {
   }
 }
 
-void SetTopicProperties(NT_Topic topic, const wpi::json& properties) {
+bool SetTopicProperties(NT_Topic topic, const wpi::json& properties) {
   if (auto ii = InstanceImpl::GetTyped(topic, Handle::kTopic)) {
-    ii->localStorage.SetTopicProperties(topic, properties);
+    return ii->localStorage.SetTopicProperties(topic, properties);
+  } else {
+    return {};
   }
 }
 

ntcore/src/main/native/include/networktables/Topic.h

@@ -149,8 +149,9 @@ class Topic {
    * of the corresponding property.
    *
    * @param properties JSON object with keys to add/update/delete
+   * @return False if properties is not an object
    */
-  void SetProperties(const wpi::json& properties);
+  bool SetProperties(const wpi::json& properties);
 
   /**
    * Gets combined information about the topic.
@@ -340,7 +341,7 @@ class Subscriber {
   NT_Subscriber m_subHandle{0};
 };
 
-/** NetworkTables pubscriber. */
+/** NetworkTables publisher. */
 class Publisher {
  public:
   virtual ~Publisher();

ntcore/src/main/native/include/networktables/Topic.inc

@@ -54,8 +54,8 @@ inline void Topic::DeleteProperty(std::string_view name) {
   ::nt::DeleteTopicProperty(m_handle, name);
 }
 
-inline void Topic::SetProperties(const wpi::json& properties) {
-  ::nt::SetTopicProperties(m_handle, properties);
+inline bool Topic::SetProperties(const wpi::json& properties) {
+  return ::nt::SetTopicProperties(m_handle, properties);
 }
 
 inline TopicInfo Topic::GetInfo() const {
@@ -71,6 +71,9 @@ inline Subscriber::Subscriber(Subscriber&& rhs) : m_subHandle{rhs.m_subHandle} {
 }
 
 inline Subscriber& Subscriber::operator=(Subscriber&& rhs) {
+  if (m_subHandle != 0) {
+    ::nt::Release(m_subHandle);
+  }
   m_subHandle = rhs.m_subHandle;
   rhs.m_subHandle = 0;
   return *this;
@@ -97,6 +100,9 @@ inline Publisher::Publisher(Publisher&& rhs) : m_pubHandle{rhs.m_pubHandle} {
 }
 
 inline Publisher& Publisher::operator=(Publisher&& rhs) {
+  if (m_pubHandle != 0) {
+    ::nt::Release(m_pubHandle);
+  }
   m_pubHandle = rhs.m_pubHandle;
   rhs.m_pubHandle = 0;
   return *this;

ntcore/src/main/native/include/ntcore_c.h

@@ -671,6 +671,7 @@ char* NT_GetTopicProperties(NT_Topic topic, size_t* len);
  *
  * @param topic topic handle
  * @param properties JSON object string with keys to add/update/delete
+ * @return False if properties are not a valid JSON object
  */
 NT_Bool NT_SetTopicProperties(NT_Topic topic, const char* properties);
 

ntcore/src/main/native/include/ntcore_cpp.h

@@ -651,8 +651,9 @@ wpi::json GetTopicProperties(NT_Topic topic);
  *
  * @param topic topic handle
  * @param update JSON object with keys to add/update/delete
+ * @return False if update is not a JSON object
  */
-void SetTopicProperties(NT_Topic topic, const wpi::json& update);
+bool SetTopicProperties(NT_Topic topic, const wpi::json& update);
 
 /**
  * Creates a new subscriber to value changes on a topic.