Skip to content
Permalink
Browse files

Make info(), pathCreate() and pathSync() optional for storage drivers.

These functions are not required for repository storage so make them optional and error if they are not implemented for non-repository storage, .e.g. pg or spool.

The goal is to simplify the drivers (e.g. S3) that are intended only for repository storage.
  • Loading branch information...
dwsteele committed May 24, 2019
1 parent 39645fc commit d12d94c53c466eb808459b98669a877c19b5716b
Showing with 15 additions and 88 deletions.
  1. +2 −74 src/storage/s3/storage.c
  2. +13 −9 src/storage/storage.c
  3. +0 −5 test/src/module/storage/s3Test.c
@@ -389,30 +389,6 @@ storageS3Exists(THIS_VOID, const String *path)
FUNCTION_LOG_RETURN(BOOL, result);
}

/***********************************************************************************************************************************
File/path info
***********************************************************************************************************************************/
static StorageInfo
storageS3Info(THIS_VOID, const String *file, bool ignoreMissing, bool followLink)
{
THIS(StorageS3);

FUNCTION_LOG_BEGIN(logLevelDebug);
FUNCTION_LOG_PARAM(STORAGE_S3, this);
FUNCTION_LOG_PARAM(STRING, file);
FUNCTION_LOG_PARAM(BOOL, ignoreMissing);
FUNCTION_LOG_PARAM(BOOL, followLink);
FUNCTION_LOG_END();

ASSERT(this != NULL);
ASSERT(file != NULL);
ASSERT(followLink == false);

THROW(AssertError, "NOT YET IMPLEMENTED");

FUNCTION_LOG_RETURN(STORAGE_INFO, (StorageInfo){0});
}

/***********************************************************************************************************************************
Get a list of files from a directory
***********************************************************************************************************************************/
@@ -596,31 +572,6 @@ storageS3NewWrite(
FUNCTION_LOG_RETURN(STORAGE_WRITE, storageWriteS3New(this, file, this->partSize));
}

/***********************************************************************************************************************************
Create a path
There are no physical paths on S3 so just return success.
***********************************************************************************************************************************/
static void
storageS3PathCreate(THIS_VOID, const String *path, bool errorOnExists, bool noParentCreate, mode_t mode)
{
THIS(StorageS3);

FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM(STORAGE_S3, this);
FUNCTION_LOG_PARAM(STRING, path);
FUNCTION_LOG_PARAM(BOOL, errorOnExists);
FUNCTION_LOG_PARAM(BOOL, noParentCreate);
FUNCTION_LOG_PARAM(MODE, mode);
FUNCTION_LOG_END();

ASSERT(this != NULL);
ASSERT(path != NULL);
ASSERT(mode == 0);

FUNCTION_LOG_RETURN_VOID();
}

/***********************************************************************************************************************************
Remove a path
***********************************************************************************************************************************/
@@ -740,28 +691,6 @@ storageS3PathRemove(THIS_VOID, const String *path, bool errorOnMissing, bool rec
FUNCTION_LOG_RETURN_VOID();
}

/***********************************************************************************************************************************
Sync a path
There's no need for this on S3 so just return success.
***********************************************************************************************************************************/
static void
storageS3PathSync(THIS_VOID, const String *path, bool ignoreMissing)
{
THIS(StorageS3);

FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM(STORAGE_S3, this);
FUNCTION_LOG_PARAM(STRING, path);
FUNCTION_LOG_PARAM(BOOL, ignoreMissing);
FUNCTION_LOG_END();

ASSERT(this != NULL);
ASSERT(path != NULL);

FUNCTION_LOG_RETURN_VOID();
}

