variable renaming and sqlite code optimization

Change-Id: Ida2c854f87c528820945bc6246678d87543b1ed9
diff --git a/src/ca-module.cpp b/src/ca-module.cpp
index 0cd35cc..00d2208 100644
--- a/src/ca-module.cpp
+++ b/src/ca-module.cpp
@@ -336,28 +336,28 @@
 
   // create new request instance
   std::string requestId = std::to_string(random::generateWord64());
-  RequestState certRequest(m_config.m_caPrefix, requestId, requestType, Status::BEFORE_CHALLENGE, *clientCert,
+  RequestState requestState(m_config.m_caPrefix, requestId, requestType, Status::BEFORE_CHALLENGE, *clientCert,
           makeBinaryBlock(tlv::ContentType_Key, aesKey, sizeof(aesKey)));
-  m_storage->addRequest(certRequest);
+  m_storage->addRequest(requestState);
   Data result;
   result.setName(request.getName());
   result.setFreshnessPeriod(DEFAULT_DATA_FRESHNESS_PERIOD);
   if (requestType == RequestType::NEW) {
     result.setContent(NEW::encodeDataContent(myEcdhPubKeyBase64,
                                              std::to_string(saltInt),
-                                             certRequest,
+                                             requestState,
                                              m_config.m_supportedChallenges));
   }
   else if (requestType == RequestType::REVOKE) {
     result.setContent(REVOKE::encodeDataContent(myEcdhPubKeyBase64,
                                                 std::to_string(saltInt),
-                                                certRequest,
+                                                requestState,
                                                 m_config.m_supportedChallenges));
   }
   m_keyChain.sign(result, signingByIdentity(m_config.m_caPrefix));
   m_face.put(result);
   if (m_config.m_statusUpdateCallback) {
-    m_config.m_statusUpdateCallback(certRequest);
+    m_config.m_statusUpdateCallback(requestState);
   }
 }
 
