Created a shared unit test for both objectstore backends.

Implemented the new interface on the Rados backend.
Fixed several bugs.

objectstore/Backend.hpp

@@ -97,6 +97,12 @@ public:
    * @return pointer to the newly created representation.
    */
   virtual Parameters * getParams() = 0;
+  
+  /**
+   * Return the name of the class. Mostly usefull in tests
+   * @return name of the class
+   */
+  virtual std::string typeName() = 0;
 };
 
 }} // end of cta::objectstore

objectstore/BackendAbstractTests.cpp

+#include "BackendAbstractTests.hpp"
+#include "BackendVFS.hpp"
+#include "BackendRados.hpp"
+
+TEST_P(BackendAbstractTest, BasicReadWrite) {
+  std::cout << "Type=" << m_os->typeName() << std::endl;
+  const std::string testValue = "1234";
+  const std::string testSecondValue = "1234";
+  const std::string testObjectName = "testObject";
+  // Check we can verify the absence of an object
+  ASSERT_EQ(false, m_os->exists(testObjectName));
+  // Check that an update attempt fails on a non-existing object
+  ASSERT_THROW(m_os->atomicOverwrite(testObjectName, testSecondValue), cta::exception::Exception);
+  // Check the creation of the obecjt
+  m_os->create(testObjectName, testValue);
+  // Check that re-creating an existing object throws exception
+  ASSERT_THROW(m_os->create(testObjectName, testValue), cta::exception::Exception);
+  // Check we can validate the presence of the object
+  ASSERT_EQ(true, m_os->exists(testObjectName));
+  // Check that we can read back after creation
+  ASSERT_EQ(testValue, m_os->read(testObjectName));
+  m_os->atomicOverwrite(testObjectName, testSecondValue);
+  // Check that an update goes through
+  ASSERT_EQ(testSecondValue, m_os->read(testObjectName));
+  // Check that we read back the value
+  ASSERT_EQ(testSecondValue, m_os->read(testObjectName));
+  // Check we can delete the object
+  ASSERT_NO_THROW(m_os->remove(testObjectName));
+  // Check that the object is actually gone
+  ASSERT_EQ(false, m_os->exists(testObjectName));
+}
+
+cta::objectstore::BackendVFS osVFS;
+cta::objectstore::BackendRados osRados("tapetest", "tapetest");
+INSTANTIATE_TEST_CASE_P(BackendTest, BackendAbstractTest, ::testing::Values(&osVFS, &osRados));
\ No newline at end of file

objectstore/BackendAbstractTests.hpp

+#pragma once
+
+#include <gtest/gtest.h>
+#include "Backend.hpp"
+
+class BackendAbstractTest: public ::testing::TestWithParam<cta::objectstore::Backend *> {
+protected:
+  BackendAbstractTest() {}
+  virtual void SetUp() {
+    m_os = GetParam();
+  }
+  cta::objectstore::Backend * m_os;
+};

objectstore/BackendRados.hpp

 #pragma once
 
 #include "Backend.hpp"
