Skip to content

Commit

Permalink
Add unit tests
Browse files Browse the repository at this point in the history 
Signed-off-by: Gavin Halliday <[email protected]>
  • Loading branch information
@ghalliday
ghalliday committed Nov 17, 2023
1 parent 129af46 commit 1a27d38
Show file tree
Hide file tree
Showing 3 changed files with 185 additions and 16 deletions.
7 changes: 5 additions & 2 deletions system/jlib/jptree.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -438,11 +438,14 @@ extern jlib_decl unsigned getPropertyTreeHash(const IPropertyTree & source, unsi
//to not be modified and to remain valid and consistent until it is released.
interface ISyncedPropertyTree : extends IInterface
{
//The following functions check whether something is up to date before returning their values.
//Return a version-hash which changes whenever the property tree changes - so that a caller can determine whether it needs to update
virtual unsigned getVersion() const = 0;
virtual const IPropertyTree * getTree() const = 0;
virtual bool getProp(MemoryBuffer & result, const char * xpath) const = 0;
virtual bool getProp(StringBuffer & result, const char * xpath) const = 0;
//Return a version-hash which changes whenever the property tree changes - so that a caller can determine whether it needs to update
virtual unsigned getVersion() const = 0;

// The following functions return the current cached state - they do not force a check to see if the value is up to date
virtual bool isStale() const = 0; // An indication that the property tree may be out of date because it couldn't be resynchronized.
virtual bool isValid() const = 0; // Is the property tree non-null? Typically called at startup to check configuration is provided.
};
Expand Down
20 changes: 6 additions & 14 deletions system/jlib/jsecrets.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,11 @@ class SecretCacheEntry : public CInterface
friend class SecretCache;

public:
SecretCacheEntry(IPropertyTree * _contents, cache_timestamp _now)
: contents(_contents), contentTimestamp(_now), accessedTimestamp(_now), checkedTimestamp(_now)
//A cache entry is initally created that has a create an access time of now, but the checkTimestamp
//is set so that needsRefresh() will return true.
SecretCacheEntry(cache_timestamp _now)
: contentTimestamp(_now), accessedTimestamp(_now), checkedTimestamp(_now - 2 * secretTimeoutMs)
{
updateHash();
}

unsigned getHash() const
Expand Down Expand Up @@ -474,7 +475,7 @@ class SecretCache
else
{
//Insert an entry with a null value that is marked as out of date
result = new SecretCacheEntry(nullptr, now - 2 * secretTimeoutMs);
result = new SecretCacheEntry(now);
secrets.emplace(secretKey, result);
}
return result;
Expand Down Expand Up @@ -1155,7 +1156,6 @@ class CSecret final : public CInterfaceOf<ISyncedPropertyTree>
CSecret(const char *_category, const char * _name, const char * _vaultId, const char * _version, SecretCacheEntry * _secret)
: category(_category), name(_name), vaultId(_vaultId), version(_version), secret(_secret)
{
updateHash();
}

virtual const IPropertyTree * getTree() const override;
Expand All @@ -1182,7 +1182,7 @@ class CSecret final : public CInterfaceOf<ISyncedPropertyTree>
{
CriticalBlock block(secretCs);
checkUptoDate();
return secretHash;
return secret->getHash();
}
virtual bool isValid() const override
{
Expand All @@ -1191,7 +1191,6 @@ class CSecret final : public CInterfaceOf<ISyncedPropertyTree>

protected:
void checkUptoDate() const;
void updateHash() const;

protected:
StringAttr category;
Expand All @@ -1200,7 +1199,6 @@ class CSecret final : public CInterfaceOf<ISyncedPropertyTree>
StringAttr version;
mutable CriticalSection secretCs;
mutable SecretCacheEntry * secret;
mutable unsigned secretHash = 0;
};


Expand All @@ -1225,7 +1223,6 @@ void CSecret::checkUptoDate() const
//Check the secret is always returned consistently. It would be possible to call a slightly
//more optimal function to refresh a secret, but this is simplest.
assertex(secret == newSecret);
updateHash();
}
catch (IException * e)
{
Expand All @@ -1236,11 +1233,6 @@ void CSecret::checkUptoDate() const
}
}