/***********************************************************************************************************************************
Remove a file
***********************************************************************************************************************************/
@@ -861,9 +790,8 @@ storageS3New(

this = storageNewP(
STORAGE_S3_TYPE_STR, path, 0, 0, write, pathExpressionFunction, driver,
.exists = storageS3Exists, .info = storageS3Info, .list = storageS3List, .newRead = storageS3NewRead,
.newWrite = storageS3NewWrite, .pathCreate = storageS3PathCreate, .pathRemove = storageS3PathRemove,
.pathSync = storageS3PathSync, .remove = storageS3Remove);
.exists = storageS3Exists, .list = storageS3List, .newRead = storageS3NewRead, .newWrite = storageS3NewWrite,
.pathRemove = storageS3PathRemove, .remove = storageS3Remove);
}
MEM_CONTEXT_NEW_END();

@@ -57,13 +57,10 @@ storageNew(
ASSERT(path == NULL || (strSize(path) >= 1 && strPtr(path)[0] == '/'));
ASSERT(driver != NULL);
ASSERT(interface.exists != NULL);
ASSERT(interface.info != NULL);
ASSERT(interface.list != NULL);
ASSERT(interface.newRead != NULL);
ASSERT(interface.newWrite != NULL);
ASSERT(interface.pathCreate != NULL);
ASSERT(interface.pathRemove != NULL);
ASSERT(interface.pathSync != NULL);
ASSERT(interface.remove != NULL);

Storage *this = NULL;
@@ -243,6 +240,7 @@ storageInfo(const Storage *this, const String *fileExp, StorageInfoParam param)
FUNCTION_LOG_END();

ASSERT(this != NULL);
ASSERT(this->interface.info != NULL);

StorageInfo result = {0};

@@ -351,6 +349,7 @@ storageMove(const Storage *this, StorageRead *source, StorageWrite *destination)
FUNCTION_LOG_PARAM(STORAGE_WRITE, destination);
FUNCTION_LOG_END();

ASSERT(this != NULL);
ASSERT(this->interface.move != NULL);
ASSERT(source != NULL);
ASSERT(destination != NULL);
@@ -573,6 +572,7 @@ storagePathCreate(const Storage *this, const String *pathExp, StoragePathCreateP
FUNCTION_LOG_END();

ASSERT(this != NULL);
ASSERT(this->interface.pathCreate != NULL);
ASSERT(this->write);

// It doesn't make sense to combine these parameters because if we are creating missing parent paths why error when they exist?
@@ -661,15 +661,19 @@ void storagePathSync(const Storage *this, const String *pathExp, StoragePathSync
ASSERT(this != NULL);
ASSERT(this->write);

MEM_CONTEXT_TEMP_BEGIN()
// Not all storage requires path sync so just do nothing if the function is not implemented
if (this->interface.pathSync != NULL)
{
// Build the path
String *path = storagePathNP(this, pathExp);
MEM_CONTEXT_TEMP_BEGIN()
{
// Build the path
String *path = storagePathNP(this, pathExp);

// Call driver function
this->interface.pathSync(this->driver, path, param.ignoreMissing);
// Call driver function
this->interface.pathSync(this->driver, path, param.ignoreMissing);
}
MEM_CONTEXT_TEMP_END();
}
MEM_CONTEXT_TEMP_END();

FUNCTION_LOG_RETURN_VOID();
}
@@ -661,7 +661,6 @@ testRun(void)

// Coverage for noop functions
// -------------------------------------------------------------------------------------------------------------------------
TEST_RESULT_VOID(storagePathCreateNP(s3, strNew("path")), "path create is a noop");
TEST_RESULT_VOID(storagePathSyncNP(s3, strNew("path")), "path sync is a noop");

// storageS3NewRead() and StorageS3FileRead
@@ -776,10 +775,6 @@ testRun(void)
// storageDriverRemove()
// -------------------------------------------------------------------------------------------------------------------------
TEST_RESULT_VOID(storageRemoveNP(s3, strNew("/path/to/test.txt")), "remove file");

// Coverage for unimplemented functions
// -------------------------------------------------------------------------------------------------------------------------
TEST_ERROR(storageInfoNP(s3, strNew("file.txt")), AssertError, "NOT YET IMPLEMENTED");
}

FUNCTION_HARNESS_RESULT_VOID();

0 comments on commit d12d94c

Please sign in to comment.
You can’t perform that action at this time.