security: modernize Key and KeyContainer; add logging

Change-Id: Ibbe6a4ea54e2a1cc7ad7a7e00ea88a29ab1f6c3d

ndn-cxx/security/pib/impl/key-impl.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2021 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -21,21 +21,22 @@
 
 #include "ndn-cxx/security/pib/impl/key-impl.hpp"
 #include "ndn-cxx/security/pib/pib-impl.hpp"
-#include "ndn-cxx/security/pib/pib.hpp"
 #include "ndn-cxx/security/transform/public-key.hpp"
+#include "ndn-cxx/util/logger.hpp"
 
 namespace ndn {
 namespace security {
 namespace pib {
 namespace detail {
 
+NDN_LOG_INIT(ndn.security.Key);
+
 KeyImpl::KeyImpl(const Name& keyName, span<const uint8_t> key, shared_ptr<PibImpl> pibImpl)
   : m_identity(extractIdentityFromKeyName(keyName))
   , m_keyName(keyName)
   , m_key(key.begin(), key.end())
   , m_pib(std::move(pibImpl))
   , m_certificates(keyName, m_pib)
-  , m_isDefaultCertificateLoaded(false)
 {
   BOOST_ASSERT(m_pib != nullptr);
 
@@ -56,7 +57,6 @@ KeyImpl::KeyImpl(const Name& keyName, shared_ptr<PibImpl> pibImpl)
   , m_keyName(keyName)
   , m_pib(std::move(pibImpl))
   , m_certificates(keyName, m_pib)
-  , m_isDefaultCertificateLoaded(false)
 {
   BOOST_ASSERT(m_pib != nullptr);
 
@@ -68,10 +68,10 @@ KeyImpl::KeyImpl(const Name& keyName, shared_ptr<PibImpl> pibImpl)
 }
 
 void
-KeyImpl::addCertificate(const Certificate& certificate)
+KeyImpl::addCertificate(const Certificate& cert)
 {
   BOOST_ASSERT(m_certificates.isConsistent());
-  m_certificates.add(certificate);
+  m_certificates.add(cert);
 }
 
 void
@@ -79,42 +79,23 @@ KeyImpl::removeCertificate(const Name& certName)
 {
   BOOST_ASSERT(m_certificates.isConsistent());
 
-  if (m_isDefaultCertificateLoaded && m_defaultCertificate.getName() == certName)
-    m_isDefaultCertificateLoaded = false;
-
+  if (m_defaultCert && m_defaultCert->getName() == certName) {
+    NDN_LOG_DEBUG("Removing default certificate " << certName);
+    m_defaultCert = nullopt;
+  }
   m_certificates.remove(certName);
 }
 
-Certificate
-KeyImpl::getCertificate(const Name& certName) const
-{
-  BOOST_ASSERT(m_certificates.isConsistent());
-  return m_certificates.get(certName);
-}
-
-const CertificateContainer&
-KeyImpl::getCertificates() const
-{
-  BOOST_ASSERT(m_certificates.isConsistent());
-  return m_certificates;
-}
-
 const Certificate&
-KeyImpl::setDefaultCertificate(const Name& certName)
+KeyImpl::setDefaultCert(Certificate cert)
 {
   BOOST_ASSERT(m_certificates.isConsistent());
 
-  m_defaultCertificate = m_certificates.get(certName);
-  m_pib->setDefaultCertificateOfKey(m_keyName, certName);
-  m_isDefaultCertificateLoaded = true;
-  return m_defaultCertificate;
-}
+  m_defaultCert = std::move(cert);
+  m_pib->setDefaultCertificateOfKey(m_keyName, m_defaultCert->getName());
+  NDN_LOG_DEBUG("Default certificate set to " << m_defaultCert->getName());
 
-const Certificate&
-KeyImpl::setDefaultCertificate(const Certificate& certificate)
-{
-  addCertificate(certificate);
-  return setDefaultCertificate(certificate.getName());
+  return *m_defaultCert;
 }
 
 const Certificate&
@@ -122,13 +103,14 @@ KeyImpl::getDefaultCertificate() const
 {
   BOOST_ASSERT(m_certificates.isConsistent());
 
-  if (!m_isDefaultCertificateLoaded) {
-    m_defaultCertificate = m_pib->getDefaultCertificateOfKey(m_keyName);
-    m_isDefaultCertificateLoaded = true;
+  if (!m_defaultCert) {
+    m_defaultCert = m_pib->getDefaultCertificateOfKey(m_keyName);
+    NDN_LOG_DEBUG("Caching default certificate " << m_defaultCert->getName());
   }
-  BOOST_ASSERT(m_pib->getDefaultCertificateOfKey(m_keyName).wireEncode() == m_defaultCertificate.wireEncode());
 
-  return m_defaultCertificate;
+  BOOST_ASSERT(m_defaultCert);
+  BOOST_ASSERT(m_defaultCert->getName() == m_pib->getDefaultCertificateOfKey(m_keyName).getName());
+  return *m_defaultCert;
 }
 
 } // namespace detail

ndn-cxx/security/pib/impl/key-impl.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2021 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -39,143 +39,110 @@ namespace detail {
  * A Key has only one backend instance, but may have multiple frontend handles.
  * Each frontend handle is associated with the only one backend KeyImpl.
  *
- * @throw PibImpl::Error when underlying implementation has non-semantic error.
+ * @throw PibImpl::Error When the underlying implementation has a non-semantic error.
+ * @sa Key
  */
 class KeyImpl : noncopyable
 {
 public:
   /**
-   * @brief Create a KeyImpl with @p keyName.
+   * @brief Create a new key with name @p keyName.
    *
    * If the key does not exist in the backend, it will be added.
    * If a key with the same name already exists, it will be overwritten.
    *
    * @param keyName The name of the key.
    * @param key The public key bits.
-   * @param pibImpl The Pib backend implementation.
-   * @throw std::invalid_argument @p key is invalid.
+   * @param pibImpl The PIB backend implementation.
+   * @throw std::invalid_argument @p key is invalid or unsupported.
    */
   KeyImpl(const Name& keyName, span<const uint8_t> key, shared_ptr<PibImpl> pibImpl);
 
   /**
-   * @brief Create a KeyImpl with @p keyName.
+   * @brief Load an existing key with name @p keyName.
    *
    * @param keyName The name of the key.
-   * @param pibImpl The Pib backend implementation.
-   * @throw Pib::Error the key does not exist.
+   * @param pibImpl The PIB backend implementation.
+   * @throw Pib::Error The key does not exist in the backend.
    */
   KeyImpl(const Name& keyName, shared_ptr<PibImpl> pibImpl);
 
-  /**
-   * @brief Get the name of the key.
-   */
+  // See security::pib::Key for the documentation of the following methods
+
   const Name&
   getName() const
   {
     return m_keyName;
   }
 
-  /**
-   * @brief Get the name of the belonging identity.
-   */
   const Name&
   getIdentity() const
   {
     return m_identity;
   }
 
-  /**
-   * @brief Get key type.
-   */
   KeyType
   getKeyType() const
   {
     return m_keyType;
   }
 
-  /**
-   * @brief Get public key bits.
-   */
   span<const uint8_t>
   getPublicKey() const
   {
     return m_key;
   }
 
-  /**
-   * @brief Add @p certificate.
-   *
-   * If no default certificate is set before, the new certificate will be set as the default
-   * certificate of the key.
-   *
-   * If a certificate with the same name (without implicit digest) already exists, it will
-   * be overwritten.
-   *
-   * @throw std::invalid_argument the certificate name does not match the key name.
-   */
   void
-  addCertificate(const Certificate& certificate);
+  addCertificate(const Certificate& cert);
 
-  /**
-   * @brief Remove a certificate with @p certName.
-   * @throw std::invalid_argument @p certName does not match the key name.
-   */
   void
   removeCertificate(const Name& certName);
 
-  /**
-   * @brief Get a certificate with @p certName.
-   * @throw std::invalid_argument @p certName does not match the key name.
-   * @throw Pib::Error the certificate does not exist.
-   */
   Certificate
-  getCertificate(const Name& certName) const;
+  getCertificate(const Name& certName) const
+  {
+    BOOST_ASSERT(m_certificates.isConsistent());
+    return m_certificates.get(certName);
+  }
 
-  /**
-   * @brief Get all the certificates for this key.
-   */
   const CertificateContainer&
-  getCertificates() const;
+  getCertificates() const
+  {
+    BOOST_ASSERT(m_certificates.isConsistent());
+    return m_certificates;
+  }
 
-  /**
-   * @brief Set an existing certificate with name @p certName as the default certificate.
-   * @throw std::invalid_argument @p certName does not match the key name.
-   * @throw Pib::Error the certificate does not exist.
-   * @return the default certificate
-   */
   const Certificate&
-  setDefaultCertificate(const Name& certName);
+  setDefaultCertificate(const Name& certName)
+  {
+    return setDefaultCert(m_certificates.get(certName));
+  }
 
-  /**
-   * @brief Add @p certificate and set it as the default certificate for this key.
-   *
-   * If a certificate with the same name (without implicit digest) already exists, it will
-   * be overwritten.
-   *
-   * @throw std::invalid_argument @p certificate does not match the key name.
-   * @return the default certificate
-   */
-  const Certificate&
-  setDefaultCertificate(const Certificate& certificate);
+  void
+  setDefaultCertificate(const Certificate& cert)
+  {
+    m_certificates.add(cert);
+    setDefaultCert(cert);
+  }
 
-  /**
-   * @brief Get the default certificate for this key.
-   * @throw Pib::Error the default certificate does not exist.
-   */
   const Certificate&
   getDefaultCertificate() const;
 
 private:
-  Name m_identity;
-  Name m_keyName;
+  const Certificate&
+  setDefaultCert(Certificate cert);
+
+private:
+  const Name m_identity;
+  const Name m_keyName;
   Buffer m_key;
   KeyType m_keyType;
 
-  shared_ptr<PibImpl> m_pib;
+  const shared_ptr<PibImpl> m_pib;
 
   CertificateContainer m_certificates;
-  mutable bool m_isDefaultCertificateLoaded;
-  mutable Certificate m_defaultCertificate;
+  mutable optional<Certificate> m_defaultCert;
 };
 
 } // namespace detail

ndn-cxx/security/pib/key-container.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2021 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -23,20 +23,18 @@
 #include "ndn-cxx/security/pib/impl/key-impl.hpp"
 #include "ndn-cxx/security/pib/pib-impl.hpp"
 #include "ndn-cxx/util/concepts.hpp"
+#include "ndn-cxx/util/logger.hpp"
 
 namespace ndn {
 namespace security {
 namespace pib {
 
+NDN_LOG_INIT(ndn.security.KeyContainer);
+
 NDN_CXX_ASSERT_FORWARD_ITERATOR(KeyContainer::const_iterator);
 
-KeyContainer::const_iterator::const_iterator()
-  : m_container(nullptr)
-{
-}
-
-KeyContainer::const_iterator::const_iterator(std::set<Name>::const_iterator it,
-                                             const KeyContainer& container)
+KeyContainer::const_iterator::const_iterator(NameSet::const_iterator it,
+                                             const KeyContainer& container) noexcept
   : m_it(it)
   , m_container(&container)
 {
@@ -49,35 +47,14 @@ KeyContainer::const_iterator::operator*()
   return m_container->get(*m_it);
 }
 
-KeyContainer::const_iterator&
-KeyContainer::const_iterator::operator++()
-{
-  ++m_it;
-  return *this;
-}
-
-KeyContainer::const_iterator
-KeyContainer::const_iterator::operator++(int)
-{
-  const_iterator it(*this);
-  ++m_it;
-  return it;
-}
-
 bool
-KeyContainer::const_iterator::operator==(const const_iterator& other)
+KeyContainer::const_iterator::operator==(const const_iterator& other) const
 {
   bool isThisEnd = m_container == nullptr || m_it == m_container->m_keyNames.end();
   bool isOtherEnd = other.m_container == nullptr || other.m_it == other.m_container->m_keyNames.end();
-  return ((isThisEnd || isOtherEnd) ?
-          (isThisEnd == isOtherEnd) :
-          m_container->m_pib == other.m_container->m_pib && m_it == other.m_it);
-}
-
-bool
-KeyContainer::const_iterator::operator!=(const const_iterator& other)
-{
-  return !(*this == other);
+  if (isThisEnd)
+    return isOtherEnd;
+  return !isOtherEnd && m_container->m_pib == other.m_container->m_pib && m_it == other.m_it;
 }
 
 KeyContainer::KeyContainer(const Name& identity, shared_ptr<PibImpl> pibImpl)
@@ -88,41 +65,26 @@ KeyContainer::KeyContainer(const Name& identity, shared_ptr<PibImpl> pibImpl)
   m_keyNames = m_pib->getKeysOfIdentity(identity);
 }
 
-KeyContainer::const_iterator
-KeyContainer::begin() const
-{
-  return {m_keyNames.begin(), *this};
-}
-
-KeyContainer::const_iterator
-KeyContainer::end() const
-{
-  return {};
-}
-
 KeyContainer::const_iterator
 KeyContainer::find(const Name& keyName) const
 {
   return {m_keyNames.find(keyName), *this};
 }
 
-size_t
-KeyContainer::size() const
-{
-  return m_keyNames.size();
-}
-
 Key
-KeyContainer::add(span<const uint8_t> key, const Name& keyName)
+KeyContainer::add(span<const uint8_t> keyBits, const Name& keyName)
 {
   if (m_identity != extractIdentityFromKeyName(keyName)) {
     NDN_THROW(std::invalid_argument("Key name `" + keyName.toUri() + "` does not match identity "
                                     "`" + m_identity.toUri() + "`"));
   }
 
-  m_keyNames.insert(keyName);
-  m_keys[keyName] = make_shared<detail::KeyImpl>(keyName, key, m_pib);
-  return get(keyName);
+  bool isNew = m_keyNames.insert(keyName).second;
+  NDN_LOG_DEBUG((isNew ? "Adding " : "Replacing ") << keyName);
+
+  auto key = std::make_shared<detail::KeyImpl>(keyName, keyBits, m_pib);
+  m_keys[keyName] = key; // use insert_or_assign in C++17
+  return Key(key);
 }
 
 void
@@ -133,8 +95,14 @@ KeyContainer::remove(const Name& keyName)
                                     "`" + m_identity.toUri() + "`"));
   }
 
-  m_keyNames.erase(keyName);
-  m_keys.erase(keyName);
+  if (m_keyNames.erase(keyName) > 0) {
+    NDN_LOG_DEBUG("Removing " << keyName);
+    m_keys.erase(keyName);
+  }
+  else {
+    // consistency check
+    BOOST_ASSERT(m_keys.find(keyName) == m_keys.end());
+  }
   m_pib->removeKey(keyName);
 }
 
@@ -146,17 +114,13 @@ KeyContainer::get(const Name& keyName) const
                                     "`" + m_identity.toUri() + "`"));
   }
 
