Synchronize with SQLite table

src/ripple/app/main/DBInit.cpp

@@ -133,9 +133,9 @@ const char* WalletDBInit[] =
 
     // Peer reservations.
     // REVIEWER: How do we handle table migrations if we need to add a column?
-    "CREATE TABLE IF NOT EXISTS PeerReservations (   \
-        PublicKey       CHARACTER(53) NOT NULL,      \
-        Name            CHARACTER(32) NULL,          \
+    "CREATE TABLE IF NOT EXISTS PeerReservations (     \
+        PublicKey       CHARACTER(53) UNIQUE NOT NULL, \
+        Name            CHARACTER(32) NULL,            \
     );",
 
     // Validator Manifests

src/ripple/overlay/PeerReservationTable.h

@@ -36,7 +36,7 @@ namespace ripple {
 struct PeerReservation
 {
 public:
-    PublicKey const identity_;
+    PublicKey const nodeId_;
     boost::optional<std::string> name_;
 };
 
@@ -58,6 +58,22 @@ class PeerReservationTable {
     // Our dependencies are not prepared until the second phase.
     bool init(DatabaseCon& connection);
 
+    /**
+     * @return true iff the node did not already have a reservation
+     */
+    // REVIEWER: Without taking any special effort, this function can throw
+    // because its dependencies can throw. Do we want to try a different error
+    // mechanism? Perhaps Boost.Outcome in anticipation of Herbceptions?
+    bool upsert(
+            PublicKey const& nodeId,
+            boost::optional<std::string> const& name
+    );
+
+    /**
+     * @return true iff the node had a reservation.
+     */
+    bool erase(PublicKey const& nodeId);
+
 private:
     beast::Journal mutable journal_;
     // REVIEWER: What is the policy on forward declarations? We can use one

src/ripple/overlay/impl/PeerReservationTable.cpp

@@ -35,7 +35,7 @@ namespace ripple {
 bool
 PeerReservationTable::load(DatabaseCon& connection)
 {
-    auto db = connection.checkoutDb();
+    auto db = connection_->checkoutDb();
 
     boost::optional<std::string> valPubKey, valName;
     // REVIEWER: We should really abstract the table and column names into
@@ -62,11 +62,44 @@ PeerReservationTable::load(DatabaseCon& connection)
             }
             // TODO: Remove any invalid public keys?
         }
-        PeerReservation const res{.identity_ = *pk, .name_ = valName};
+        PeerReservation const res{.nodeId_ = *pk, .name_ = valName};
         table_.emplace(*pk, res);
     }
 
     return true;
 }
 
+bool PeerReservationTable::upsert(
+        PublicKey const& nodeId,
+        boost::optional<std::string> const& name
+)
+{
+    auto emplaced = table_.emplace(
+        std::make_pair(nodeId, PeerReservation{nodeId, name}));
+    if (!emplaced.second)
+        // The node already has a reservation.
+        // I think most people just want to overwrite existing reservations.
+        // TODO: Make a formal team decision that we do not want to raise an
+        // error upon a collision.
+        emplaced.first->second.name_ = name;
+
+    auto db = connection_->checkoutDb();
+    *db << "INSERT INTO PeerReservations (PublicKey, Name) "
+        "VALUES (:nodeId, :name) "
+        "ON CONFLICT(PublicKey) DO UPDATE SET Name=excluded.Name",
+        use(nodeId), use(name);
+
+    return emplaced.second;
+}
+
+bool PeerReservationTable::erase(PublicKey const& nodeId)
+{
+    bool const removed = table_.erase(nodeId) > 0;
+
+    auto db = connection_->checkoutDb();
+    *db << "DELETE FROM PeerReservations WHERE PublicKey = :nodeId", use(nodeId);
+
+    return removed;
+}
+
 }  // namespace ripple

src/ripple/rpc/handlers/Reservations.cpp

@@ -71,26 +71,37 @@ doReservationsAdd(RPC::Context& context)
         params[jss::public_key].asString());
     if (!optPk)
         return rpcError(rpcPUBLIC_MALFORMED);
-    PublicKey const& identity = *optPk;
+    PublicKey const& nodeId = *optPk;
 
-    // I think most people just want to overwrite existing reservations.
-    // TODO: Make a formal team decision that we do not want to raise an
-    // error upon a collision.
-    auto emplaced = context.app.peerReservations().emplace(
-        std::make_pair(identity, PeerReservation{identity, name}));
-    if (!emplaced.second)
-        // The public key already has a reservation. Overwrite it.
-        emplaced.first->second.name_ = name;
+    bool const added = context.app.peerReservations().upsert(nodeId, name);
 
     Json::Value result;
-    // TODO: Need to indicate whether it was inserted or overwritten.
+    // TODO: Should we indicate whether it was inserted or overwritten?
     return result;
 }
 
 Json::Value
 doReservationsDel(RPC::Context& context)
 {
+    auto const& params = context.params;
+
+    // We repeat much of the parameter parsing from `doReservationsAdd`.
+    if (!params.isMember(jss::public_key))
+        return RPC::missing_field_error(jss::public_key);
+    if (!params[jss::public_key].isString())
+        return RPC::expected_field_error(jss::public_key, "a string");
+
+    boost::optional<PublicKey> optPk = parseBase58<PublicKey>(
+        TokenType::NodePublic,
+        params[jss::public_key].asString());
+    if (!optPk)
+        return rpcError(rpcPUBLIC_MALFORMED);
+    PublicKey const& nodeId = *optPk;
+
+    context.app.peerReservations().erase(nodeId);
+
     Json::Value result;
+    // TODO: Should we indicate whether it existed before?
     return result;
 }