+#include "exception/Errnum.hpp"
+#include <rados/librados.hpp>
+#include <sys/syscall.h>
+#include <errno.h>
 
 namespace cta { namespace objectstore {
-
-class ObjectStoreRados: public Backend {
+/**
+ * An implementation of the object store primitives, using Rados.
+ */
+class BackendRados: public Backend {
 public:
-  ObjectStoreRados(std::string path, std::string userId, std::string pool): 
+  /**
+   * The constructor, connecting to the storage pool 'pool' using the user id
+   * 'userId'
+   * @param userId
+   * @param pool
+   */
+  BackendRados(std::string userId, std::string pool): 
     m_user(userId), m_pool(pool), m_cluster(), m_radosCtx() {
     cta::exception::Errnum::throwOnNonZero(m_cluster.init(userId.c_str()),
       "In ObjectStoreRados::ObjectStoreRados, failed to m_cluster.init");
@@ -24,7 +36,7 @@ public:
       throw;
     }
   }
-  virtual ~ObjectStoreRados() {
+  virtual ~BackendRados() {
     m_radosCtx.close();
     m_cluster.shutdown();
   }
@@ -37,14 +49,25 @@ public:
   
 
   virtual void create(std::string name, std::string content) {
-    atomicOverwrite(name, content);
+    librados::ObjectWriteOperation wop;
+    const bool createExclusive = true;
+    wop.create(createExclusive);
+    ceph::bufferlist bl;
+    bl.append(content.c_str(), content.size());
+    wop.write_full(bl);
+    cta::exception::Errnum::throwOnNonZero(m_radosCtx.operate(name, &wop),
+      std::string("In ObjectStoreRados::create, failed to create exclusively or write ")
+      + name);
   }
   
   virtual void atomicOverwrite(std::string name, std::string content) {
+    librados::ObjectWriteOperation wop;
+    wop.assert_exists();
     ceph::bufferlist bl;
     bl.append(content.c_str(), content.size());
-    cta::exception::Errnum::throwOnNonZero(m_radosCtx.write_full(name, bl),
-      std::string("In ObjectStoreRados::atomicOverwrite, failed to write_full ")
+    wop.write_full(bl);
+    cta::exception::Errnum::throwOnNonZero(m_radosCtx.operate(name, &wop),
+      std::string("In ObjectStoreRados::atomicOverwrite, failed to assert existence or write ")
       + name);
   }
 
@@ -67,7 +90,43 @@ public:
     cta::exception::Errnum::throwOnNegative(m_radosCtx.remove(name));
   }
   
-  virtual void lockExclusive(std::string name, ContextHandle & context, std::string where) {
+  virtual bool exists(std::string name) {
+    uint64_t size;
+    time_t date;
+    if (m_radosCtx.stat(name, &size, &date)) {
+      return false;
+    } else {
+      return true;
+    }
+  }
+  
+  class ScopedLock: public Backend::ScopedLock {
+    friend class BackendRados;
+  public:
+    virtual void release() {
+      if (!m_lockSet) return;
+      cta::exception::Errnum::throwOnReturnedErrno(
+        -m_context.unlock(m_oid, "lock", m_clientId),
+        std::string("In cta::objectstore::ScopedLock::release, failed unlock ")+
+      m_oid);
+      m_lockSet = false;
+    }
+    virtual ~ScopedLock() { release(); }
+  private:
+    ScopedLock(librados::IoCtx & ioCtx): m_lockSet(false), m_context(ioCtx) {}
+    void set(const std::string & oid, const std::string clientId) {
+      m_oid = oid;
+      m_clientId = clientId;\
+      m_lockSet = true;
+    }
+    bool m_lockSet;
+    librados::IoCtx & m_context;
+    std::string m_clientId;
+    std::string m_oid;
+  };
+  
+private:
+  std::string createUniqueClientId() {
     // Build a unique client name: host:thread
     char buff[200];
     cta::exception::Errnum::throwOnMinusOne(gethostname(buff, sizeof(buff)),
@@ -75,60 +134,73 @@ public:
     pid_t tid = syscall(SYS_gettid);
     std::stringstream client;
     client << buff << ":" << tid;
+    return client.str();
+  }
+  
+public:  
+  virtual ScopedLock * lockExclusive(std::string name) {
+    std::string client = createUniqueClientId();
     struct timeval tv;
     tv.tv_usec = 0;
     tv.tv_sec = 10;
     int rc;
+    std::auto_ptr<ScopedLock> ret(new ScopedLock(m_radosCtx));
     do {
-      rc=m_radosCtx.lock_exclusive(name, "lock", client.str(), "", &tv, 0);
+      rc=m_radosCtx.lock_exclusive(name, "lock", client, "", &tv, 0);
     } while (-EBUSY==rc);
     cta::exception::Errnum::throwOnReturnedErrno(-rc,
       std::string("In ObjectStoreRados::lockExclusive, failed to librados::IoCtx::lock_exclusive: ")+
-      name + "/" + "lock" + "/" + client.str() + "//");
-    context.set();
-    //std::cout << "LockedExclusive: " << name << "/" << "lock" << "/" << client.str() << "//@" << where << std::endl;
+      name + "/" + "lock" + "/" + client + "//");
+    ret->set(name, client);
+    return ret.release();
   }
 
 
-  virtual void lockShared(std::string name, ContextHandle & context, std::string where) {
-    // Build a unique client name: host:thread
-    char buff[200];
-    cta::exception::Errnum::throwOnMinusOne(gethostname(buff, sizeof(buff)),
-      "In ObjectStoreRados::lockExclusive:  failed to gethostname");
-    pid_t tid = syscall(SYS_gettid);
-    std::stringstream client;
-    client << buff << ":" << tid;
+  virtual ScopedLock * lockShared(std::string name) {
+    std::string client = createUniqueClientId();
     struct timeval tv;
     tv.tv_usec = 0;
     tv.tv_sec = 10;
     int rc;
+    std::auto_ptr<ScopedLock> ret(new ScopedLock(m_radosCtx));
     do {
-      rc=m_radosCtx.lock_shared(name, "lock", client.str(), "", "", &tv, 0);
+      rc=m_radosCtx.lock_shared(name, "lock", client, "", "", &tv, 0);
     } while (-EBUSY==rc);
     cta::exception::Errnum::throwOnReturnedErrno(-rc,
       std::string("In ObjectStoreRados::lockShared, failed to librados::IoCtx::lock_shared: ")+
-      name + "/" + "lock" + "/" + client.str() + "//");
-    context.set();
-    //std::cout << "LockedShared: " << name << "/" << "lock" << "/" << client.str() << "//@" << where << std::endl;
+      name + "/" + "lock" + "/" + client + "//");
+    ret->set(name, client);
+    return ret.release();
   }
-
-  virtual void unlock(std::string name, ContextHandle & context, std::string where) {
-    // Build a unique client name: host:thread
-    char buff[200];
-    cta::exception::Errnum::throwOnMinusOne(gethostname(buff, sizeof(buff)),
-      "In ObjectStoreRados::lockExclusive:  failed to gethostname");
-    pid_t tid = syscall(SYS_gettid);
-    std::stringstream client;
-    client << buff << ":" << tid;
-    cta::exception::Errnum::throwOnReturnedErrno(
-      -m_radosCtx.unlock(name, "lock", client.str()),
-      std::string("In ObjectStoreRados::lockExclusive,  failed to lock_exclusive ")+
-      name);
-    context.reset();
-    //std::cout << "Unlocked: " << name << "/" << "lock" << "/" << client.str() << "//@" << where << std::endl;
+  
+  class Parameters: public Backend::Parameters {
+    friend class BackendRados;
+  public:
+    /**
+     * The standard-issue params to string for logging
+     * @return a string representation of the parameters for logging
+     */
+    virtual std::string toStr() {
+      std::stringstream ret;
+      ret << "userId=" << m_userId << " pool=" << m_pool;
+      return ret.str();
+    }
+  private:
+    std::string m_userId;
+    std::string m_pool;
+  };
+  
+  virtual Parameters * getParams() {
+    std::auto_ptr<Parameters> ret (new Parameters);
+    ret->m_pool = m_pool;
+    ret->m_userId = m_user;
+    return ret.release();
   }
 
-
+  virtual std::string typeName() {
+    return "cta::objectstore::BackendRados";
+  }
+  
 private:
   std::string m_user;
   std::string m_pool;

objectstore/BackendVFS.cpp

@@ -61,25 +61,29 @@ BackendVFS::~BackendVFS() {
 void BackendVFS::create(std::string name, std::string content) {
   std::string path = m_root + "/" + name;
   std::string lockPath = m_root + "/." + name + ".lock";
+  bool fileCreated = false;
+  bool lockCreated = false;
   try {
-    int fd = ::creat(path.c_str(), S_IRWXU);
+    int fd = ::open(path.c_str(), O_WRONLY | O_CREAT | O_EXCL, S_IRWXU);
     // Create and fill up the path
     cta::exception::Errnum::throwOnMinusOne(fd,
-        "In ObjectStoreVFS::create, failed to creat the file");
+        "In ObjectStoreVFS::create, failed to open the file");
+    fileCreated = true;
     cta::exception::Errnum::throwOnMinusOne(
         ::write(fd, content.c_str(), content.size()),
         "In ObjectStoreVFS::create, failed to write to file");
     cta::exception::Errnum::throwOnMinusOne(::close(fd),
         "In ObjectStoreVFS::create, failed to close the file");
     // Create the lock file
-    int fdLock = ::creat(lockPath.c_str(), S_IRWXU);
+    int fdLock = ::open(lockPath.c_str(), O_WRONLY | O_CREAT | O_EXCL, S_IRWXU);
+    lockCreated = true;
     cta::exception::Errnum::throwOnMinusOne(fdLock,
         "In ObjectStoreVFS::create, failed to creat the lock file");
     cta::exception::Errnum::throwOnMinusOne(::close(fdLock),
         "In ObjectStoreVFS::create, failed to close the lock file");
   } catch (...) {
-    unlink(path.c_str());
-    unlink(lockPath.c_str());
+    if (fileCreated) unlink(path.c_str());
+    if (lockCreated) unlink(lockPath.c_str());
     throw;
   }
 }

objectstore/BackendVFS.hpp

@@ -87,6 +87,12 @@ public:
   };
   
   virtual Parameters * getParams();
+  
+
+  virtual std::string typeName() {
+    return "cta::objectstore::BackendVFS";
+  }
+
 
 private:
   std::string m_root;

objectstore/CMakeLists.txt

@@ -49,6 +49,11 @@ add_library (CTAObjectStore
 # target_link_libraries(createEnvironment
 #   protobuf rados CTAObjectStore)
 
-add_executable(unitTests unitTests.cpp BackendVFSTest.cpp)
+set(ObjectStoreUnitTests
+  BackendVFSTest.cpp
+  BackendAbstractTests.cpp)
+#  RootEntryTest.cpp)
+
+add_executable(unitTests unitTests.cpp ${ObjectStoreUnitTests})
 target_link_libraries(unitTests
   protobuf rados CTAObjectStore gtest gmock)
\ No newline at end of file

objectstore/ObjectOps.hpp

 #pragma once
 
-#include "BackendStore.hpp.hpp"
+#include "Backend.hpp"
 
 namespace cta { namespace objectstore {
 

objectstore/ObjectStoreChoice.hpp

@@ -3,7 +3,7 @@
 #include "ObjectOps.hpp"
 #define USE_RADOS 1
 #if USE_RADOS
-typedef cta::objectstore::ObjectStoreRados myOS;
+typedef cta::objectstore::BackendRados myOS;
 #else
 typedef cta::objectstore::BackendVFS myOS;
 #endif
\ No newline at end of file

objectstore/RootEntry.hpp

@@ -2,7 +2,7 @@
 
 #include "objectstore/cta.pb.h"
 
-#include "BackendStore.hpp.hpp"
+#include "Backend.hpp"
 #include "ObjectOps.hpp"
 #include "Agent.hpp"
 

objectstore/RootEntryTest.cpp

@@ -3,6 +3,6 @@
 #include "exception/Exception.hpp"
 #include "RootEntry.hpp"
 
-TEST(RootEntry.BasicAccess) {
+TEST(RootEntry, BasicAccess) {
   
-}
+};
\ No newline at end of file