szaszm commented on code in PR #1877:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1877#discussion_r1801154844
##########
extensions/standard-processors/processors/TailFile.h:
##########
@@ -253,21 +253,21 @@ class TailFile : public core::Processor {
const std::filesystem::path& full_file_name,
TailState &state);
bool getStateFromStateManager(std::map<std::filesystem::path, TailState>
&new_tail_states) const;
- bool getStateFromLegacyStateFile(core::ProcessContext& context,
+ bool getStateFromLegacyStateFile(const core::ProcessContext& context,
std::map<std::filesystem::path, TailState>
&new_tail_states) const;
void doMultifileLookup(core::ProcessContext& context);
void checkForRemovedFiles();
void checkForNewFiles(core::ProcessContext& context);
static std::string baseDirectoryFromAttributes(const
controllers::AttributeProviderService::AttributeMap& attribute_map,
core::ProcessContext& context);
void updateFlowFileAttributes(const std::filesystem::path& full_file_name,
const TailState &state, const std::filesystem::path& fileName,
const std::string &baseName, const std::string
&extension,
- std::shared_ptr<core::FlowFile> &flow_file)
const;
+ core::FlowFile& flow_file) const;
static void updateStateAttributes(TailState &state, uint64_t size, uint64_t
checksum);
- bool isOldFileInitiallyRead(TailState &state) const;
+ bool isOldFileInitiallyRead(const TailState &state) const;
static const char *CURRENT_STR;
static const char *POSITION_STR;
- static const int BUFFER_SIZE = 512;
+ static constexpr int BUFFER_SIZE = 512;
Review Comment:
We could increase this to improve performance
##########
extensions/standard-processors/processors/TailFile.cpp:
##########
@@ -695,8 +694,8 @@ void TailFile::processSingleFile(core::ProcessSession&
session,
auto flow_file = session.create();
session.write(flow_file, std::ref(file_reader));
- if (file_reader.useLatestFlowFile()) {
- updateFlowFileAttributes(full_file_name, state_copy, fileName,
baseName, extension, flow_file);
+ if (file_reader.endedWithDelimiter() || (state.is_rotated_ &&
flow_file->getSize() > 0)) {
Review Comment:
is this the fix?
##########
extensions/standard-processors/tests/unit/TailFileTests.cpp:
##########
@@ -18,138 +18,112 @@
#include <cstdio>
#include <fstream>
-#include <map>
-#include <memory>
-#include <utility>
#include <string>
#include <iostream>
#include <set>
#include <algorithm>
#include <random>
-#include <cstdlib>
#include "FlowController.h"
#include "unit/TestBase.h"
#include "unit/Catch.h"
#include "core/Core.h"
-#include "core/FlowFile.h"
#include "utils/file/FileUtils.h"
#include "utils/file/PathUtils.h"
#include "unit/ProvenanceTestHelper.h"
#include "core/Processor.h"
#include "core/ProcessContext.h"
#include "core/ProcessSession.h"
-#include "core/ProcessorNode.h"
#include "core/Resource.h"
#include "TailFile.h"
#include "LogAttribute.h"
#include "unit/TestUtils.h"
#include "utils/StringUtils.h"
#include "unit/SingleProcessorTestController.h"
+#include "TextFragmentUtils.h"
using namespace std::literals::chrono_literals;
-static const std::string NEWLINE_FILE = "" // NOLINT
+static constexpr std::string_view NEWLINE_FILE = "" // NOLINT
"one,two,three\n"
"four,five,six, seven";
-static const char *TMP_FILE = "minifi-tmpfile.txt";
-static const char *ROLLED_OVER_TMP_FILE = "minifi-tmpfile.txt.old.1";
-static const char *STATE_FILE = "minifi-state-file.txt";
-static const std::string ROLLED_OVER_TAIL_DATA = "rolled_over_data\n";
-static const std::string NEW_TAIL_DATA = "newdata\n";
-static const std::string ADDITIONALY_CREATED_FILE_CONTENT = "additional file
data\n";
+static constexpr std::string_view TMP_FILE = "minifi-tmpfile.txt";
+static constexpr std::string_view ROLLED_OVER_TMP_FILE =
"minifi-tmpfile.txt.old.1";
+static constexpr std::string_view STATE_FILE = "minifi-state-file.txt";
+static constexpr std::string_view ROLLED_OVER_TAIL_DATA = "rolled_over_data\n";
+static constexpr std::string_view NEW_TAIL_DATA = "newdata\n";
+static constexpr std::string_view ADDITIONALY_CREATED_FILE_CONTENT =
"additional file data\n";
namespace {
-std::filesystem::path createTempFile(const std::filesystem::path& directory,
const std::filesystem::path& file_name, const std::string& contents,
- std::ios_base::openmode open_mode = std::ios::out | std::ios::binary) {
+std::filesystem::path createTempFile(const std::filesystem::path& directory,
const std::filesystem::path& file_name,
+ const std::string_view contents, const std::ios_base::openmode open_mode =
std::ios::out | std::ios::binary,
+ const std::chrono::file_clock::duration offset = 0ms) {
if (!utils::file::exists(directory)) {
std::filesystem::create_directories(directory);
}
- std::string full_file_name = (directory / file_name).string();
- std::ofstream tmpfile{full_file_name, open_mode};
- tmpfile << contents;
+ auto full_file_name = directory / file_name;
+ {
+ std::ofstream tmp_file{full_file_name, open_mode};
+ tmp_file << contents;
+ }
Review Comment:
A bit of nitpick, but I would have written it like this:
```suggestion
std::ofstream{full_file_name, open_mode} << contents;
```
But the current version is fine too, feel free to close this if you like it
better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
