Skip to content

Commit

Permalink
Rename to BoundedReader, rename to m_curr_pos, only update curr_pos o…
Browse files Browse the repository at this point in the history
…n error if EOF
  • Loading branch information
gibber9809 committed Dec 7, 2024
1 parent 2ab88fd commit 8977dc4
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 82 deletions.
6 changes: 3 additions & 3 deletions components/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,8 @@ set(SOURCE_FILES_unitTest
src/clp/aws/AwsAuthenticationSigner.cpp
src/clp/aws/AwsAuthenticationSigner.hpp
src/clp/aws/constants.hpp
src/clp/BoundedReader.cpp
src/clp/BoundedReader.hpp
src/clp/BufferedFileReader.cpp
src/clp/BufferedFileReader.hpp
src/clp/BufferReader.cpp
Expand All @@ -353,8 +355,6 @@ set(SOURCE_FILES_unitTest
src/clp/clp/run.hpp
src/clp/clp/utils.cpp
src/clp/clp/utils.hpp
src/clp/CheckpointReader.cpp
src/clp/CheckpointReader.hpp
src/clp/CurlDownloadHandler.cpp
src/clp/CurlDownloadHandler.hpp
src/clp/CurlEasyHandle.hpp
Expand Down Expand Up @@ -550,8 +550,8 @@ set(SOURCE_FILES_unitTest
submodules/sqlite3/sqlite3ext.h
tests/LogSuppressor.hpp
tests/test-Array.cpp
tests/test-BoundedReader.cpp
tests/test-BufferedFileReader.cpp
tests/test-CheckpointReader.cpp
tests/test-clp_s-end_to_end.cpp
tests/test-EncodedVariableInterpreter.cpp
tests/test-encoding_methods.cpp
Expand Down
39 changes: 39 additions & 0 deletions components/core/src/clp/BoundedReader.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#include "BoundedReader.hpp"

namespace clp {
auto BoundedReader::try_seek_from_begin(size_t pos) -> ErrorCode {
auto next_pos = pos > m_bound ? m_bound : pos;
if (auto const rc = m_reader->try_seek_from_begin(next_pos); ErrorCode_Success != rc) {
m_curr_pos = ErrorCode_EndOfFile == rc ? next_pos : m_curr_pos;
return rc;
}
m_curr_pos = next_pos;
if (m_curr_pos >= m_bound) {
return ErrorCode_EndOfFile;
}
return ErrorCode_Success;
}

auto BoundedReader::try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read)
-> ErrorCode {
if (m_curr_pos == m_bound) {
num_bytes_read = 0;
return ErrorCode_EndOfFile;
}

if ((m_curr_pos + num_bytes_to_read) > m_bound) {
num_bytes_to_read = m_bound - m_curr_pos;
}

auto const rc = m_reader->try_read(buf, num_bytes_to_read, num_bytes_read);
m_curr_pos += num_bytes_read;
if (ErrorCode_EndOfFile == rc) {
if (0 == num_bytes_read) {
return ErrorCode_EndOfFile;
}
} else if (ErrorCode_Success != rc) {
return rc;
}
return ErrorCode_Success;
}
} // namespace clp
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
#ifndef CLP_CHECKPOINTREADER_HPP
#define CLP_CHECKPOINTREADER_HPP
#ifndef CLP_BOUNDEDREADER_HPP
#define CLP_BOUNDEDREADER_HPP

#include "ReaderInterface.hpp"

namespace clp {
/**
* CheckpointReader is a ReaderInterface designed to wrap other ReaderInterfaces and prevent users
* BoundedReader is a ReaderInterface designed to wrap other ReaderInterfaces and prevent users
* from reading or seeking beyond a certain point in the underlying input stream.
*
* This is useful when the underlying input stream is divided into several logical segments and we
* want to prevent a reader for an earlier segment consuming any bytes from a later segment. In
* particular, reading part of a later segment may force the reader for that later segment to seek
* backwards, which can be either inefficient or impossible for certain kinds of input streams.
*/
class CheckpointReader : public ReaderInterface {
class BoundedReader : public ReaderInterface {
public:
// Constructor
explicit CheckpointReader(ReaderInterface* reader, size_t checkpoint)
explicit BoundedReader(ReaderInterface* reader, size_t bound)
: m_reader(reader),
m_checkpoint(checkpoint) {
m_bound(bound) {
if (nullptr == m_reader) {
throw ReaderInterface::OperationFailed(ErrorCode_BadParam, __FILE__, __LINE__);
}
m_cur_pos = m_reader->get_pos();
if (m_cur_pos > m_checkpoint) {
m_curr_pos = m_reader->get_pos();
if (m_curr_pos > m_bound) {
throw ReaderInterface::OperationFailed(ErrorCode_BadParam, __FILE__, __LINE__);
}
}
Expand Down Expand Up @@ -70,9 +70,9 @@ class CheckpointReader : public ReaderInterface {

private:
ReaderInterface* m_reader{nullptr};
size_t m_checkpoint{};
size_t m_cur_pos{};
size_t m_bound{};
size_t m_curr_pos{};
};
} // namespace clp

#endif // CLP_CHECKPOINTREADER_HPP
#endif // CLP_BOUNDEDREADER_HPP
37 changes: 0 additions & 37 deletions components/core/src/clp/CheckpointReader.cpp

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,98 +2,98 @@

#include <Catch2/single_include/catch2/catch.hpp>

#include "../src/clp/CheckpointReader.hpp"
#include "../src/clp/BoundedReader.hpp"
#include "../src/clp/ErrorCode.hpp"
#include "../src/clp/StringReader.hpp"

TEST_CASE("Test Checkpoint Reader", "[CheckpointReader]") {
TEST_CASE("Test Bounded Reader", "[BoundedReader]") {
constexpr char cTestString[]{"0123456789"};
constexpr size_t cTestStringLen{std::char_traits<char>::length(cTestString)};

SECTION("CheckpointReader does not support try_read_to_delimiter") {
SECTION("BoundedReader does not support try_read_to_delimiter") {
clp::StringReader string_reader;
string_reader.open(cTestString);
clp::CheckpointReader checkpoint_reader{&string_reader, cTestStringLen};
clp::BoundedReader bounded_reader{&string_reader, cTestStringLen};
std::string tmp;
REQUIRE(clp::ErrorCode_Unsupported
== checkpoint_reader.try_read_to_delimiter('0', false, false, tmp));
== bounded_reader.try_read_to_delimiter('0', false, false, tmp));
}

SECTION("CheckpointReader does not allow reads beyond end of underlying stream.") {
SECTION("BoundedReader does not allow reads beyond end of underlying stream.") {
clp::StringReader string_reader;
string_reader.open(cTestString);
clp::CheckpointReader checkpoint_reader{&string_reader, cTestStringLen + 1};
clp::BoundedReader bounded_reader{&string_reader, cTestStringLen + 1};
char buf[cTestStringLen + 1];
size_t num_bytes_read{};
auto rc = checkpoint_reader.try_read(buf, cTestStringLen + 1, num_bytes_read);
auto rc = bounded_reader.try_read(buf, cTestStringLen + 1, num_bytes_read);
REQUIRE(clp::ErrorCode_Success == rc);
REQUIRE(num_bytes_read == cTestStringLen);
REQUIRE(cTestStringLen == string_reader.get_pos());
REQUIRE(cTestStringLen == checkpoint_reader.get_pos());
REQUIRE(cTestStringLen == bounded_reader.get_pos());
}

SECTION("CheckpointReader does not allow reads beyond checkpoint.") {
SECTION("BoundedReader does not allow reads beyond checkpoint.") {
clp::StringReader string_reader;
string_reader.open(cTestString);
clp::CheckpointReader checkpoint_reader{&string_reader, 1};
clp::BoundedReader bounded_reader{&string_reader, 1};
char buf[cTestStringLen];
size_t num_bytes_read{};
auto rc = checkpoint_reader.try_read(buf, cTestStringLen, num_bytes_read);
auto rc = bounded_reader.try_read(buf, cTestStringLen, num_bytes_read);
REQUIRE(clp::ErrorCode_Success == rc);
REQUIRE(1 == num_bytes_read);
REQUIRE(1 == string_reader.get_pos());
REQUIRE(1 == checkpoint_reader.get_pos());
rc = checkpoint_reader.try_read(buf, 1, num_bytes_read);
REQUIRE(1 == bounded_reader.get_pos());
rc = bounded_reader.try_read(buf, 1, num_bytes_read);
REQUIRE(clp::ErrorCode_EndOfFile == rc);
REQUIRE(0 == num_bytes_read);
REQUIRE(1 == string_reader.get_pos());
REQUIRE(1 == checkpoint_reader.get_pos());
REQUIRE(1 == bounded_reader.get_pos());
}

SECTION("CheckpointReader does allow reads before checkpoint.") {
SECTION("BoundedReader does allow reads before checkpoint.") {
clp::StringReader string_reader;
string_reader.open(cTestString);
clp::CheckpointReader checkpoint_reader{&string_reader, 1};
clp::BoundedReader bounded_reader{&string_reader, 1};
char buf{};
size_t num_bytes_read{};
auto rc = checkpoint_reader.try_read(&buf, 1, num_bytes_read);
auto rc = bounded_reader.try_read(&buf, 1, num_bytes_read);
REQUIRE(clp::ErrorCode_Success == rc);
REQUIRE(1 == num_bytes_read);
REQUIRE(1 == string_reader.get_pos());
REQUIRE(1 == checkpoint_reader.get_pos());
REQUIRE(1 == bounded_reader.get_pos());
}

SECTION("CheckpointReader does not allow seeks beyond end of underlying stream.") {
SECTION("BoundedReader does not allow seeks beyond end of underlying stream.") {
clp::StringReader string_reader;
string_reader.open(cTestString);
clp::CheckpointReader checkpoint_reader{&string_reader, cTestStringLen + 1};
auto rc = checkpoint_reader.try_seek_from_begin(cTestStringLen + 1);
clp::BoundedReader bounded_reader{&string_reader, cTestStringLen + 1};
auto rc = bounded_reader.try_seek_from_begin(cTestStringLen + 1);
REQUIRE(clp::ErrorCode_EndOfFile == rc);
REQUIRE(cTestStringLen == string_reader.get_pos());
REQUIRE(cTestStringLen == checkpoint_reader.get_pos());
REQUIRE(cTestStringLen == bounded_reader.get_pos());
}

SECTION("CheckpointReader does not allow seeks beyond checkpoint.") {
SECTION("BoundedReader does not allow seeks beyond checkpoint.") {
clp::StringReader string_reader;
string_reader.open(cTestString);
clp::CheckpointReader checkpoint_reader{&string_reader, 1};
clp::BoundedReader bounded_reader{&string_reader, 1};
char buf[cTestStringLen];
size_t num_bytes_read{};
auto rc = checkpoint_reader.try_seek_from_begin(cTestStringLen);
auto rc = bounded_reader.try_seek_from_begin(cTestStringLen);
REQUIRE(clp::ErrorCode_EndOfFile == rc);
REQUIRE(1 == string_reader.get_pos());
REQUIRE(1 == checkpoint_reader.get_pos());
REQUIRE(1 == bounded_reader.get_pos());
}

SECTION("CheckpointReader does allow seeks before checkpoint.") {
SECTION("BoundedReader does allow seeks before checkpoint.") {
clp::StringReader string_reader;
string_reader.open(cTestString);
clp::CheckpointReader checkpoint_reader{&string_reader, 2};
clp::BoundedReader bounded_reader{&string_reader, 2};
char buf[cTestStringLen];
size_t num_bytes_read{};
auto rc = checkpoint_reader.try_seek_from_begin(1);
auto rc = bounded_reader.try_seek_from_begin(1);
REQUIRE(clp::ErrorCode_Success == rc);
REQUIRE(1 == string_reader.get_pos());
REQUIRE(1 == checkpoint_reader.get_pos());
REQUIRE(1 == bounded_reader.get_pos());
}
}

0 comments on commit 8977dc4

Please sign in to comment.