Made encryptionKey parameter of Catalogue::createTape() optional

This commit also enforces the interdiction of empty strings being
bound as string database parameters.  Either a non-empty string is
bound or no value at all is bound (nullopt).

catalogue/Catalogue.hpp

@@ -159,13 +159,17 @@ public:
   /**
    * Creates a tape which is assumed to have logical block protection (LBP)
    * enabled.
+   *
+   * @param encryptionKey The optional identifier of the encrption key.  This
+   * optional parameter should either have a non-empty string value or no value
+   * at all.  Empty strings are prohibited.
    */
   virtual void createTape(
     const common::dataStructures::SecurityIdentity &cliIdentity,
     const std::string &vid,
     const std::string &logicalLibraryName,
     const std::string &tapePoolName,
-    const std::string &encryptionKey,
+    const optional<std::string> &encryptionKey,
     const uint64_t capacityInBytes,
     const bool disabled,
     const bool full,

catalogue/RdbmsCatalogue.cpp

@@ -1074,12 +1074,17 @@ void RdbmsCatalogue::createTape(
   const std::string &vid,
   const std::string &logicalLibraryName,
   const std::string &tapePoolName,
-  const std::string &encryptionKey,
+  const optional<std::string> &encryptionKey,
   const uint64_t capacityInBytes,
   const bool disabled,
   const bool full,
   const std::string &comment) {
   try {
+    if(encryptionKey && encryptionKey.value().empty()) {
+      throw(exception::Exception(std::string("The identifier of the encrption key for tape ") + vid + " has been set "
+        "to the empty string.  This optional value should either have a non-empty string value or no value at all"));
+    }
+
     auto conn = m_connPool.getConn();
     if(tapeExists(*conn, vid)) {
       throw exception::UserError(std::string("Cannot create tape ") + vid +
@@ -1134,7 +1139,7 @@ void RdbmsCatalogue::createTape(
     stmt->bindString(":VID", vid);
     stmt->bindString(":LOGICAL_LIBRARY_NAME", logicalLibraryName);
     stmt->bindString(":TAPE_POOL_NAME", tapePoolName);
-    stmt->bindString(":ENCRYPTION_KEY", encryptionKey);
+    stmt->bindOptionalString(":ENCRYPTION_KEY", encryptionKey);
     stmt->bindUint64(":CAPACITY_IN_BYTES", capacityInBytes);
     stmt->bindUint64(":DATA_IN_BYTES", 0);
     stmt->bindUint64(":LAST_FSEQ", 0);

catalogue/RdbmsCatalogue.hpp

@@ -147,13 +147,17 @@ public:
   /**
    * Creates a tape which is assumed to have logical block protection (LBP)
    * enabled.
+   *
+   * @param encryptionKey The optional identifier of the encrption key.  This
+   * optional parameter should either have a non-empty string value or no value
+   * at all.  Empty strings are prohibited.
    */
   virtual void createTape(
     const common::dataStructures::SecurityIdentity &cliIdentity,
     const std::string &vid,
     const std::string &logicalLibraryName,
     const std::string &tapePoolName,
-    const std::string &encryptionKey,
+    const optional<std::string> &encryptionKey,
     const uint64_t capacityInBytes,
     const bool disabled,
     const bool full,

common/dataStructures/Tape.hpp

@@ -48,7 +48,15 @@ struct Tape {
   std::string tapePoolName;
   uint64_t capacityInBytes;
   uint64_t dataOnTapeInBytes;
+
+  /**
+   * The optional identifier of the encrption key.
+   *
+   * This optional should either have a non-empty string value or no value at
+   * all.  Empty strings are prohibited.
+   */
   optional<std::string> encryptionKey;
+
   bool lbp;
   bool full;
   bool disabled;

rdbms/OcciStmt.cpp

@@ -102,8 +102,28 @@ void OcciStmt::bindUint64(const std::string &paramName, const uint64_t paramValu
 //------------------------------------------------------------------------------
 void OcciStmt::bindString(const std::string &paramName, const std::string &paramValue) {
   try {
+    bindOptionalString(paramName, paramValue);
+  } catch(exception::Exception &ex) {
+    throw exception::Exception(std::string(__FUNCTION__) + " failed: " + ex.getMessage().str());
+  }
+}
+
+//------------------------------------------------------------------------------
+// bindOptionalString
+//------------------------------------------------------------------------------
+void OcciStmt::bindOptionalString(const std::string &paramName, const optional<std::string> &paramValue) {
+  try {
+    if(paramValue && paramValue.value().empty()) {
+      throw exception::Exception(std::string("Optional string parameter ") + paramName + " is an empty string. "
+        " An optional string parameter should either have a non-empty string value or no value at all."); 
+    }
+
     const unsigned paramIdx = m_paramNameToIdx.getIdx(paramName);
-    m_stmt->setString(paramIdx, paramValue);
+    if(paramValue) {
+      m_stmt->setString(paramIdx, paramValue.value());
+    } else {
+      m_stmt->setString(paramIdx, nullptr);
+    }
   } catch(exception::Exception &ex) {
     throw exception::Exception(std::string(__FUNCTION__) + " failed for SQL statement " + getSql() + ": " +
       ex.getMessage().str());

rdbms/OcciStmt.hpp

@@ -86,14 +86,30 @@ public:
    */
   virtual void bindUint64(const std::string &paramName, const uint64_t paramValue) override;
 
-  /**
-   * Binds an SQL parameter.
+  /** 
+   * Binds an SQL parameter of type string.
+   *
+   * Please note that this method will throw an exception if the string
+   * parameter is empty.  If a null value is to be bound then the
+   * bindOptionalString() method should be used.
    *
    * @param paramName The name of the parameter.
    * @param paramValue The value to be bound.
-   */
+   */ 
   virtual void bindString(const std::string &paramName, const std::string &paramValue) override;
 
+  /** 
+   * Binds an SQL parameter of type optional-string.
+   *
+   * Please note that this method will throw an exception if the optional string
+   * parameter has the empty string as its value.  An optional string parameter
+   * should either have a non-empty string value or no value at all.
+   *
+   * @param paramName The name of the parameter.
+   * @param paramValue The value to be bound.
+   */ 
+  virtual void bindOptionalString(const std::string &paramName, const optional<std::string> &paramValue) override;
+
   /**
    *  Executes the statement and returns the result set.
    *

rdbms/SqliteStmt.cpp

@@ -101,12 +101,36 @@ void SqliteStmt::bindUint64(const std::string &paramName, const uint64_t paramVa
 // bind
 //------------------------------------------------------------------------------
 void SqliteStmt::bindString(const std::string &paramName, const std::string &paramValue) {
-  const unsigned int paramIdx = m_paramNameToIdx.getIdx(paramName);
-  const int bindRc = paramValue.empty() ?
-    sqlite3_bind_text(m_stmt, paramIdx, nullptr, 0, SQLITE_TRANSIENT) :
-    sqlite3_bind_text(m_stmt, paramIdx, paramValue.c_str(), -1, SQLITE_TRANSIENT);
-  if(SQLITE_OK != bindRc) {
-    throw exception::Exception(std::string(__FUNCTION__) + "failed for SQL statement " + getSql());
+  try {
+    bindOptionalString(paramName, paramValue);
+  } catch(exception::Exception &ex) {
+    throw exception::Exception(std::string(__FUNCTION__) + " failed for SQL statement " + getSql() + ": " +
+      ex.getMessage().str()); 
+  }
+}
+
+//------------------------------------------------------------------------------
+// bindOptionalString
+//------------------------------------------------------------------------------
+void SqliteStmt::bindOptionalString(const std::string &paramName, const optional<std::string> &paramValue) {
+  try {
+    if(paramValue && paramValue.value().empty()) {
+      throw exception::Exception(std::string("Optional string parameter ") + paramName + " is an empty string. "
+        " An optional string parameter should either have a non-empty string value or no value at all.");
+    }
+    const unsigned int paramIdx = m_paramNameToIdx.getIdx(paramName);
+    int bindRc = 0;
+    if(paramValue) {
+      bindRc = sqlite3_bind_text(m_stmt, paramIdx, paramValue.value().c_str(), -1, SQLITE_TRANSIENT);
+    } else {    
+      bindRc = sqlite3_bind_text(m_stmt, paramIdx, nullptr, 0, SQLITE_TRANSIENT);
+    }
+    if(SQLITE_OK != bindRc) {
+      throw exception::Exception(getSql());
+    }
+  } catch(exception::Exception &ex) {
+    throw exception::Exception(std::string(__FUNCTION__) + " failed for SQL statement " + getSql() + ": " +
+      ex.getMessage().str()); 
   }
 }
 

rdbms/SqliteStmt.hpp

@@ -84,13 +84,29 @@ public:
   virtual void bindUint64(const std::string &paramName, const uint64_t paramValue) override;
 
   /** 
-   * Binds an SQL parameter.
+   * Binds an SQL parameter of type string.
+   *
+   * Please note that this method will throw an exception if the string
+   * parameter is empty.  If a null value is to be bound then the
+   * bindOptionalString() method should be used.
    *
    * @param paramName The name of the parameter.
    * @param paramValue The value to be bound.
    */ 
   virtual void bindString(const std::string &paramName, const std::string &paramValue) override;
 
+  /** 
+   * Binds an SQL parameter of type optional-string.
+   *
+   * Please note that this method will throw an exception if the optional string
+   * parameter has the empty string as its value.  An optional string parameter
+   * should either have a non-empty string value or no value at all.
+   *
+   * @param paramName The name of the parameter.
+   * @param paramValue The value to be bound.
+   */ 
+  virtual void bindOptionalString(const std::string &paramName, const optional<std::string> &paramValue) override;
+
   /**
    * Executes the statement and returns the result set.
    *

rdbms/Stmt.hpp

@@ -18,6 +18,7 @@
 
 #pragma once
 
+#include "common/optional.hpp"
 #include "rdbms/Rset.hpp"
 
 #include <memory>
@@ -59,13 +60,29 @@ public:
   virtual void bindUint64(const std::string &paramName, const uint64_t paramValue) = 0;
 
   /** 
-   * Binds an SQL parameter.
+   * Binds an SQL parameter of type string.
+   *
+   * Please note that this method will throw an exception if the string
+   * parameter is empty.  If a null value is to be bound then the
+   * bindOptionalString() method should be used.
    *
    * @param paramName The name of the parameter.
    * @param paramValue The value to be bound.
    */ 
   virtual void bindString(const std::string &paramName, const std::string &paramValue) = 0;
 
+  /** 
+   * Binds an SQL parameter of type optional-string.
+   *
+   * Please note that this method will throw an exception if the optional string
+   * parameter has the empty string as its value.  An optional string parameter
+   * should either have a non-empty string value or no value at all.
+   *
+   * @param paramName The name of the parameter.
+   * @param paramValue The value to be bound.
+   */ 
+  virtual void bindOptionalString(const std::string &paramName, const optional<std::string> &paramValue) = 0;
+
   /**
    *  Executes the statement and returns the result set.
    *

scheduler/SchedulerTest.cpp

@@ -391,8 +391,8 @@ TEST_P(SchedulerTest, archive_and_retrieve_new_file) {
   const std::string tapeComment = "Tape comment";
   bool notDisabled = false;
   bool notFull = false;
-  catalogue.createTape(s_adminOnAdminHost, s_vid, s_libraryName,
-    s_tapePoolName, "", capacityInBytes, notDisabled, notFull, tapeComment);
+  catalogue.createTape(s_adminOnAdminHost, s_vid, s_libraryName, s_tapePoolName, nullopt, capacityInBytes,
+    notDisabled, notFull, tapeComment);
 
   {
     // Emulate a tape server by asking for a mount and then a file (and succeed
@@ -528,8 +528,8 @@ TEST_P(SchedulerTest, retry_archive_until_max_reached) {
   const std::string tapeComment = "Tape comment";
   bool notDisabled = false;
   bool notFull = false;
-  catalogue.createTape(s_adminOnAdminHost, s_vid, s_libraryName,
-    s_tapePoolName, "", capacityInBytes, notDisabled, notFull, tapeComment);
+  catalogue.createTape(s_adminOnAdminHost, s_vid, s_libraryName, s_tapePoolName, nullopt, capacityInBytes, notDisabled,
+    notFull, tapeComment);
   
   {
     // Emulate a tape server by asking for a mount and then a file

tapeserver/castor/tape/tapeserver/daemon/DataTransferSessionTest.cpp

@@ -352,8 +352,8 @@ TEST_P(DataTransferSessionTest, DataTransferSessionGooddayRecall) {
   const std::string tapeComment = "Tape comment";
   bool notDisabled = false;
   bool notFull = false;
-  catalogue.createTape(s_adminOnAdminHost, s_vid, s_libraryName,
-    s_tapePoolName, "", capacityInBytes, notDisabled, notFull, tapeComment);
+  catalogue.createTape(s_adminOnAdminHost, s_vid, s_libraryName, s_tapePoolName, cta::nullopt, capacityInBytes,
+    notDisabled, notFull, tapeComment);
   
   // 6) Prepare files for reading by writing them to the mock system
   {