Skip to content

Commit

Permalink
vrs: pass fileSpec.extras as options to DiskFile.create
Browse files Browse the repository at this point in the history
Summary:
This allows `WriteFileHandler` objects to make use of URI parameters that they do not save off at `checkChunks` time. (Like `DiskFile`).

Not that some `WriteFileHandler`s just preserve the entire URI in the `chunk.path` field, allowing them to access these later. `DiskFile` notably does NOT, which is why this is necesary.

Reviewed By: georges-berenger

Differential Revision: D57869385

fbshipit-source-id: c4fa7c4d9972545cbd6c7a62d74835b31e488cde
  • Loading branch information
Jeremy Braun authored and facebook-github-bot committed Jun 5, 2024
1 parent 9f2871f commit 222e66b
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 11 deletions.
7 changes: 4 additions & 3 deletions vrs/DiskFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,10 @@ bool DiskFile::isOpened() const {
return currentChunk_ != nullptr;
}

int DiskFile::create(const string& newFilePath) {
int DiskFile::create(const string& newFilePath, const map<string, string>& options) {
close();
readOnly_ = false;
options_ = options;
return addChunk(newFilePath);
}

Expand Down Expand Up @@ -556,9 +557,9 @@ AtomicDiskFile::~AtomicDiskFile() {
AtomicDiskFile::close(); // overrides not available in constructors & destructors
}

int AtomicDiskFile::create(const std::string& newFilePath) {
int AtomicDiskFile::create(const std::string& newFilePath, const map<string, string>& options) {
finalName_ = newFilePath;
return DiskFile::create(os::getUniquePath(finalName_, 10));
return DiskFile::create(os::getUniquePath(finalName_, 10), options);
}

int AtomicDiskFile::close() {
Expand Down
6 changes: 3 additions & 3 deletions vrs/DiskFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class DiskFile : public WriteFileHandler {
bool isOpened() const override;

/// Create a new file
int create(const string& newFilePath) override;
int create(const string& newFilePath, const map<string, string>& options = {}) override;
/// Call this method to forget any chunk beyond this file size.
void forgetFurtherChunks(int64_t fileSize) override;
/// Get the total size of all the chunks considered.
Expand Down Expand Up @@ -158,7 +158,7 @@ class DiskFile : public WriteFileHandler {
}
bool trySetPosInCurrentChunk(int64_t offset);

map<string, string> options_; // optional options provided in openSpec
map<string, string> options_; // optional options provided in openSpec or createFile
vector<DiskFileChunk> chunks_; // all the chunks, when a VRS file is opened.
DiskFileChunk* currentChunk_{}; // always points to the current chunk within chunks_.
int filesOpenCount_{};
Expand Down Expand Up @@ -187,7 +187,7 @@ class AtomicDiskFile : public DiskFile {
public:
~AtomicDiskFile() override;

int create(const string& newFilePath) override;
int create(const string& newFilePath, const map<string, string>& options = {}) override;
int close() override;

void abort();
Expand Down
9 changes: 5 additions & 4 deletions vrs/WriteFileHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,14 @@ class WriteFileHandler : public FileHandler {
/// The path of the file to create is expected to be in the first chunk.
/// Optional URI parameters might be provided in the spec' extras.
virtual int create(const FileSpec& spec) {
return spec.chunks.empty() ? INVALID_FILE_SPEC : create(spec.chunks.front());
return spec.chunks.empty() ? INVALID_FILE_SPEC : create(spec.chunks.front(), spec.extras);
}

/// Create a new file for writing.
/// @param newFilePath: a disk path to create the file.
/// @param options: optional parameters to pass when creating the file.
/// @return A status code, 0 meaning success.
virtual int create(const string& newFilePath) = 0;
virtual int create(const string& newFilePath, const map<string, string>& options = {}) = 0;

/// Create a new file for writing, in split-head file mode, the body part.
/// @param spec: spec as converted already from initialFilePath, if that helps.
Expand All @@ -63,9 +64,9 @@ class WriteFileHandler : public FileHandler {
virtual int createSplitFile(const FileSpec& spec, const string& initialFilePath) {
// create the (first) user record chunk
if (spec.chunks.size() == 1) {
return create(spec.chunks.front() + "_1");
return create(spec.chunks.front() + "_1", spec.extras);
} else {
return create(initialFilePath);
return create(initialFilePath, spec.extras);
}
}

Expand Down
2 changes: 1 addition & 1 deletion vrs/test/file_tests/UserRecordsFileHandlerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class UserRecordsFileHandler : public WriteFileHandler {

/// Minimal implementations needed for a custom WriteFileHandler used for data writes only
/// It can only write forward, no seek operations, no read back
int create(const string& newFilePath) override {
int create(const string& newFilePath, const map<string, string>& /* options = {} */) override {
if (file_ != nullptr) {
return FILE_ALREADY_OPEN;
}
Expand Down

0 comments on commit 222e66b

Please sign in to comment.