From 4d36082398c4f706ea941ee7b4c0f76c9152d33c Mon Sep 17 00:00:00 2001
From: Florent Rougon <f.rougon@free.fr>
Date: Tue, 2 Jan 2018 10:07:49 +0100
Subject: [PATCH] Add-ons: detailed, machine-parseable metadata for authors and
 maintainers

(everything described here lives in the namespace flightgear::addons)

New classes: Author, Maintainer, and Contact. Author and Maintainer
derive from Contact. For each contact, the following can be defined in
addon-metadata.xml: name, email, and url. See [1] for details about the
syntax and usage policy. Nasal bindings have been updated accordingly,
there are three new ghosts: addons.Contact, addons.Author and
addons.Maintainer.

The enum class UrlType has two new members: author and maintainer. The
Addon::getUrls() method has a new signature:

  std::multimap<UrlType, QualifiedUrl> getUrls() const;

because non-empty 'url' fields for authors and maintainers contribute to
the result, and there can be an arbitrary number of authors and an
arbitrary number of maintainers defined for a given add-on---therefore,
std::map can't be used anymore.

Finally, QualifiedUrl has a new field (detail) which stores the author
name (resp. maintainer name) when the QualifiedUrl type is
UrlType::author (resp. UrlType::maintainer). Currently, this 'detail'
field is not used for other URL types, but this could be changed if
desired.

[1] https://sourceforge.net/p/flightgear/fgdata/ci/next/tree/Docs/README.add-ons
---
 src/Add-ons/Addon.cxx         | 114 ++++++++++++++++++++++--------
 src/Add-ons/Addon.hxx         |  38 +++++++---
 src/Add-ons/CMakeLists.txt    |   2 +
 src/Add-ons/addon_fwd.hxx     |   8 +++
 src/Add-ons/contacts.cxx      | 127 ++++++++++++++++++++++++++++++++++
 src/Add-ons/contacts.hxx      | 126 +++++++++++++++++++++++++++++++++
 src/Scripting/NasalAddons.cxx |   6 +-
 tests/CMakeLists.txt          |   1 +
 8 files changed, 384 insertions(+), 38 deletions(-)
 create mode 100644 src/Add-ons/contacts.cxx
 create mode 100644 src/Add-ons/contacts.hxx

diff --git a/src/Add-ons/Addon.cxx b/src/Add-ons/Addon.cxx
index 5928a0048..a91fbc432 100644
--- a/src/Add-ons/Addon.cxx
+++ b/src/Add-ons/Addon.cxx
@@ -34,7 +34,6 @@
 #include <simgear/nasal/naref.h>
 #include <simgear/props/props.hxx>
 #include <simgear/props/props_io.hxx>
-#include <simgear/sg_inlines.h>
 
 #include <Main/globals.hxx>
 #include <Scripting/NasalSys.hxx>
@@ -42,7 +41,9 @@
 #include "addon_fwd.hxx"
 #include "Addon.hxx"
 #include "AddonVersion.hxx"
+#include "contacts.hxx"
 #include "exceptions.hxx"
+#include "pointer_traits.hxx"
 
 namespace strutils = simgear::strutils;
 
@@ -59,9 +60,10 @@ namespace addons
 // *                              QualifiedUrl                               *
 // ***************************************************************************
 
-QualifiedUrl::QualifiedUrl(UrlType type, std::string url)
+QualifiedUrl::QualifiedUrl(UrlType type, string url, string detail)
   : _type(type),
-    _url(std::move(url))
+    _url(std::move(url)),
+    _detail(std::move(detail))
 { }
 
 UrlType QualifiedUrl::getType() const
@@ -76,6 +78,12 @@ std::string QualifiedUrl::getUrl() const
 void QualifiedUrl::setUrl(const std::string& url)
 { _url = url; }
 
+std::string QualifiedUrl::getDetail() const
+{ return _detail; }
+
+void QualifiedUrl::setDetail(const std::string& detail)
+{ _detail = detail; }
+
 // ***************************************************************************
 // *                                  Addon                                  *
 // ***************************************************************************