-  shared_ptr<detail::KeyImpl> key;
   auto it = m_keys.find(keyName);
-
   if (it != m_keys.end()) {
-    key = it->second;
-  }
-  else {
-    key = make_shared<detail::KeyImpl>(keyName, m_pib);
-    m_keys[keyName] = key;
+    return Key(it->second);
   }
 
+  auto key = std::make_shared<detail::KeyImpl>(keyName, m_pib);
+  m_keys[keyName] = key;
   return Key(key);
 }
 

ndn-cxx/security/pib/key-container.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2021 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -35,18 +35,23 @@ namespace pib {
 class PibImpl;
 
 namespace detail {
-class KeyImpl;
 class IdentityImpl;
+class KeyImpl;
 } // namespace detail
 
 /**
- * @brief Container of keys of an identity
+ * @brief Container of keys of an identity.
  *
- * The container is used to search/enumerate keys of an identity.
- * The container can be created only by detail::IdentityImpl.
+ * The container is used to search/enumerate the keys of an identity.
+ * It can be created only by the IdentityImpl private class.
+ *
+ * @sa Identity::getKeys()
  */
 class KeyContainer : noncopyable
 {
+private:
+  using NameSet = std::set<Name>;
+
 public:
   class const_iterator
   {
@@ -57,117 +62,133 @@ public:
     using pointer           = value_type*;
     using reference         = value_type&;
 
-    const_iterator();
+    const_iterator() = default;
 
     Key
     operator*();
 
     const_iterator&
-    operator++();
+    operator++()
+    {
+      ++m_it;
+      return *this;
+    }
 
     const_iterator
-    operator++(int);
+    operator++(int)
+    {
+      const_iterator it(*this);
+      ++m_it;
+      return it;
+    }
 
     bool
-    operator==(const const_iterator& other);
+    operator==(const const_iterator& other) const;
 
     bool
-    operator!=(const const_iterator& other);
+    operator!=(const const_iterator& other) const
+    {
+      return !this->operator==(other);
+    }
 
   private:
-    const_iterator(std::set<Name>::const_iterator it, const KeyContainer& container);
+    const_iterator(NameSet::const_iterator it, const KeyContainer& container) noexcept;
 
   private:
-    std::set<Name>::const_iterator m_it;
-    const KeyContainer* m_container;
+    NameSet::const_iterator m_it;
+    const KeyContainer* m_container = nullptr;
 
-    friend class KeyContainer;
+    friend KeyContainer;
   };
 
   using iterator = const_iterator;
 
 public:
   const_iterator
-  begin() const;
+  begin() const noexcept
+  {
+    return {m_keyNames.begin(), *this};
+  }
 
   const_iterator
-  end() const;
+  end() const noexcept
+  {
+    return {};
+  }
 
   const_iterator
   find(const Name& keyName) const;
 
+  /**
+   * @brief Check whether the container is empty.
+   */
+  NDN_CXX_NODISCARD bool
+  empty() const noexcept
+  {
+    return m_keyNames.empty();
+  }
+
+  /**
+   * @brief Return the number of keys in the container.
+   */
   size_t
-  size() const;
+  size() const noexcept
+  {
+    return m_keyNames.size();
+  }
 
   /**
    * @brief Add @p key with name @p keyName into the container.
    *
    * If a key with the same name already exists, it will be overwritten.
    *
-   * @throw std::invalid_argument @p keyName does not match the identity
+   * @throw std::invalid_argument @p keyName does not match the identity.
    */
   Key
   add(span<const uint8_t> key, const Name& keyName);
 
   /**
-   * @brief Remove a key with @p keyName from the container
-   * @throw std::invalid_argument @p keyName does not match the identity
+   * @brief Remove a key with @p keyName from the container.
+   * @throw std::invalid_argument @p keyName does not match the identity.
    */
   void
   remove(const Name& keyName);
 
   /**
-   * @brief Get a key with @p keyName from the container
-   * @throw std::invalid_argument @p keyName does not match the identity
-   * @throw Pib::Error the key does not exist
+   * @brief Return a key by name.
+   * @throw Pib::Error The key does not exist.
+   * @throw std::invalid_argument @p keyName does not match the identity.
    */
   Key
   get(const Name& keyName) const;
 
   /**
-   * @brief Check if the container is consistent with the backend storage
-   *
-   * @note this method is heavyweight and should be used in debugging mode only.
+   * @brief Check if the container is consistent with the backend storage.
+   * @note This method is heavyweight and should be used in debugging mode only.
    */
   bool
   isConsistent() const;
 
-NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE:
+NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE: // private interface for IdentityImpl
   /**
-   * @brief Create key container for @p identity
+   * @brief Create key container for @p identity.
    * @param pibImpl The PIB backend implementation.
    */
   KeyContainer(const Name& identity, shared_ptr<PibImpl> pibImpl);
 
-  const std::set<Name>&
-  getKeyNames() const
-  {
-    return m_keyNames;
-  }
-
-  const std::unordered_map<Name, shared_ptr<detail::KeyImpl>>&
-  getLoadedKeys() const
-  {
-    return m_keys;
-  }
-
-private:
-  Name m_identity;
-  std::set<Name> m_keyNames;
-  /// @brief Cache of loaded detail::KeyImpl.
+NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE:
+  // cache of loaded KeyImpl
   mutable std::unordered_map<Name, shared_ptr<detail::KeyImpl>> m_keys;
 
-  shared_ptr<PibImpl> m_pib;
+private:
+  NameSet m_keyNames;
+  const Name m_identity;
+  const shared_ptr<PibImpl> m_pib;
 
-#ifndef DOXYGEN
   friend detail::IdentityImpl;
-#endif
 };
 
 } // namespace pib