void CSecret::updateHash() const
{
secretHash = secret->getHash();
}

ISyncedPropertyTree * resolveSecret(const char *category, const char * name, const char * optVaultId, const char * optVersion)
{
validateCategoryName(category);
Expand Down
174 changes: 174 additions & 0 deletions testing/unittests/jlibtests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@
#include "jlzw.hpp"
#include "jqueue.hpp"
#include "jregexp.hpp"
#include "jsecrets.hpp"
#include "jutil.hpp"
#include "junicode.hpp"

#include "unittests.hpp"

#define CPPUNIT_ASSERT_EQUAL_STR(x, y) CPPUNIT_ASSERT_EQUAL(std::string(x),std::string(y))

static const unsigned oneMinute = 60000; // msec

class JlibTraceTest : public CppUnit::TestFixture
Expand Down Expand Up @@ -3611,4 +3614,175 @@ CPPUNIT_TEST_SUITE_REGISTRATION( JLibUnicodeTest );
CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( JLibUnicodeTest, "JLibUnicodeTest" );


class JLibSecretsTest : public CppUnit::TestFixture
{
public:
CPPUNIT_TEST_SUITE(JLibSecretsTest);
CPPUNIT_TEST(setup);
CPPUNIT_TEST(testUpdate);
CPPUNIT_TEST_SUITE_END();

//Each test creates a different instance of the class(!) so member values cannot be used to pass items
//from one test to another
StringBuffer secretRoot;

protected:
void checkSecret(const char * secret, const char * key, const char * expectedValue)
{
Owned<const IPropertyTree> match = getSecret("testing", secret);
CPPUNIT_ASSERT(match);
const char * secretValue = match->queryProp(key);
CPPUNIT_ASSERT(secretValue);
CPPUNIT_ASSERT_EQUAL_STR(secretValue, expectedValue);
}

bool hasSecret(const char * name)
{
Owned<const IPropertyTree> match = getSecret("testing", name);
return match != nullptr;
}

void initPath()
{
char cwd[1024];
CPPUNIT_ASSERT(GetCurrentDirectory(1024, cwd));
secretRoot.set(cwd).append(PATHSEPCHAR).append("unittest-secrets");
secretRoot.append(PATHSEPCHAR).append("testing"); // catgegory
}

void setup()
{
char cwd[1024];
CPPUNIT_ASSERT(GetCurrentDirectory(1024, cwd));
secretRoot.append(cwd).append(PATHSEPCHAR).append("unittest-secrets");

recursiveRemoveDirectory(secretRoot);
CPPUNIT_ASSERT(recursiveCreateDirectory(secretRoot.str()));
setSecretMount(secretRoot);
setSecretTimeout(100); // Set the timeout so we can check it is working.

secretRoot.append(PATHSEPCHAR).append("testing"); // catgegory
CPPUNIT_ASSERT(recursiveCreateDirectory(secretRoot.str()));
}

void testUpdate()
{
initPath(); // secretRoot needs to be called for each test

CPPUNIT_ASSERT(!hasSecret("secret1"));
writeTestingSecret("secret1", "value", "secret1Value");
//Secret should not appear yet - null should be cached.
CPPUNIT_ASSERT(!hasSecret("secret1"));

Owned<ISyncedPropertyTree> secret2 = resolveSecret("testing", "secret2", nullptr, nullptr);
CPPUNIT_ASSERT(!secret2->isValid());
CPPUNIT_ASSERT(!secret2->isStale());

MilliSleep(50);
//Secret should not appear yet - null should be cached.
CPPUNIT_ASSERT(!hasSecret("secret1"));
CPPUNIT_ASSERT(!secret2->isValid());
CPPUNIT_ASSERT(!secret2->isStale());

MilliSleep(100);
//Secret should not be updated - enough time has passed
checkSecret("secret1", "value", "secret1Value");
CPPUNIT_ASSERT(!secret2->isValid());
CPPUNIT_ASSERT(secret2->isStale());

Owned<ISyncedPropertyTree> secret3 = resolveSecret("testing", "secret3", nullptr, nullptr);
unsigned version = secret3->getVersion();
CPPUNIT_ASSERT(!secret3->isValid());
CPPUNIT_ASSERT(!secret3->isStale());
writeTestingSecret("secret3", "value", "secret3Value");
CPPUNIT_ASSERT(!secret3->isValid());
CPPUNIT_ASSERT_EQUAL(version, secret3->getVersion());

//After sleep new value should not have been picked up
MilliSleep(50);
CPPUNIT_ASSERT(!secret3->isValid());
CPPUNIT_ASSERT_EQUAL(version, secret3->getVersion());

//After sleep new value should now have been picked up
MilliSleep(100);
checkSecret("secret3", "value", "secret3Value");
unsigned version2 = secret3->getVersion();
CPPUNIT_ASSERT(!secret3->isStale());
CPPUNIT_ASSERT(secret3->isValid());
CPPUNIT_ASSERT(version != version2);

//Sleep and check that the hash value has not changed
MilliSleep(200);
checkSecret("secret3", "value", "secret3Value");
CPPUNIT_ASSERT(!secret3->isStale());
CPPUNIT_ASSERT(secret3->isValid());
CPPUNIT_ASSERT_EQUAL(version2, secret3->getVersion());

//Remove the secret - should have no immediate effect
writeTestingSecret("secret3", "value", nullptr);
CPPUNIT_ASSERT(!secret3->isStale());
CPPUNIT_ASSERT(secret3->isValid());
CPPUNIT_ASSERT_EQUAL(version2, secret3->getVersion());

MilliSleep(50);
CPPUNIT_ASSERT(!secret3->isStale());
CPPUNIT_ASSERT(secret3->isValid());
CPPUNIT_ASSERT_EQUAL(version2, secret3->getVersion());
checkSecret("secret3", "value", "secret3Value");

MilliSleep(100);
CPPUNIT_ASSERT(secret3->isStale()); // Value has gone, but the old value is still returned
CPPUNIT_ASSERT(secret3->isValid());
CPPUNIT_ASSERT_EQUAL(version2, secret3->getVersion());
checkSecret("secret3", "value", "secret3Value");

//Update the value = the change should not be seen until the cache entry expires
writeTestingSecret("secret3", "value", "secret3NewValue");
CPPUNIT_ASSERT(secret3->isStale());
CPPUNIT_ASSERT(secret3->isValid());
CPPUNIT_ASSERT_EQUAL(version2, secret3->getVersion());
checkSecret("secret3", "value", "secret3Value");

MilliSleep(50);
CPPUNIT_ASSERT(secret3->isStale());
CPPUNIT_ASSERT(secret3->isValid());
CPPUNIT_ASSERT_EQUAL(version2, secret3->getVersion());
checkSecret("secret3", "value", "secret3Value");

MilliSleep(100);
//These functions do not check for up to date values, so they return the same as before
CPPUNIT_ASSERT(secret3->isStale()); // Value still appears to be out of date
CPPUNIT_ASSERT(secret3->isValid());

//This call should force the value to be updated
CPPUNIT_ASSERT(version2 != secret3->getVersion());
CPPUNIT_ASSERT(!secret3->isStale()); // New value has now been picked up
CPPUNIT_ASSERT(secret3->isValid());
checkSecret("secret3", "value", "secret3NewValue");
}

void writeTestingSecret(const char * secret, const char * key, const char * value)
{
StringBuffer filename;
filename.append(secretRoot).append(PATHSEPCHAR).append(secret);
CPPUNIT_ASSERT(recursiveCreateDirectory(filename.str()));

filename.append(PATHSEPCHAR).append(key);

Owned<IFile> file = createIFile(filename.str());
if (value)
{
Owned<IFileIO> io = file->open(IFOcreate);
io->write(0, strlen(value), value);
}
else
file->remove();
}
};

CPPUNIT_TEST_SUITE_REGISTRATION( JLibSecretsTest );
CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( JLibSecretsTest, "JLibSecretsTest" );



#endif // _USE_CPPUNIT

0 comments on commit 1a27d38

Please sign in to comment.