@@ -123,16 +131,16 @@ AddonVersionRef Addon::getVersion() const
 void Addon::setVersion(const AddonVersion& addonVersion)
 { _version.reset(new AddonVersion(addonVersion)); }
 
-std::string Addon::getAuthors() const
+std::vector<AuthorRef> Addon::getAuthors() const
 { return _authors; }
 
-void Addon::setAuthors(const std::string& addonAuthors)
+void Addon::setAuthors(const std::vector<AuthorRef>& addonAuthors)
 { _authors = addonAuthors; }
 
-std::string Addon::getMaintainers() const
+std::vector<MaintainerRef> Addon::getMaintainers() const
 { return _maintainers; }
 
-void Addon::setMaintainers(const std::string& addonMaintainers)
+void Addon::setMaintainers(const std::vector<MaintainerRef>& addonMaintainers)
 { _maintainers = addonMaintainers; }
 
 std::string Addon::getShortDescription() const
@@ -373,17 +381,10 @@ Addon Addon::fromAddonDir(const SGPath& addonPath)
   }
   AddonVersion addonVersion(strutils::strip(versionNode->getStringValue()));
 
-  string addonAuthors;
-  SGPropertyNode *authorsNode = addonNode->getChild("authors");
-  if (authorsNode != nullptr) {
-    addonAuthors = strutils::strip(authorsNode->getStringValue());
-  }
-
-  string addonMaintainers;
-  SGPropertyNode *maintainersNode = addonNode->getChild("maintainers");
-  if (maintainersNode != nullptr) {
-    addonMaintainers = strutils::strip(maintainersNode->getStringValue());
-  }
+  auto addonAuthors = parseContactsNode<Author>(metadataFile,
+                                                addonNode->getChild("authors"));
+  auto addonMaintainers = parseContactsNode<Maintainer>(
+    metadataFile, addonNode->getChild("maintainers"));
 
   string addonShortDescription;
   SGPropertyNode *shortDescNode = addonNode->getChild("short-description");
@@ -467,6 +468,62 @@ Addon Addon::fromAddonDir(const SGPath& addonPath)
   return addon;
 }
 
+// Utility function for Addon::parseContactsNode<>()
+//
+// Read a node such as "name", "email" or "url", child of a contact node (e.g.,
+// of an "author" or "maintainer" node).
+static string
+parseContactsNode_readNode(const SGPath& metadataFile,
+                           SGPropertyNode* contactNode,
+                           string subnodeName, bool allowEmpty)
+{
+  SGPropertyNode *node = contactNode->getChild(subnodeName);
+  string contents;
+
+  if (node != nullptr) {
+    contents = simgear::strutils::strip(node->getStringValue());
+  }
+
+  if (!allowEmpty && contents.empty()) {
+    throw errors::error_loading_metadata_file(
+      "in add-on metadata file '" + metadataFile.utf8Str() + "': "
+      "when the node " + contactNode->getPath(true) + " exists, it must have "
+      "a non-empty '" + subnodeName + "' child node");
+  }
+
+  return contents;
+};
+
+// Static method template (private and only used in this file)
+template <class T>
+vector<typename ContactTraits<T>::strong_ref>
+Addon::parseContactsNode(const SGPath& metadataFile, SGPropertyNode* mainNode)
+{
+  using contactTraits = ContactTraits<T>;
+  vector<typename contactTraits::strong_ref> res;
+
+  if (mainNode != nullptr) {
+    auto contactNodes = mainNode->getChildren(contactTraits::xmlNodeName());
+    res.reserve(contactNodes.size());
+
+    for (const auto& contactNode: contactNodes) {
+      string name, email, url;
+
+      name = parseContactsNode_readNode(metadataFile, contactNode.get(),
+                                        "name", false /* allowEmpty */);
+      email = parseContactsNode_readNode(metadataFile, contactNode.get(),
+                                         "email", true);
+      url = parseContactsNode_readNode(metadataFile, contactNode.get(),
+                                       "url", true);
+
+      using ptr_traits = shared_ptr_traits<typename contactTraits::strong_ref>;
+      res.push_back(ptr_traits::makeStrongRef(name, email, url));
+    }
+  }
+
+  return res;
+};
+
 // Static method
 std::tuple<string, SGPath, string>
 Addon::parseLicenseNode(const SGPath& addonPath, SGPropertyNode* addonNode)