-
-using pib::KeyContainer;
-
 } // namespace security
 } // namespace ndn
 

ndn-cxx/security/pib/key.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2021 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -21,15 +21,14 @@
 
 #include "ndn-cxx/security/pib/key.hpp"
 #include "ndn-cxx/security/pib/impl/key-impl.hpp"
-#include "ndn-cxx/security/certificate.hpp"
 
 namespace ndn {
 namespace security {
 namespace pib {
 
-Key::Key() = default;
+Key::Key() noexcept = default;
 
-Key::Key(weak_ptr<detail::KeyImpl> impl)
+Key::Key(weak_ptr<detail::KeyImpl> impl) noexcept
   : m_impl(std::move(impl))
 {
 }
@@ -88,7 +87,7 @@ Key::setDefaultCertificate(const Name& certName) const
   return lock()->setDefaultCertificate(certName);
 }
 
-const Certificate&
+void
 Key::setDefaultCertificate(const Certificate& certificate) const
 {
   return lock()->setDefaultCertificate(certificate);
@@ -100,7 +99,7 @@ Key::getDefaultCertificate() const
   return lock()->getDefaultCertificate();
 }
 
-Key::operator bool() const
+Key::operator bool() const noexcept
 {
   return !m_impl.expired();
 }
@@ -109,30 +108,17 @@ shared_ptr<detail::KeyImpl>
 Key::lock() const
 {
   auto impl = m_impl.lock();
-
   if (impl == nullptr) {
-    NDN_THROW(std::domain_error("Invalid key instance"));
+    NDN_THROW(std::domain_error("Invalid PIB key instance"));
   }
-
   return impl;
 }
 
 bool
-operator!=(const Key& lhs, const Key& rhs)
+Key::equals(const Key& other) const noexcept
 {
-  return lhs.m_impl.owner_before(rhs.m_impl) || rhs.m_impl.owner_before(lhs.m_impl);
-}
-
-std::ostream&
-operator<<(std::ostream& os, const Key& key)
-{
-  if (key) {
-    os << key.getName();
-  }
-  else {
-    os << "(empty)";
-  }
-  return os;
+  return !this->m_impl.owner_before(other.m_impl) &&
+         !other.m_impl.owner_before(this->m_impl);
 }
 
 } // namespace pib
@@ -142,18 +128,16 @@ inline namespace v2 {
 Name
 constructKeyName(const Name& identity, const name::Component& keyId)
 {
-  Name keyName = identity;
-  keyName
-    .append(Certificate::KEY_COMPONENT)
-    .append(keyId);
-  return keyName;
+  return Name(identity)
+         .append(Certificate::KEY_COMPONENT)
+         .append(keyId);
 }
 
 bool
 isValidKeyName(const Name& keyName)
 {
-  return (keyName.size() >= Certificate::MIN_KEY_NAME_LENGTH &&
-          keyName.get(-Certificate::MIN_KEY_NAME_LENGTH) == Certificate::KEY_COMPONENT);
+  return keyName.size() >= Certificate::MIN_KEY_NAME_LENGTH &&
+         keyName.get(-Certificate::MIN_KEY_NAME_LENGTH) == Certificate::KEY_COMPONENT;
 }
 
 Name

ndn-cxx/security/pib/key.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2021 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -34,135 +34,136 @@ class KeyChain;
 
 namespace pib {
 
+class KeyContainer;
+
 namespace detail {
 class KeyImpl;
 } // namespace detail
 
 /**
- * @brief A frontend handle of a key instance
+ * @brief Frontend handle for a key in the PIB.
  *
- * Key is at the second level in PIB's Identity-Key-Certificate hierarchy.  A Key has a Name
- * (identity + "KEY" + keyId), and contains one or more certificates, one of which is set as
- * the default certificate of this key.  A certificate can be directly accessed from a Key
- * object.
+ * Key is at the second level in PIB's Identity-Key-Certificate hierarchy. A key has the name
+ * `/<Identity>/KEY/<KeyId>`, and contains one or more certificates, one of which is set as
+ * default certificate of that key. Certificates can be directly accessed from a Key object.
  */
 class Key
 {
 public:
   /**
-   * @brief Default Constructor
+   * @brief Default constructor.
    *
-   * Key created using this default constructor is just a place holder.
-   * It can obtain an actual instance from Identity::getKey(...).  A typical
+   * A Key created using this default constructor is just a placeholder.
+   * You can obtain an actual instance from Identity::getKey(). A typical
    * usage would be for exception handling:
    *
-   *   Key key;
-   *   try {
-   *     key = identity.getKey(...);
-   *   }
-   *   catch (const Pib::Error&) {
-   *     ...
-   *   }
+   * @code
+   * Key key;
+   * try {
+   *   key = identity.getKey(...);
+   * }
+   * catch (const Pib::Error&) {
+   *   ...
+   * }
+   * @endcode
    *
-   * A Key instance created using this constructor is invalid. Calling a
-   * member method on an invalid Key instance may cause an std::domain_error.
+   * An instance created using this constructor is invalid. Calling a member
+   * function on an invalid Key instance may throw an std::domain_error.
    */
-  Key();
+  Key() noexcept;
 
   /**
-   * @brief Create a Key with a backend implementation @p impl.
-   *
-   * This method should only be used by KeyContainer.
-   */
-  explicit
-  Key(weak_ptr<detail::KeyImpl> impl);
-
-  /**
-   * @brief Get key name.
+   * @brief Return the name of the key.
    */
   const Name&
   getName() const;
 
   /**
-   * @brief Get the name of the belonging identity.
+   * @brief Return the name of the owning identity.
    */
   const Name&
   getIdentity() const;
 
   /**
-   * @brief Get key type.
+   * @brief Return the key type.
    */
   KeyType
   getKeyType() const;
 
   /**
-   * @brief Get public key bits.
+   * @brief Return the raw public key bits.
    */
   span<const uint8_t>
   getPublicKey() const;
 
   /**
-   * @brief Get a certificate with @p certName
-   * @throw std::invalid_argument @p certName does not match key name
+   * @brief Return the certificate with the given name.
+   * @throw std::invalid_argument @p certName does not match the key name.
    * @throw Pib::Error the certificate does not exist.
    */
   Certificate
   getCertificate(const Name& certName) const;
 
   /**
-   * @brief Get all certificates for this key.
+   * @brief Return all the certificates of this key.
    */
   const CertificateContainer&
   getCertificates() const;
 
   /**
-   * @brief Get the default certificate for this Key.
+   * @brief Return the default certificate for this key.
    * @throw Pib::Error the default certificate does not exist.
    */
   const Certificate&
   getDefaultCertificate() const;
 
   /**
-   * @brief Check if the Key instance is valid.
+   * @brief Returns true if the instance is valid.
    */
   explicit
-  operator bool() const;
+  operator bool() const noexcept;
 
-NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE: // write operations should be private
+NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE: // write operations are accessible only by KeyChain
   /**
    * @brief Add @p certificate.
-   * @throw std::invalid_argument certificate name does not match key name
    *
-   * If a certificate with the same name (without implicit digest) already exists, overwrite
-   * the certificate.
+   * If no default certificate is set before, the new certificate will be set as the default
+   * certificate of the key.
+   * If a certificate with the same name (excluding implicit digest) already exists, it will
+   * be overwritten.
+   *
+   * @throw std::invalid_argument the certificate name does not match the key name.
    */
   void
   addCertificate(const Certificate& certificate) const;
 
   /**
-   * @brief Remove a certificate with @p certName
-   * @throw std::invalid_argument @p certName does not match key name
+   * @brief Remove a certificate with @p certName.
+   * @throw std::invalid_argument @p certName does not match the key name.
    */
   void
   removeCertificate(const Name& certName) const;
 
   /**
-   * @brief Set an existing certificate with @p certName as the default certificate
-   * @throw std::invalid_argument @p certName does not match key name
+   * @brief Set an existing certificate with name @p certName as the default certificate.
+   * @return The default certificate.
+   * @throw std::invalid_argument @p certName does not match the key name.
    * @throw Pib::Error the certificate does not exist.
-   * @return the default certificate
    */
   const Certificate&
   setDefaultCertificate(const Name& certName) const;
 
   /**
-   * @brief Add @p certificate and set it as the default certificate of the key
-   * @throw std::invalid_argument @p certificate does not match key name
-   * @return the default certificate
+   * @brief Add/replace @p certificate and set it as the default certificate.
+   * @throw std::invalid_argument the certificate name does not match the key name.
    */
-  const Certificate&
+  void
   setDefaultCertificate(const Certificate& certificate) const;
 
+NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE: // private interface for KeyContainer
+  explicit
+  Key(weak_ptr<detail::KeyImpl> impl) noexcept;
+
 private:
   /**
    * @brief Check the validity of the instance
@@ -172,25 +173,38 @@ private:
   shared_ptr<detail::KeyImpl>
   lock() const;
 
+  bool
+  equals(const Key& other) const noexcept;
+
+  // NOTE
+  // The following "hidden friend" non-member operators are available
+  // via argument-dependent lookup only and must be defined inline.
+
+  friend bool
+  operator==(const Key& lhs, const Key& rhs)
+  {
+    return lhs.equals(rhs);
+  }
+
+  friend bool
+  operator!=(const Key& lhs, const Key& rhs)
+  {
+    return !lhs.equals(rhs);
+  }
+
+  friend std::ostream&
+  operator<<(std::ostream& os, const Key& key)
+  {
+    return os << (key ? key.getName() : "(empty)");
+  }
+
 private:
   weak_ptr<detail::KeyImpl> m_impl;
 
   friend KeyChain;
-  friend bool operator!=(const Key&, const Key&);
+  friend KeyContainer;
 };
 
-bool
-operator!=(const Key& lhs, const Key& rhs);
-
-inline bool
-operator==(const Key& lhs, const Key& rhs)
-{
-  return !(lhs != rhs);
-}
-
-std::ostream&
-operator<<(std::ostream& os, const Key& key);
-
 } // namespace pib
 
 using pib::Key;
@@ -216,7 +230,6 @@ Name
 extractIdentityFromKeyName(const Name& keyName);
 
 } // inline namespace v2
-
 } // namespace security
 } // namespace ndn
 

tests/unit/security/pib/impl/key-impl.t.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2021 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -37,7 +37,7 @@ BOOST_AUTO_TEST_SUITE(Security)
 BOOST_AUTO_TEST_SUITE(Pib)
 BOOST_FIXTURE_TEST_SUITE(TestKeyImpl, security::tests::PibDataFixture)
 
-using security::Pib;
+using pib::Pib;
 
 BOOST_AUTO_TEST_CASE(Basic)
 {
@@ -47,18 +47,16 @@ BOOST_AUTO_TEST_CASE(Basic)
   BOOST_CHECK_EQUAL(key11.getName(), id1Key1Name);
   BOOST_CHECK_EQUAL(key11.getIdentity(), id1);
   BOOST_CHECK_EQUAL(key11.getKeyType(), KeyType::EC);
-  BOOST_CHECK_EQUAL_COLLECTIONS(key11.getPublicKey().begin(), key11.getPublicKey().end(),
-                                id1Key1.begin(), id1Key1.end());
+  BOOST_TEST(key11.getPublicKey() == id1Key1, boost::test_tools::per_element());
 
   KeyImpl key11Bak(id1Key1Name, pibImpl);
   BOOST_CHECK_EQUAL(key11Bak.getName(), id1Key1Name);
   BOOST_CHECK_EQUAL(key11Bak.getIdentity(), id1);
   BOOST_CHECK_EQUAL(key11Bak.getKeyType(), KeyType::EC);
-  BOOST_CHECK_EQUAL_COLLECTIONS(key11Bak.getPublicKey().begin(), key11Bak.getPublicKey().end(),
-                                id1Key1.begin(), id1Key1.end());
+  BOOST_TEST(key11Bak.getPublicKey() == id1Key1, boost::test_tools::per_element());
 }
 
-BOOST_AUTO_TEST_CASE(CertificateOperation)
+BOOST_AUTO_TEST_CASE(CertificateOperations)
 {
   auto pibImpl = make_shared<pib::PibMemory>();
   KeyImpl key11(id1Key1Name, id1Key1, pibImpl);
@@ -72,16 +70,15 @@ BOOST_AUTO_TEST_CASE(CertificateOperation)
   // get default certificate, throw Pib::Error
   BOOST_CHECK_THROW(key11.getDefaultCertificate(), Pib::Error);
   // set non-existing certificate as default certificate, throw Pib::Error
-  BOOST_REQUIRE_THROW(key11.setDefaultCertificate(id1Key1Cert1.getName()), Pib::Error);
+  BOOST_CHECK_THROW(key11.setDefaultCertificate(id1Key1Cert1.getName()), Pib::Error);
 
   // add certificate
   key11.addCertificate(id1Key1Cert1);
-  BOOST_CHECK_NO_THROW(key11.getCertificate(id1Key1Cert1.getName()));
+  const auto& addedCert = key11.getCertificate(id1Key1Cert1.getName());
+  BOOST_CHECK_EQUAL(addedCert, id1Key1Cert1);
 
   // new certificate becomes default certificate when there was no default certificate
-  BOOST_REQUIRE_NO_THROW(key11.getDefaultCertificate());
   const auto& defaultCert0 = key11.getDefaultCertificate();
-  BOOST_CHECK_EQUAL(defaultCert0.getName(), id1Key1Cert1.getName());
   BOOST_CHECK_EQUAL(defaultCert0, id1Key1Cert1);
 
   // remove certificate
@@ -91,23 +88,16 @@ BOOST_AUTO_TEST_CASE(CertificateOperation)
 
   // set default certificate directly
   BOOST_REQUIRE_NO_THROW(key11.setDefaultCertificate(id1Key1Cert1));
-  BOOST_REQUIRE_NO_THROW(key11.getDefaultCertificate());
-  BOOST_CHECK_NO_THROW(key11.getCertificate(id1Key1Cert1.getName()));
-
-  // check default cert
   const auto& defaultCert1 = key11.getDefaultCertificate();
-  BOOST_CHECK_EQUAL(defaultCert1.getName(), id1Key1Cert1.getName());
   BOOST_CHECK_EQUAL(defaultCert1, id1Key1Cert1);
 
   // add another certificate
   key11.addCertificate(id1Key1Cert2);
   BOOST_CHECK_EQUAL(key11.getCertificates().size(), 2);
 
-  // set default certificate through name
-  BOOST_REQUIRE_NO_THROW(key11.setDefaultCertificate(id1Key1Cert2.getName()));
-  BOOST_REQUIRE_NO_THROW(key11.getDefaultCertificate());
+  // set default certificate through name and check return value
+  BOOST_CHECK_EQUAL(key11.setDefaultCertificate(id1Key1Cert2.getName()), id1Key1Cert2);
   const auto& defaultCert2 = key11.getDefaultCertificate();
-  BOOST_CHECK_EQUAL(defaultCert2.getName(), id1Key1Cert2.getName());
   BOOST_CHECK_EQUAL(defaultCert2, id1Key1Cert2);
 
   // remove certificate
@@ -115,10 +105,9 @@ BOOST_AUTO_TEST_CASE(CertificateOperation)
   BOOST_CHECK_THROW(key11.getCertificate(id1Key1Cert1.getName()), Pib::Error);
   BOOST_CHECK_EQUAL(key11.getCertificates().size(), 1);
 
-  // set default certificate directly again, change the default setting
+  // set removed certificate as default, certificate is implicitly added
   BOOST_REQUIRE_NO_THROW(key11.setDefaultCertificate(id1Key1Cert1));
   const auto& defaultCert3 = key11.getDefaultCertificate();
-  BOOST_CHECK_EQUAL(defaultCert3.getName(), id1Key1Cert1.getName());
   BOOST_CHECK_EQUAL(defaultCert3, id1Key1Cert1);
   BOOST_CHECK_EQUAL(key11.getCertificates().size(), 2);
 
@@ -128,8 +117,8 @@ BOOST_AUTO_TEST_CASE(CertificateOperation)
   BOOST_CHECK_EQUAL(key11.getCertificates().size(), 1);
   key11.removeCertificate(id1Key1Cert2.getName());
   BOOST_CHECK_THROW(key11.getCertificate(id1Key1Cert2.getName()), Pib::Error);
-  BOOST_CHECK_THROW(key11.getDefaultCertificate(), Pib::Error);
   BOOST_CHECK_EQUAL(key11.getCertificates().size(), 0);
+  BOOST_CHECK_THROW(key11.getDefaultCertificate(), Pib::Error);
 }
 
 class OverwriteFixture : public ndn::security::tests::PibDataFixture,
@@ -158,17 +147,15 @@ BOOST_FIXTURE_TEST_CASE(Overwrite, OverwriteFixture)
 
   auto otherCert = id1Key1Cert1;
   SignatureInfo info;
-  info.setValidityPeriod(ValidityPeriod(time::system_clock::now(),
-                                        time::system_clock::now() + 1_s));
+  info.setValidityPeriod(ValidityPeriod::makeRelative(-1_s, 10_s));
   m_keyChain.sign(otherCert, SigningInfo().setSignatureInfo(info));
 
-  BOOST_CHECK_EQUAL(otherCert.getName(), id1Key1Cert1.getName());
-  BOOST_CHECK(otherCert.getContent() == id1Key1Cert1.getContent());
-  BOOST_CHECK_NE(otherCert, id1Key1Cert1);
+  BOOST_TEST(otherCert.getName() == id1Key1Cert1.getName());
+  BOOST_TEST(otherCert.getContent() == id1Key1Cert1.getContent());
+  BOOST_TEST(otherCert != id1Key1Cert1);
 
   key1.addCertificate(otherCert);
-
-  BOOST_CHECK_EQUAL(key1.getCertificate(id1Key1Cert1.getName()), otherCert);
+  BOOST_TEST(key1.getCertificate(id1Key1Cert1.getName()) == otherCert);
 }
 
 BOOST_AUTO_TEST_CASE(Errors)

tests/unit/security/pib/key-container.t.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2021 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -21,7 +21,6 @@
 
 #include "ndn-cxx/security/pib/key-container.hpp"
 #include "ndn-cxx/security/pib/impl/pib-memory.hpp"
-#include "ndn-cxx/security/pib/pib.hpp"
 
 #include "tests/boost-test.hpp"
 #include "tests/unit/security/pib/pib-data-fixture.hpp"
@@ -37,94 +36,86 @@ BOOST_AUTO_TEST_SUITE(Security)
 BOOST_AUTO_TEST_SUITE(Pib)
 BOOST_FIXTURE_TEST_SUITE(TestKeyContainer, PibDataFixture)
 
-using pib::Pib;
-
-BOOST_AUTO_TEST_CASE(Basic)
+BOOST_AUTO_TEST_CASE(AddGetRemove)
 {
   auto pibImpl = make_shared<PibMemory>();
 
-  // start with an empty container
-  KeyContainer container(id1, pibImpl);
-  BOOST_CHECK_EQUAL(container.size(), 0);
-  BOOST_CHECK_EQUAL(container.getLoadedKeys().size(), 0);
+  {
+    // start with an empty container
+    KeyContainer container(id1, pibImpl);
+    BOOST_CHECK_EQUAL(container.size(), 0);
+    BOOST_CHECK_EQUAL(container.m_keys.size(), 0);
 
-  // add the first key
-  Key key11 = container.add(id1Key1, id1Key1Name);
-  BOOST_CHECK_EQUAL(key11.getName(), id1Key1Name);
-  BOOST_CHECK_EQUAL_COLLECTIONS(key11.getPublicKey().begin(), key11.getPublicKey().end(),
-                                id1Key1.begin(), id1Key1.end());
-  BOOST_CHECK_EQUAL(container.size(), 1);
-  BOOST_CHECK_EQUAL(container.getLoadedKeys().size(), 1);
-  BOOST_CHECK(container.find(id1Key1Name) != container.end());
+    // add the first key
+    Key key11 = container.add(id1Key1, id1Key1Name);
+    BOOST_CHECK_EQUAL(key11.getName(), id1Key1Name);
+    BOOST_TEST(key11.getPublicKey() == id1Key1, boost::test_tools::per_element());
+    BOOST_CHECK_EQUAL(container.size(), 1);
+    BOOST_CHECK_EQUAL(container.m_keys.size(), 1);
+    BOOST_CHECK(container.find(id1Key1Name) != container.end());
 
-  // add the same key again
-  Key key12 = container.add(id1Key1, id1Key1Name);
-  BOOST_CHECK_EQUAL(key12.getName(), id1Key1Name);
-  BOOST_CHECK_EQUAL_COLLECTIONS(key12.getPublicKey().begin(), key12.getPublicKey().end(),
-                                id1Key1.begin(), id1Key1.end());
-  BOOST_CHECK_EQUAL(container.size(), 1);
-  BOOST_CHECK_EQUAL(container.getLoadedKeys().size(), 1);
-  BOOST_CHECK(container.find(id1Key1Name) != container.end());
+    // add the same key again
+    Key key12 = container.add(id1Key1, id1Key1Name);
+    BOOST_CHECK_EQUAL(key12.getName(), id1Key1Name);
+    BOOST_TEST(key12.getPublicKey() == id1Key1, boost::test_tools::per_element());
+    BOOST_CHECK_EQUAL(container.size(), 1);
+    BOOST_CHECK_EQUAL(container.m_keys.size(), 1);
+    BOOST_CHECK(container.find(id1Key1Name) != container.end());
 
-  // add the second key
-  Key key21 = container.add(id1Key2, id1Key2Name);
-  BOOST_CHECK_EQUAL(key21.getName(), id1Key2Name);
-  BOOST_CHECK_EQUAL_COLLECTIONS(key21.getPublicKey().begin(), key21.getPublicKey().end(),
-                                id1Key2.begin(), id1Key2.end());
-  BOOST_CHECK_EQUAL(container.size(), 2);
-  BOOST_CHECK_EQUAL(container.getLoadedKeys().size(), 2);
-  BOOST_CHECK(container.find(id1Key1Name) != container.end());
-  BOOST_CHECK(container.find(id1Key2Name) != container.end());
+    // add the second key
+    Key key21 = container.add(id1Key2, id1Key2Name);
+    BOOST_CHECK_EQUAL(key21.getName(), id1Key2Name);
+    BOOST_TEST(key21.getPublicKey() == id1Key2, boost::test_tools::per_element());
+    BOOST_CHECK_EQUAL(container.size(), 2);
+    BOOST_CHECK_EQUAL(container.m_keys.size(), 2);
+    BOOST_CHECK(container.find(id1Key1Name) != container.end());
+    BOOST_CHECK(container.find(id1Key2Name) != container.end());
 
-  // get keys
-  BOOST_REQUIRE_NO_THROW(container.get(id1Key1Name));
-  BOOST_REQUIRE_NO_THROW(container.get(id1Key2Name));
-  Name id1Key3Name = constructKeyName(id1, name::Component("non-existing-id"));
-  BOOST_CHECK_THROW(container.get(id1Key3Name), Pib::Error);
+    // check keys
+    Key key1 = container.get(id1Key1Name);
+    Key key2 = container.get(id1Key2Name);
+    BOOST_CHECK_EQUAL(key1.getName(), id1Key1Name);
+    BOOST_TEST(key1.getPublicKey() == id1Key1, boost::test_tools::per_element());
+    BOOST_CHECK_EQUAL(key2.getName(), id1Key2Name);
+    BOOST_TEST(key2.getPublicKey() == id1Key2, boost::test_tools::per_element());
+    Name id1Key3Name = constructKeyName(id1, name::Component("non-existing-id"));
+    BOOST_CHECK_THROW(container.get(id1Key3Name), pib::Pib::Error);
+  }
 
-  // check key
-  Key key1 = container.get(id1Key1Name);
-  Key key2 = container.get(id1Key2Name);
-  BOOST_CHECK_EQUAL(key1.getName(), id1Key1Name);
-  BOOST_CHECK_EQUAL_COLLECTIONS(key1.getPublicKey().begin(), key1.getPublicKey().end(),
-                                id1Key1.begin(), id1Key1.end());
-  BOOST_CHECK_EQUAL(key2.getName(), id1Key2Name);
-  BOOST_CHECK_EQUAL_COLLECTIONS(key2.getPublicKey().begin(), key2.getPublicKey().end(),
-                                id1Key2.begin(), id1Key2.end());
+  {
+    // create a container from an existing (non-empty) PibImpl
+    // names are loaded immediately but the key cache should initially be empty
+    KeyContainer container2(id1, pibImpl);
+    BOOST_CHECK_EQUAL(container2.size(), 2);
+    BOOST_CHECK_EQUAL(container2.m_keys.size(), 0);
 
-  // create another container from the same PibImpl
-  // cache should be empty
-  KeyContainer container2(id1, pibImpl);
-  BOOST_CHECK_EQUAL(container2.size(), 2);
-  BOOST_CHECK_EQUAL(container2.getLoadedKeys().size(), 0);
+    // fetching the keys should populate the cache
+    BOOST_CHECK_EQUAL(container2.get(id1Key1Name).getName(), id1Key1Name);
+    BOOST_CHECK_EQUAL(container2.size(), 2);
+    BOOST_CHECK_EQUAL(container2.m_keys.size(), 1);
 
-  // get key, cache should be filled
-  BOOST_REQUIRE_NO_THROW(container2.get(id1Key1Name));
-  BOOST_CHECK_EQUAL(container2.size(), 2);
-  BOOST_CHECK_EQUAL(container2.getLoadedKeys().size(), 1);
+    BOOST_CHECK_EQUAL(container2.get(id1Key2Name).getName(), id1Key2Name);
+    BOOST_CHECK_EQUAL(container2.size(), 2);
+    BOOST_CHECK_EQUAL(container2.m_keys.size(), 2);
 
-  BOOST_REQUIRE_NO_THROW(container2.get(id1Key2Name));
-  BOOST_CHECK_EQUAL(container2.size(), 2);
-  BOOST_CHECK_EQUAL(container2.getLoadedKeys().size(), 2);
+    // remove a key
+    container2.remove(id1Key1Name);
+    BOOST_CHECK_EQUAL(container2.size(), 1);
+    BOOST_CHECK_EQUAL(container2.m_keys.size(), 1);
+    BOOST_CHECK(container2.find(id1Key1Name) == container2.end());
+    BOOST_CHECK(container2.find(id1Key2Name) != container2.end());
 
-  // remove a key
-  container2.remove(id1Key1Name);
-  BOOST_CHECK_EQUAL(container2.size(), 1);
-  BOOST_CHECK_EQUAL(container2.getLoadedKeys().size(), 1);
-  BOOST_CHECK(container2.find(id1Key1Name) == container2.end());
-  BOOST_CHECK(container2.find(id1Key2Name) != container2.end());
-
-  // remove another key
-  container2.remove(id1Key2Name);
-  BOOST_CHECK_EQUAL(container2.size(), 0);
-  BOOST_CHECK_EQUAL(container2.getLoadedKeys().size(), 0);
-  BOOST_CHECK(container2.find(id1Key2Name) == container2.end());
+    // remove another key
+    container2.remove(id1Key2Name);
+    BOOST_CHECK_EQUAL(container2.size(), 0);
+    BOOST_CHECK_EQUAL(container2.m_keys.size(), 0);
+    BOOST_CHECK(container2.find(id1Key2Name) == container2.end());
+  }
 }
 
 BOOST_AUTO_TEST_CASE(Errors)
 {
   auto pibImpl = make_shared<PibMemory>();
-
   KeyContainer container(id1, pibImpl);
 
   BOOST_CHECK_THROW(container.add(id2Key1, id2Key1Name), std::invalid_argument);
@@ -136,16 +127,13 @@ BOOST_AUTO_TEST_CASE(Iterator)
 {
   auto pibImpl = make_shared<PibMemory>();
   KeyContainer container(id1, pibImpl);
-
   container.add(id1Key1, id1Key1Name);
   container.add(id1Key2, id1Key2Name);
 
-  std::set<Name> keyNames;
-  keyNames.insert(id1Key1Name);
-  keyNames.insert(id1Key2Name);
+  const std::set<Name> keyNames{id1Key1Name, id1Key2Name};
 
   KeyContainer::const_iterator it = container.begin();
-  std::set<Name>::const_iterator testIt = keyNames.begin();
+  auto testIt = keyNames.begin();
   BOOST_CHECK_EQUAL((*it).getName(), *testIt);
   it++;
   testIt++;
@@ -154,7 +142,8 @@ BOOST_AUTO_TEST_CASE(Iterator)
   testIt++;
   BOOST_CHECK(it == container.end());
 
-  size_t count = 0;
+  // test range-based for
+  int count = 0;
   testIt = keyNames.begin();
   for (const auto& key : container) {
     BOOST_CHECK_EQUAL(key.getIdentity(), id1);

tests/unit/security/pib/key.t.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2021 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -41,36 +41,42 @@ BOOST_FIXTURE_TEST_SUITE(TestKey, PibDataFixture)
 BOOST_AUTO_TEST_CASE(ValidityChecking)
 {
   Key key;
-  BOOST_CHECK(!key);
-  BOOST_CHECK_EQUAL(static_cast<bool>(key), false);
+  BOOST_TEST(!key);
+  BOOST_TEST(key == Key());
 
-  auto keyImpl = std::make_shared<detail::KeyImpl>(id1Key1Name, id1Key1,
-                                                   std::make_shared<pib::PibMemory>());
-  key = Key(keyImpl);
-  BOOST_CHECK(key);
-  BOOST_CHECK_EQUAL(!key, false);
+  auto impl = std::make_shared<detail::KeyImpl>(id1Key1Name, id1Key1, std::make_shared<pib::PibMemory>());
+  key = Key(impl);
+  BOOST_TEST(key);
+  BOOST_TEST(key != Key());
+
+  impl.reset();
+  BOOST_TEST(!key);
 }
 
-// pib::Key is a wrapper of pib::detail::KeyImpl.  Since the functionalities of KeyImpl
-// have already been tested in detail/key-impl.t.cpp, we only test the shared property
-// of pib::Key in this test case.
+// pib::Key is a wrapper of pib::detail::KeyImpl. Since the functionality of KeyImpl is
+// already tested in key-impl.t.cpp, we only test the shared property of pib::Key in
+// this test case.
 BOOST_AUTO_TEST_CASE(SharedImpl)
 {
   auto keyImpl = std::make_shared<detail::KeyImpl>(id1Key1Name, id1Key1,
                                                    std::make_shared<pib::PibMemory>());
   Key key1(keyImpl);
   Key key2(keyImpl);
-  BOOST_CHECK_EQUAL(key1, key2);
-  BOOST_CHECK_NE(key1, Key());
-  BOOST_CHECK_EQUAL(Key(), Key());
 
+  BOOST_TEST(key1 == key2);
+  BOOST_TEST(key1 != Key());
+  BOOST_TEST(Key() != key2);
+  BOOST_TEST(Key() == Key());
+
+  BOOST_CHECK_THROW(key2.getCertificate(id1Key1Cert1.getName()), pib::Pib::Error);
   key1.addCertificate(id1Key1Cert1);
-  BOOST_CHECK_NO_THROW(key2.getCertificate(id1Key1Cert1.getName()));
+  BOOST_TEST(key2.getCertificate(id1Key1Cert1.getName()) == id1Key1Cert1);
+
   key2.removeCertificate(id1Key1Cert1.getName());
   BOOST_CHECK_THROW(key1.getCertificate(id1Key1Cert1.getName()), pib::Pib::Error);
 
   key1.setDefaultCertificate(id1Key1Cert1);
-  BOOST_CHECK_NO_THROW(key2.getDefaultCertificate());
+  BOOST_TEST(key2.getDefaultCertificate() == id1Key1Cert1);
 }
 
 BOOST_AUTO_TEST_CASE(Helpers)