@@ -365,15 +365,15 @@
 CaModule::onChallenge(const Interest& request)
 {
   // get certificate request state
-  RequestState certRequest = getCertificateRequest(request);
-  if (certRequest.m_requestId == "") {
+  RequestState requestState = getCertificateRequest(request);
+  if (requestState.m_requestId == "") {
     _LOG_ERROR("No certificate request state can be found.");
     m_face.put(generateErrorDataPacket(request.getName(), ErrorCode::INVALID_PARAMETER,
                                        "No certificate request state can be found."));
     return;
   }
   // verify signature
-  if (!security::verifySignature(request, certRequest.m_cert)) {
+  if (!security::verifySignature(request, requestState.m_cert)) {
     _LOG_ERROR("Invalid Signature in the Interest packet.");
     m_face.put(generateErrorDataPacket(request.getName(), ErrorCode::BAD_SIGNATURE,
                                        "Invalid Signature in the Interest packet."));
@@ -382,19 +382,19 @@
   // decrypt the parameters
   Buffer paramTLVPayload;
   try {
-    paramTLVPayload = decodeBlockWithAesGcm128(request.getApplicationParameters(), certRequest.m_encryptionKey.value(),
+    paramTLVPayload = decodeBlockWithAesGcm128(request.getApplicationParameters(), requestState.m_encryptionKey.value(),
                                                (uint8_t*)"test", strlen("test"));
   }
   catch (const std::exception& e) {
     _LOG_ERROR("Interest paramaters decryption failed: " << e.what());
-    m_storage->deleteRequest(certRequest.m_requestId);
+    m_storage->deleteRequest(requestState.m_requestId);
     m_face.put(generateErrorDataPacket(request.getName(), ErrorCode::INVALID_PARAMETER,
                                        "Interest paramaters decryption failed."));
     return;
   }
   if (paramTLVPayload.size() == 0) {
     _LOG_ERROR("No parameters are found after decryption.");
-    m_storage->deleteRequest(certRequest.m_requestId);
+    m_storage->deleteRequest(requestState.m_requestId);
     m_face.put(generateErrorDataPacket(request.getName(), ErrorCode::INVALID_PARAMETER,
                                        "No parameters are found after decryption."));
     return;
@@ -407,75 +407,75 @@
   auto challenge = ChallengeModule::createChallengeModule(challengeType);
   if (challenge == nullptr) {
     _LOG_TRACE("Unrecognized challenge type: " << challengeType);
-    m_storage->deleteRequest(certRequest.m_requestId);
+    m_storage->deleteRequest(requestState.m_requestId);
     m_face.put(generateErrorDataPacket(request.getName(), ErrorCode::INVALID_PARAMETER, "Unrecognized challenge type."));
     return;
   }
 
   _LOG_TRACE("CHALLENGE module to be load: " << challengeType);
-  auto errorInfo = challenge->handleChallengeRequest(paramTLV, certRequest);
+  auto errorInfo = challenge->handleChallengeRequest(paramTLV, requestState);
   if (std::get<0>(errorInfo) != ErrorCode::NO_ERROR) {
-    m_storage->deleteRequest(certRequest.m_requestId);
+    m_storage->deleteRequest(requestState.m_requestId);
     m_face.put(generateErrorDataPacket(request.getName(), std::get<0>(errorInfo), std::get<1>(errorInfo)));
     return;
   }
 
   Block payload;
-  if (certRequest.m_status == Status::PENDING) {
+  if (requestState.m_status == Status::PENDING) {
     // if challenge succeeded
-    if (certRequest.m_requestType == RequestType::NEW) {
-      auto issuedCert = issueCertificate(certRequest);
-      certRequest.m_cert = issuedCert;
-      certRequest.m_status = Status::SUCCESS;
-      m_storage->addCertificate(certRequest.m_requestId, issuedCert);
-      m_storage->deleteRequest(certRequest.m_requestId);
+    if (requestState.m_requestType == RequestType::NEW) {
+      auto issuedCert = issueCertificate(requestState);
+        requestState.m_cert = issuedCert;
+        requestState.m_status = Status::SUCCESS;
+      m_storage->addCertificate(requestState.m_requestId, issuedCert);
+      m_storage->deleteRequest(requestState.m_requestId);
 
-      payload = CHALLENGE::encodeDataPayload(certRequest);
+      payload = CHALLENGE::encodeDataPayload(requestState);
       payload.parse();
       payload.push_back(makeNestedBlock(tlv_issued_cert_name, issuedCert.getName()));
       payload.encode();
       _LOG_TRACE("Challenge succeeded. Certificate has been issued: " << issuedCert.getName());
     }
-    else if (certRequest.m_requestType == RequestType::REVOKE) {
-      certRequest.m_status = Status::SUCCESS;
-      m_storage->deleteRequest(certRequest.m_requestId);
+    else if (requestState.m_requestType == RequestType::REVOKE) {
+        requestState.m_status = Status::SUCCESS;
+      m_storage->deleteRequest(requestState.m_requestId);
 
-      payload = CHALLENGE::encodeDataPayload(certRequest);
+      payload = CHALLENGE::encodeDataPayload(requestState);
       _LOG_TRACE("Challenge succeeded. Certificate has been revoked");
     }
   }
   else {
-    m_storage->updateRequest(certRequest);
-    payload = CHALLENGE::encodeDataPayload(certRequest);
+    m_storage->updateRequest(requestState);
+    payload = CHALLENGE::encodeDataPayload(requestState);
     _LOG_TRACE("No failure no success. Challenge moves on");
   }
 
   Data result;
   result.setName(request.getName());
   result.setFreshnessPeriod(DEFAULT_DATA_FRESHNESS_PERIOD);
-  auto contentBlock = encodeBlockWithAesGcm128(tlv::Content, certRequest.m_encryptionKey.value(), payload.value(),
+  auto contentBlock = encodeBlockWithAesGcm128(tlv::Content, requestState.m_encryptionKey.value(), payload.value(),
                                                payload.value_size(), (uint8_t*)"test", strlen("test"));
   result.setContent(contentBlock);
   m_keyChain.sign(result, signingByIdentity(m_config.m_caPrefix));
   m_face.put(result);
   if (m_config.m_statusUpdateCallback) {
-    m_config.m_statusUpdateCallback(certRequest);
+    m_config.m_statusUpdateCallback(requestState);
   }
 }
 
 security::v2::Certificate
-CaModule::issueCertificate(const RequestState& certRequest)
+CaModule::issueCertificate(const RequestState& requestState)
 {
   auto expectedPeriod =
-      certRequest.m_cert.getValidityPeriod().getPeriod();
+      requestState.m_cert.getValidityPeriod().getPeriod();
   security::ValidityPeriod period(expectedPeriod.first, expectedPeriod.second);
   security::v2::Certificate newCert;
 
-  Name certName = certRequest.m_cert.getKeyName();
+  Name certName = requestState.m_cert.getKeyName();
   certName.append("NDNCERT").append(std::to_string(random::generateSecureWord64()));
   newCert.setName(certName);
-  newCert.setContent(certRequest.m_cert.getContent());
-  _LOG_TRACE("cert request content " << certRequest.m_cert);
+  newCert.setContent(requestState.m_cert.getContent());
+  _LOG_TRACE("cert request content " << requestState.m_cert);
   SignatureInfo signatureInfo;
   signatureInfo.setValidityPeriod(period);
   security::SigningInfo signingInfo(security::SigningInfo::SIGNER_TYPE_ID,
@@ -490,7 +490,7 @@
 CaModule::getCertificateRequest(const Interest& request)
 {
   std::string requestId;
-  RequestState certRequest;
+  RequestState requestState;
   try {
     requestId = readString(request.getName().at(m_config.m_caPrefix.size() + 2));
   }
@@ -499,12 +499,12 @@
   }
   try {
     _LOG_TRACE("Request Id to query the database " << requestId);
-    certRequest = m_storage->getRequest(requestId);
+      requestState = m_storage->getRequest(requestId);
   }
   catch (const std::exception& e) {
     _LOG_ERROR("Cannot get certificate request record from the storage: " << e.what());
   }
-  return certRequest;
+  return requestState;
 }
 
 void
diff --git a/src/ca-module.hpp b/src/ca-module.hpp
index c6b3e71..c97e4f3 100644
--- a/src/ca-module.hpp
+++ b/src/ca-module.hpp
@@ -96,7 +96,7 @@
   getCertificateRequest(const Interest& request);
 
   security::v2::Certificate
-  issueCertificate(const RequestState& certRequest);
+  issueCertificate(const RequestState& requestState);
 
   void
   registerPrefix();
diff --git a/src/ca-storage-detail/ca-sqlite.cpp b/src/ca-storage-detail/ca-sqlite.cpp
index a0f24ec..d392bdd 100644
--- a/src/ca-storage-detail/ca-sqlite.cpp
+++ b/src/ca-storage-detail/ca-sqlite.cpp
@@ -37,7 +37,7 @@
 
 static const std::string INITIALIZATION = R"_DBTEXT_(
 CREATE TABLE IF NOT EXISTS
-  CertRequests(
+  RequestStates(
     id INTEGER PRIMARY KEY,
     request_id TEXT NOT NULL,
     ca_name BLOB NOT NULL,
@@ -54,9 +54,9 @@
     encryption_key BLOB NOT NULL
   );
 CREATE UNIQUE INDEX IF NOT EXISTS
-  CertRequestIdIndex ON CertRequests(request_id);
+  RequestStateIdIndex ON RequestStates(request_id);
 CREATE UNIQUE INDEX IF NOT EXISTS
-  CertRequestKeyNameIndex ON CertRequests(cert_key_name);
+  RequestStateKeyNameIndex ON RequestStates(cert_key_name);
 
 CREATE TABLE IF NOT EXISTS
   IssuedCerts(
@@ -66,7 +66,7 @@
     cert BLOB NOT NULL
   );
 CREATE UNIQUE INDEX IF NOT EXISTS
-  IssuedCertRequestIdIndex ON IssuedCerts(cert_id);
+  IssuedCertIdIndex ON IssuedCerts(cert_id);
 CREATE UNIQUE INDEX IF NOT EXISTS
   IssuedCertKeyNameIndex ON IssuedCerts(cert_key_name);
 )_DBTEXT_";
@@ -122,7 +122,7 @@
                              challenge_status, cert_request,
                              challenge_type, challenge_secrets,
                              challenge_tp, remaining_tries, remaining_time, request_type, encryption_key
-                             FROM CertRequests where request_id = ?)_SQLTEXT_");
+                             FROM RequestStates where request_id = ?)_SQLTEXT_");
   statement.bind(1, requestId, SQLITE_TRANSIENT);
 
   if (statement.step() == SQLITE_ROW) {
@@ -155,10 +155,11 @@
 void
 CaSqlite::addRequest(const RequestState& request)
 {
+
   // check whether request is there already
   auto keyNameTlv = request.m_cert.getKeyName().wireEncode();
   Sqlite3Statement statement1(m_database,
-                              R"_SQLTEXT_(SELECT * FROM CertRequests where cert_key_name = ?)_SQLTEXT_");
+          R"_SQLTEXT_(SELECT 1 FROM RequestStates where cert_key_name = ?)_SQLTEXT_");
   statement1.bind(1, keyNameTlv, SQLITE_TRANSIENT);
   if (statement1.step() == SQLITE_ROW) {
     BOOST_THROW_EXCEPTION(Error("Request for " + request.m_cert.getKeyName().toUri() + " already exists"));
@@ -166,7 +167,7 @@
 
   // check whether certificate is already issued
   Sqlite3Statement statement2(m_database,
-                              R"_SQLTEXT_(SELECT * FROM IssuedCerts where cert_key_name = ?)_SQLTEXT_");
+                              R"_SQLTEXT_(SELECT 1 FROM IssuedCerts where cert_key_name = ?)_SQLTEXT_");
   statement2.bind(1, keyNameTlv, SQLITE_TRANSIENT);
   if (statement2.step() == SQLITE_ROW) {
     BOOST_THROW_EXCEPTION(Error("Cert for " + request.m_cert.getKeyName().toUri() + " already exists"));
@@ -174,7 +175,7 @@
 
   Sqlite3Statement statement(
       m_database,
-      R"_SQLTEXT_(INSERT INTO CertRequests (request_id, ca_name, status, request_type,
+      R"_SQLTEXT_(INSERT OR ABORT INTO RequestStates (request_id, ca_name, status, request_type,
                   cert_key_name, cert_request, challenge_type, challenge_status, challenge_secrets,
                   challenge_tp, remaining_tries, remaining_time, encryption_key)
                   values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?))_SQLTEXT_");
@@ -203,7 +204,7 @@
 CaSqlite::updateRequest(const RequestState& request)
 {
   Sqlite3Statement statement(m_database,
-                             R"_SQLTEXT_(UPDATE CertRequests
+                             R"_SQLTEXT_(UPDATE RequestStates
                              SET status = ?, challenge_type = ?, challenge_status = ?, challenge_secrets = ?,
                              challenge_tp = ?, remaining_tries = ?, remaining_time = ?
                              WHERE request_id = ?)_SQLTEXT_");
@@ -238,7 +239,7 @@
   Sqlite3Statement statement(m_database, R"_SQLTEXT_(SELECT id, request_id, ca_name, status,
                              challenge_status, cert_key_name, cert_request, challenge_type, challenge_secrets,
                              challenge_tp, remaining_tries, remaining_time, request_type, encryption_key
-                             FROM CertRequests)_SQLTEXT_");
+                             FROM RequestStates)_SQLTEXT_");
   while (statement.step() == SQLITE_ROW) {
     auto requestId = statement.getString(1);
     Name caName(statement.getBlock(2));
@@ -273,7 +274,7 @@
                              R"_SQLTEXT_(SELECT id, request_id, ca_name, status,
                              challenge_status, cert_key_name, cert_request, challenge_type, challenge_secrets,
                              challenge_tp, remaining_tries, remaining_time, request_type, encryption_key
-                             FROM CertRequests WHERE ca_name = ?)_SQLTEXT_");
+                             FROM RequestStates WHERE ca_name = ?)_SQLTEXT_");
   statement.bind(1, caName.wireEncode(), SQLITE_TRANSIENT);
 
   while (statement.step() == SQLITE_ROW) {
@@ -306,7 +307,7 @@
 CaSqlite::deleteRequest(const std::string& requestId)
 {
   Sqlite3Statement statement(m_database,
-                             R"_SQLTEXT_(DELETE FROM CertRequests WHERE request_id = ?)_SQLTEXT_");
+                             R"_SQLTEXT_(DELETE FROM RequestStates WHERE request_id = ?)_SQLTEXT_");
   statement.bind(1, requestId, SQLITE_TRANSIENT);
   statement.step();
 }
diff --git a/tests/unit-tests/ca-sqlite.t.cpp b/tests/unit-tests/ca-sqlite.t.cpp
index 4bfb740..8e0be7e 100644
--- a/tests/unit-tests/ca-sqlite.t.cpp
+++ b/tests/unit-tests/ca-sqlite.t.cpp
@@ -121,6 +121,21 @@
   BOOST_CHECK_EQUAL(allRequests.size(), 0);
 }
 
+BOOST_AUTO_TEST_CASE(DuplicateAdd)
+{
+    CaSqlite storage(dbDir.string());
+
+    auto identity1 = addIdentity(Name("/ndn/site1"));
+    auto key1 = identity1.getDefaultKey();
+    auto cert1 = key1.getDefaultCertificate();
+
+    // add operation
+    RequestState request1(Name("/ndn/site1"), "123", RequestType::NEW, Status::BEFORE_CHALLENGE, cert1, makeEmptyBlock(tlv::ContentType_Key));
+    BOOST_CHECK_NO_THROW(storage.addRequest(request1));
+    // add again
+    BOOST_CHECK_THROW(storage.addRequest(request1), std::exception);
+}
+
 BOOST_AUTO_TEST_SUITE_END() // TestCaModule
 
 } // namespace tests