@@ -535,21 +592,24 @@ Addon::parseLicenseNode(const SGPath& addonPath, SGPropertyNode* addonNode)
   return std::make_tuple(licenseDesignation, licenseFile, licenseUrl);
 }
 
-std::map<UrlType, QualifiedUrl> Addon::getUrls() const
+std::multimap<UrlType, QualifiedUrl> Addon::getUrls() const
 {
-  std::map<UrlType, QualifiedUrl> res;
+  std::multimap<UrlType, QualifiedUrl> res;
 
-  auto appendIfNonEmpty = [&res](UrlType type, const string& url) {
+  auto appendIfNonEmpty = [&res](UrlType type, string url, string detail = "") {
     if (!url.empty()) {
-      auto emplaceRetval = res.emplace(type, QualifiedUrl(type, url));
-      // We start with an empty std::map<> and don't call this lambda more
-      // than once for the same type, therefore the same key can't be seen
-      // twice.
-      assert(emplaceRetval.second);
-      SG_UNUSED(emplaceRetval); // 'cause asserts are removed in Release builds
+      res.emplace(type, QualifiedUrl(type, std::move(url), std::move(detail)));
     }
   };
 
+  for (const auto& author: _authors) {
+    appendIfNonEmpty(UrlType::author, author->getUrl(), author->getName());
+  }
+
+  for (const auto& maint: _maintainers) {
+    appendIfNonEmpty(UrlType::maintainer, maint->getUrl(), maint->getName());
+  }
+
   appendIfNonEmpty(UrlType::homePage, getHomePage());
   appendIfNonEmpty(UrlType::download, getDownloadUrl());
   appendIfNonEmpty(UrlType::support, getSupportUrl());
diff --git a/src/Add-ons/Addon.hxx b/src/Add-ons/Addon.hxx
index 82ff428d1..7fb09bdce 100644
--- a/src/Add-ons/Addon.hxx
+++ b/src/Add-ons/Addon.hxx
@@ -1,7 +1,7 @@
 // -*- coding: utf-8 -*-
 //
 // Addon.hxx --- FlightGear class holding add-on metadata
-// Copyright (C) 2017  Florent Rougon
+// Copyright (C) 2017, 2018  Florent Rougon
 //
 // This program is free software; you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
@@ -34,6 +34,7 @@
 
 #include "addon_fwd.hxx"
 #include "AddonVersion.hxx"
+#include "contacts.hxx"
 
 namespace flightgear
 {
@@ -42,6 +43,8 @@ namespace addons
 {
 
 enum class UrlType {
+  author,
+  maintainer,
   homePage,
   download,
   support,
@@ -49,9 +52,10 @@ enum class UrlType {
   license
 };
 
-class QualifiedUrl {
+class QualifiedUrl
+{
 public:
-  QualifiedUrl(UrlType type, std::string url);
+  QualifiedUrl(UrlType type, std::string url, std::string detail = "");
 
   UrlType getType() const;
   void setType(UrlType type);
@@ -59,9 +63,16 @@ public:
   std::string getUrl() const;
   void setUrl(const std::string& url);
 
+  std::string getDetail() const;
+  void setDetail(const std::string& detail);
+
 private:
   UrlType _type;
   std::string _url;
+  // Used to store the author or maintainer name when _type is UrlType::author
+  // or UrlType::maintainer. Could be used to record details about a website
+  // too (e.g., for a UrlType::support, something like “official forum”).
+  std::string _detail;
 };
 
 class Addon : public SGReferenced {
@@ -89,11 +100,11 @@ public:
   AddonVersionRef getVersion() const;
   void setVersion(const AddonVersion& addonVersion);
 
-  std::string getAuthors() const;
-  void setAuthors(const std::string& addonAuthors);
+  std::vector<AuthorRef> getAuthors() const;
+  void setAuthors(const std::vector<AuthorRef>& addonAuthors);
 
-  std::string getMaintainers() const;
-  void setMaintainers(const std::string& addonMaintainers);
+  std::vector<MaintainerRef> getMaintainers() const;
+  void setMaintainers(const std::vector<MaintainerRef>& addonMaintainers);
 
   std::string getShortDescription() const;
   void setShortDescription(const std::string& addonShortDescription);
@@ -156,7 +167,7 @@ public:
   void setLoadSequenceNumber(int num);
 
   // Get all non-empty URLs pertaining to this add-on
-  std::map<UrlType, QualifiedUrl> getUrls() const;
+  std::multimap<UrlType, QualifiedUrl> getUrls() const;
 
   // Simple string representation
   std::string str() const;
@@ -167,6 +178,13 @@ private:
   static std::tuple<string, SGPath, string>
   parseLicenseNode(const SGPath& addonPath, SGPropertyNode* addonNode);
 
+  // Parse an addon-metadata.xml node such as <authors> or <maintainers>.
+  // Return the corresponding vector<AuthorRef> or vector<MaintainerRef>. If
+  // the 'mainNode' argument is nullptr, return an empty vector.
+  template <class T>
+  static std::vector<typename ContactTraits<T>::strong_ref>
+  parseContactsNode(const SGPath& metadataFile, SGPropertyNode* mainNode);
+
   // The add-on identifier, in reverse DNS style. The AddonManager refuses to
   // register two add-ons with the same id in a given FlightGear session.
   std::string _id;
@@ -176,8 +194,8 @@ private:
   // needing to copy the data every time.
   AddonVersionRef _version;
 
-  std::string _authors;
-  std::string _maintainers;
+  std::vector<AuthorRef> _authors;
+  std::vector<MaintainerRef> _maintainers;
 
   // Strings describing what the add-on does
   std::string _shortDescription;
diff --git a/src/Add-ons/CMakeLists.txt b/src/Add-ons/CMakeLists.txt
index c31a67881..e7081eaa8 100644
--- a/src/Add-ons/CMakeLists.txt
+++ b/src/Add-ons/CMakeLists.txt
@@ -3,6 +3,7 @@ include(FlightGearComponent)
 set(SOURCES Addon.cxx
             AddonManager.cxx
             AddonVersion.cxx
+            contacts.cxx
             exceptions.cxx
             )
 
@@ -10,6 +11,7 @@ set(HEADERS addon_fwd.hxx
             Addon.hxx
             AddonManager.hxx
             AddonVersion.hxx
+            contacts.hxx
             exceptions.hxx
             pointer_traits.hxx
             )
diff --git a/src/Add-ons/addon_fwd.hxx b/src/Add-ons/addon_fwd.hxx
index fbfdb55f8..50ce27d57 100644
--- a/src/Add-ons/addon_fwd.hxx
+++ b/src/Add-ons/addon_fwd.hxx
@@ -37,8 +37,16 @@ class AddonVersionSuffix;
 enum class UrlType;
 class QualifiedUrl;
 
+enum class ContactType;
+class Contact;
+class Author;
+class Maintainer;
+
 using AddonRef = SGSharedPtr<Addon>;
 using AddonVersionRef = SGSharedPtr<AddonVersion>;
+using ContactRef = SGSharedPtr<Contact>;
+using AuthorRef = SGSharedPtr<Author>;
+using MaintainerRef = SGSharedPtr<Maintainer>;
 
 namespace errors
 {
diff --git a/src/Add-ons/contacts.cxx b/src/Add-ons/contacts.cxx
new file mode 100644
index 000000000..93dbc2e06
--- /dev/null
+++ b/src/Add-ons/contacts.cxx
@@ -0,0 +1,127 @@
+// -*- coding: utf-8 -*-
+//
+// contacts.cxx --- FlightGear classes holding add-on contact metadata
+// Copyright (C) 2018  Florent Rougon
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program; if not, write to the Free Software Foundation, Inc.,
+// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+#include <string>
+#include <utility>
+
+#include <simgear/nasal/cppbind/Ghost.hxx>
+#include <simgear/nasal/cppbind/NasalHash.hxx>
+#include <simgear/sg_inlines.h>
+#include <simgear/structure/exception.hxx>
+
+#include "addon_fwd.hxx"
+#include "contacts.hxx"
+
+using std::string;
+using simgear::enumValue;
+
+namespace flightgear
+{
+
+namespace addons
+{
+
+// ***************************************************************************
+// *                                 Contact                                 *
+// ***************************************************************************
+
+Contact::Contact(ContactType type, string name, string email, string url)
+  : _type(type),
+    _name(std::move(name)),
+    _email(std::move(email)),
+    _url(std::move(url))
+{ }
+
+ContactType Contact::getType() const
+{ return _type; }
+
+string Contact::getTypeString() const
+{
+  switch (getType()) {
+  case ContactType::author:
+    return "author";
+  case ContactType::maintainer:
+    return "maintainer";
+  default:
+    throw sg_error("unexpected value for member of "
+                   "flightgear::addons::ContactType: " +
+                   std::to_string(enumValue(getType())));
+  }
+}
+
+string Contact::getName() const
+{ return _name; }
+
+void Contact::setName(const string& name)
+{ _name = name; }
+
+string Contact::getEmail() const
+{ return _email; }
+
+void Contact::setEmail(const string& email)
+{ _email = email; }
+
+string Contact::getUrl() const
+{ return _url; }
+
+void Contact::setUrl(const string& url)
+{ _url = url; }
+
+// Static method
+void Contact::setupGhost(nasal::Hash& addonsModule)
+{
+  nasal::Ghost<ContactRef>::init("addons.Contact")
+    .member("name", &Contact::getName)
+    .member("email", &Contact::getEmail)
+    .member("url", &Contact::getUrl);
+}
+
+// ***************************************************************************
+// *                                 Author                                  *
+// ***************************************************************************
+
+Author::Author(string name, string email, string url)
+  : Contact(ContactType::author, name, email, url)
+{ }
+
+// Static method
+void Author::setupGhost(nasal::Hash& addonsModule)
+{
+  nasal::Ghost<AuthorRef>::init("addons.Author")
+    .bases<ContactRef>();
+}
+
+// ***************************************************************************
+// *                               Maintainer                                *
+// ***************************************************************************
+
+Maintainer::Maintainer(string name, string email, string url)
+  : Contact(ContactType::maintainer, name, email, url)
+{ }
+
+// Static method
+void Maintainer::setupGhost(nasal::Hash& addonsModule)
+{
+  nasal::Ghost<MaintainerRef>::init("addons.Maintainer")
+    .bases<ContactRef>();
+}
+
+} // of namespace addons
+
+} // of namespace flightgear
diff --git a/src/Add-ons/contacts.hxx b/src/Add-ons/contacts.hxx
new file mode 100644
index 000000000..6dacdfdda
--- /dev/null
+++ b/src/Add-ons/contacts.hxx
@@ -0,0 +1,126 @@
+// -*- coding: utf-8 -*-
+//
+// contacts.hxx --- FlightGear classes holding add-on contact metadata
+// Copyright (C) 2018  Florent Rougon
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program; if not, write to the Free Software Foundation, Inc.,
+// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+#ifndef FG_ADDON_CONTACTS_HXX
+#define FG_ADDON_CONTACTS_HXX
+
+#include <string>
+
+#include <simgear/structure/SGReferenced.hxx>
+
+#include "addon_fwd.hxx"
+
+namespace nasal
+{
+  class Hash;                   // forward declaration
+};
+
+namespace flightgear
+{
+
+namespace addons
+{
+
+enum class ContactType {
+  author,
+  maintainer
+};
+
+// Class used to store info about an author or maintainer (possibly also a
+// mailing-list, things like that)
+class Contact : public SGReferenced
+{
+public:
+  Contact(ContactType type, std::string name, std::string email = "",
+          std::string url = "");
+  virtual ~Contact() = default;
+
+  ContactType getType() const;
+  std::string getTypeString() const;
+
+  std::string getName() const;
+  void setName(const std::string& name);
+
+  std::string getEmail() const;
+  void setEmail(const std::string& email);
+
+  std::string getUrl() const;
+  void setUrl(const std::string& url);
+
+  static void setupGhost(nasal::Hash& addonsModule);
+
+private:
+  const ContactType _type;
+  std::string _name;
+  std::string _email;
+  std::string _url;
+};
+
+class Author : public Contact
+{
+public:
+  Author(std::string name, std::string email = "", std::string url = "");
+
+  static void setupGhost(nasal::Hash& addonsModule);
+};
+
+class Maintainer : public Contact
+{
+public:
+  Maintainer(std::string name, std::string email = "", std::string url = "");
+
+  static void setupGhost(nasal::Hash& addonsModule);
+};
+
+// ***************************************************************************
+// *                              ContactTraits                              *
+// ***************************************************************************
+
+template <typename T>
+struct ContactTraits;
+
+template<>
+struct ContactTraits<Author>
+{
+  using contact_type = Author;
+  using strong_ref = AuthorRef;
+
+  static std::string xmlNodeName()
+  {
+    return "author";
+  }
+};
+
+template<>
+struct ContactTraits<Maintainer>
+{
+  using contact_type = Maintainer;
+  using strong_ref = MaintainerRef;
+
+  static std::string xmlNodeName()
+  {
+    return "maintainer";
+  }
+};
+
+} // of namespace addons
+
+} // of namespace flightgear
+
+#endif  // of FG_ADDON_CONTACTS_HXX
diff --git a/src/Scripting/NasalAddons.cxx b/src/Scripting/NasalAddons.cxx
index db16de7fb..c06b86703 100644
--- a/src/Scripting/NasalAddons.cxx
+++ b/src/Scripting/NasalAddons.cxx
@@ -1,7 +1,7 @@
 // -*- coding: utf-8 -*-
 //
 // NasalAddons.cxx --- Expose add-on classes to Nasal
-// Copyright (C) 2017  Florent Rougon
+// Copyright (C) 2017, 2018  Florent Rougon
 //
 // This program is free software; you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
@@ -32,6 +32,7 @@
 #include <Add-ons/Addon.hxx>
 #include <Add-ons/AddonManager.hxx>
 #include <Add-ons/AddonVersion.hxx>
+#include <Add-ons/contacts.hxx>
 
 namespace flightgear
 {
@@ -192,6 +193,9 @@ void initAddonClassesForNasal(naRef globals, naContext c)
   wrapAddonManagerMethods(addonsModule);
 
   Addon::setupGhost(addonsModule);
+  Contact::setupGhost(addonsModule);
+  Author::setupGhost(addonsModule);
+  Maintainer::setupGhost(addonsModule);
 
   AddonVersion::setupGhost(addonsModule);
   addonsModule.createHash("AddonVersion").set("new", &f_createAddonVersion);
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 33afc4398..5d3240dcb 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -10,6 +10,7 @@ set(sources
   Add-ons/Addon.cxx
   Add-ons/AddonManager.cxx
   Add-ons/AddonVersion.cxx
+  Add-ons/contacts.cxx
   Add-ons/exceptions.cxx
   Aircraft/controls.cxx
   Aircraft/FlightHistory.cxx