Feat/zip features by cutullic · Pull Request #139 · SolarFramework/SolARFramework

cutullic · 2026-09-02T15:07:55Z

No description provided.

jim-bcom · 2026-09-02T15:28:51Z

+ try {
+ // Create a working directory for zip/unzip features
+ fs::path wp(m_workingPath);
+ if (!fs::exists(wp)) {


Use temp_directory_pathhttps://en.cppreference.com/cpp/filesystem/temp_directory_path ?
This way you are guaranteed to get an existing directory, and it might enventually be removed when the machine restarts, if for some reason you were not able to delete it sooner.
Here, if the directory does exists, you might use it, and potentially overwrite user data, since it's not handled as an error.

Now that I think of it, concerning the two PR comment this seems to address: you might want to split the RAII part you did with the dtor of ZipBuffer, from the utility methods.
Make ZipBufferUtils (or whatever :)) have these 2 methods of extraction/compression as static helpers
ZipBufferUtils::extract() ZipBufferUtils::create/compress/...()
create a RAII object responsible to create the tmp dir and deleting it on destruction, something like:
classScopedTmpDir { ScopedTmpDir () { m_path = temp_directory_path();} ~ScopedTmpDir { fs::remove_all(m_path ); } fs::path getPath(); };
And use it in the static helper function.
This adds the benefit of deleting things ASAP, instead of waiting that the ZipBuffer object goes out of scope, which may be long after the end of the function (which I gues is why you kept deleting things at the end of the methods, instead of relying on the destructor, as I commented elsewhere).
What do you think of this approach ?

I like this approach :-) Made the modifications.

jim-bcom · 2026-09-02T15:32:44Z

+ try {
+ // Check working directory
+ fs::path wp(m_workingPath);
+ if (!fs::is_directory(wp)) {


If temp_directory_path this check is probably not needed (even in the current version). Unless we fear something might happen to this directory between the creation of ZipBuffer and the call to this.

Removed because temp_directory_path is now used.

jim-bcom · 2026-09-02T15:34:30Z

+ fs::path op(originalPath);
+ if (!fs::is_directory(op)) {
+ LOG_ERROR("The original path is not a directory: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


Maybe !exists => NOT_FOUND but !directory => ERROR (if we don't have BAD_ARGUMENT) ?

jim-bcom · 2026-09-02T15:35:45Z

+ }
+ if (fs::is_empty(op)) {
+ LOG_ERROR("The original path is empty: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


SUCCESS? You give an empty dir, you get an empty vector, job done ?

OK returns SUCCESS but with a warning (and clear the input buffer)

jim-bcom · 2026-09-02T15:42:05Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Can't we avoid the copy ? For example by invoking zip directly from originalPath ?
std::string command = "cd " + originalPath + ";zip -r " + m_workingPath+ "/data.zip .";

No, I prefer not to work on the original directory, to ensure I don't modify it.

Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

OK, I see the point. I will try to change this behavior.

jim-bcom · 2026-09-02T15:53:33Z

+ fs::remove(zipFile);
+
+ // Copy unzipped data in the destination directory
+ const auto copyOptions = fs::copy_options::recursive;


Could we avoid the copy by invoking zip from the destination directory?
std::string command = "cd " + destinationPath+ "; unzip " + m_workingPath + " /data.zip";

See previous comment

jim-bcom · 2026-09-02T15:53:49Z

+
+// private
+
+void ZipBuffer::cleanWorkingDirectory()


No longer useful at all: removed

jim-bcom · 2026-09-02T16:01:02Z

+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * FrameworkReturnCode::_NOT_FOUND if data is not found in original path
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode zipToBuffer(const std::string & originalPath,


nit: use directly fs::path i.o. string for path ?

I thought using std::string avoided forcing the caller to use std::filesystem?

jim-bcom · 2026-09-02T16:03:33Z

+ /// @return
+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode bufferToUnzip(const std::vector<unsigned char> & compressedZipBuffer,


nit: matter of taste, but with verbs for method ? ZipBuffer::extract()/ZipBuffer::compress()? Or something else.

jim-bcom · 2026-09-02T16:07:21Z

+ *
+ */
+
+class ZipBuffer {


Again naming things is hard... It does not per say represent a ZipBuffer, but more like utils functions... But I'm not sure about ZipBufferUtils... I don't have a better idea :). Something like ZipBufferExtractor (but it does also compress, so ...), or ZipBufferProcessor,...

What about just ZipUtils ?

Let's go for ZipBufferUtils class and extractand compressmethods

jim-bcom · 2026-09-03T12:05:57Z

+#include <vector>
+#include <filesystem>
+
+namespace fs = std::filesystem;


Remove, avoid aliasing in header, especially at this scope.

Yep, sorry!

jim-bcom · 2026-09-03T12:14:09Z

+ * @brief <B>Create a temporary working directory</B>
+ *
+ */
+ class ScopedWorkingDir {


This class is decorrelated to ZipBufferUtils, so 2 suggestions:
define in its own file for anyboady to reuse
define it in the cpp file, because it's not referenced here, so it does not have to be defined here.
It's only an implementation detail that does not have to surface here (except if this class is needed elsewhere, but then this would have to be defined in its own file, as suggested in 1.

Also maybe keep Tmp or Temp in the name (ScopedTmpDir), so that we know it uses the tmp folder API, and this is where the file is created ...?

jim-bcom · 2026-09-03T12:24:21Z

+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }
+ ~ScopedWorkingDir() { fs::remove_all(m_workingPath); }


Delete copy operations to prevent double deletion
ScopedWorkingDir(const ScopedWorkingDir&) = delete; ScopedWorkingDir& operator=(const ScopedWorkingDir&) = delete; ScopedWorkingDir(ScopedWorkingDir&&) = delete; ScopedWorkingDir& operator=(ScopedWorkingDir&&) = delete;
(move operations are already deleted by the fact of defining the destructor, but this way it's explicit)

jim-bcom · 2026-09-03T12:30:14Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


Make the subdir a (optional?) parameter of the (explicit) ctor?
explicitScopedWorkingDir(const std::string& subDir = "");

This class has moved to the .cpp file, and will only be used by ZipBufferUtils static methods, so I don't think it's usefull to make the sudir configurable

jim-bcom · 2026-09-03T12:32:10Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


nit: use operator / for path separator, example of syntax:
m_workingPath = m_workingPath / "solar"; m_workingPath /= "solar";

Both operations are available : why this one instead of the other one?

OK, changed

jim-bcom · 2026-09-03T12:58:38Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

jim-bcom · 2026-09-03T13:01:08Z

+ LOG_DEBUG("ZipBufferUtils::extract - Working temporary path: {}", workingDir.getStringPath());
+
+ // Check/create the working directory
+ if (!fs::exists(workingDir.getPath())) {


No longer needed?

Yes, still needed here: the subdir must be created here before creating the zip file (tested)

jim-bcom · 2026-09-03T13:03:19Z

+ }
+
+ try {
+ fs::path dp(destinationPath);


In the other function you clear the output buffer, here we could delete the content of the destination path as well ?

I don't think so: this destination directory can contain other files, we don't know.

jim-bcom · 2026-09-03T13:04:24Z

+ return FrameworkReturnCode::_ERROR_;
+ }
+
+ // Delete the zip file


No longer needed ?

Ah yes you do it because the zip is in the same dir as the one you copy later on, OK.

jim-bcom · 2026-09-03T13:06:30Z

+ file.close();
+
+ // Try to unzip the file content
+ std::string command = "cd " + workingDir.getStringPath() + "; unzip data.zip";


Here you could avoid the copy by cd in the dest dir, there's really no risk of messing with existing files, since it's supposed to receive ours, no?

Yes, I think you're right and I will change it

cutullic · 2026-09-02T15:07:55Z

No description provided.

jim-bcom · 2026-09-02T15:28:51Z

+ try {
+ // Create a working directory for zip/unzip features
+ fs::path wp(m_workingPath);
+ if (!fs::exists(wp)) {


Use temp_directory_pathhttps://en.cppreference.com/cpp/filesystem/temp_directory_path ?
This way you are guaranteed to get an existing directory, and it might enventually be removed when the machine restarts, if for some reason you were not able to delete it sooner.
Here, if the directory does exists, you might use it, and potentially overwrite user data, since it's not handled as an error.

Now that I think of it, concerning the two PR comment this seems to address: you might want to split the RAII part you did with the dtor of ZipBuffer, from the utility methods.
Make ZipBufferUtils (or whatever :)) have these 2 methods of extraction/compression as static helpers
ZipBufferUtils::extract() ZipBufferUtils::create/compress/...()
create a RAII object responsible to create the tmp dir and deleting it on destruction, something like:
classScopedTmpDir { ScopedTmpDir () { m_path = temp_directory_path();} ~ScopedTmpDir { fs::remove_all(m_path ); } fs::path getPath(); };
And use it in the static helper function.
This adds the benefit of deleting things ASAP, instead of waiting that the ZipBuffer object goes out of scope, which may be long after the end of the function (which I gues is why you kept deleting things at the end of the methods, instead of relying on the destructor, as I commented elsewhere).
What do you think of this approach ?

I like this approach :-) Made the modifications.

jim-bcom · 2026-09-02T15:32:44Z

+ try {
+ // Check working directory
+ fs::path wp(m_workingPath);
+ if (!fs::is_directory(wp)) {


If temp_directory_path this check is probably not needed (even in the current version). Unless we fear something might happen to this directory between the creation of ZipBuffer and the call to this.

Removed because temp_directory_path is now used.

jim-bcom · 2026-09-02T15:34:30Z

+ fs::path op(originalPath);
+ if (!fs::is_directory(op)) {
+ LOG_ERROR("The original path is not a directory: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


Maybe !exists => NOT_FOUND but !directory => ERROR (if we don't have BAD_ARGUMENT) ?

jim-bcom · 2026-09-02T15:35:45Z

+ }
+ if (fs::is_empty(op)) {
+ LOG_ERROR("The original path is empty: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


SUCCESS? You give an empty dir, you get an empty vector, job done ?

OK returns SUCCESS but with a warning (and clear the input buffer)

jim-bcom · 2026-09-02T15:42:05Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Can't we avoid the copy ? For example by invoking zip directly from originalPath ?
std::string command = "cd " + originalPath + ";zip -r " + m_workingPath+ "/data.zip .";

No, I prefer not to work on the original directory, to ensure I don't modify it.

Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

OK, I see the point. I will try to change this behavior.

jim-bcom · 2026-09-02T15:53:33Z

+ fs::remove(zipFile);
+
+ // Copy unzipped data in the destination directory
+ const auto copyOptions = fs::copy_options::recursive;


Could we avoid the copy by invoking zip from the destination directory?
std::string command = "cd " + destinationPath+ "; unzip " + m_workingPath + " /data.zip";

See previous comment

jim-bcom · 2026-09-02T15:53:49Z

+
+// private
+
+void ZipBuffer::cleanWorkingDirectory()


No longer useful at all: removed

jim-bcom · 2026-09-02T16:01:02Z

+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * FrameworkReturnCode::_NOT_FOUND if data is not found in original path
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode zipToBuffer(const std::string & originalPath,


nit: use directly fs::path i.o. string for path ?

I thought using std::string avoided forcing the caller to use std::filesystem?

jim-bcom · 2026-09-02T16:03:33Z

+ /// @return
+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode bufferToUnzip(const std::vector<unsigned char> & compressedZipBuffer,


nit: matter of taste, but with verbs for method ? ZipBuffer::extract()/ZipBuffer::compress()? Or something else.

jim-bcom · 2026-09-02T16:07:21Z

+ *
+ */
+
+class ZipBuffer {


Again naming things is hard... It does not per say represent a ZipBuffer, but more like utils functions... But I'm not sure about ZipBufferUtils... I don't have a better idea :). Something like ZipBufferExtractor (but it does also compress, so ...), or ZipBufferProcessor,...

What about just ZipUtils ?

Let's go for ZipBufferUtils class and extractand compressmethods

jim-bcom · 2026-09-03T12:05:57Z

+#include <vector>
+#include <filesystem>
+
+namespace fs = std::filesystem;


Remove, avoid aliasing in header, especially at this scope.

Yep, sorry!

jim-bcom · 2026-09-03T12:14:09Z

+ * @brief <B>Create a temporary working directory</B>
+ *
+ */
+ class ScopedWorkingDir {


This class is decorrelated to ZipBufferUtils, so 2 suggestions:
define in its own file for anyboady to reuse
define it in the cpp file, because it's not referenced here, so it does not have to be defined here.
It's only an implementation detail that does not have to surface here (except if this class is needed elsewhere, but then this would have to be defined in its own file, as suggested in 1.

Also maybe keep Tmp or Temp in the name (ScopedTmpDir), so that we know it uses the tmp folder API, and this is where the file is created ...?

jim-bcom · 2026-09-03T12:24:21Z

+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }
+ ~ScopedWorkingDir() { fs::remove_all(m_workingPath); }


Delete copy operations to prevent double deletion
ScopedWorkingDir(const ScopedWorkingDir&) = delete; ScopedWorkingDir& operator=(const ScopedWorkingDir&) = delete; ScopedWorkingDir(ScopedWorkingDir&&) = delete; ScopedWorkingDir& operator=(ScopedWorkingDir&&) = delete;
(move operations are already deleted by the fact of defining the destructor, but this way it's explicit)

jim-bcom · 2026-09-03T12:30:14Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


Make the subdir a (optional?) parameter of the (explicit) ctor?
explicitScopedWorkingDir(const std::string& subDir = "");

This class has moved to the .cpp file, and will only be used by ZipBufferUtils static methods, so I don't think it's usefull to make the sudir configurable

jim-bcom · 2026-09-03T12:32:10Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


nit: use operator / for path separator, example of syntax:
m_workingPath = m_workingPath / "solar"; m_workingPath /= "solar";

Both operations are available : why this one instead of the other one?

OK, changed

jim-bcom · 2026-09-03T12:58:38Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

jim-bcom · 2026-09-03T13:01:08Z

+ LOG_DEBUG("ZipBufferUtils::extract - Working temporary path: {}", workingDir.getStringPath());
+
+ // Check/create the working directory
+ if (!fs::exists(workingDir.getPath())) {


No longer needed?

Yes, still needed here: the subdir must be created here before creating the zip file (tested)

jim-bcom · 2026-09-03T13:03:19Z

+ }
+
+ try {
+ fs::path dp(destinationPath);


In the other function you clear the output buffer, here we could delete the content of the destination path as well ?

I don't think so: this destination directory can contain other files, we don't know.

jim-bcom · 2026-09-03T13:04:24Z

+ return FrameworkReturnCode::_ERROR_;
+ }
+
+ // Delete the zip file


No longer needed ?

Ah yes you do it because the zip is in the same dir as the one you copy later on, OK.

jim-bcom · 2026-09-03T13:06:30Z

+ file.close();
+
+ // Try to unzip the file content
+ std::string command = "cd " + workingDir.getStringPath() + "; unzip data.zip";


Here you could avoid the copy by cd in the dest dir, there's really no risk of messing with existing files, since it's supposed to receive ours, no?

Yes, I think you're right and I will change it

cutullic · 2026-09-02T15:07:55Z

No description provided.

jim-bcom · 2026-09-02T15:28:51Z

+ try {
+ // Create a working directory for zip/unzip features
+ fs::path wp(m_workingPath);
+ if (!fs::exists(wp)) {


Use temp_directory_pathhttps://en.cppreference.com/cpp/filesystem/temp_directory_path ?
This way you are guaranteed to get an existing directory, and it might enventually be removed when the machine restarts, if for some reason you were not able to delete it sooner.
Here, if the directory does exists, you might use it, and potentially overwrite user data, since it's not handled as an error.

Now that I think of it, concerning the two PR comment this seems to address: you might want to split the RAII part you did with the dtor of ZipBuffer, from the utility methods.
Make ZipBufferUtils (or whatever :)) have these 2 methods of extraction/compression as static helpers
ZipBufferUtils::extract() ZipBufferUtils::create/compress/...()
create a RAII object responsible to create the tmp dir and deleting it on destruction, something like:
classScopedTmpDir { ScopedTmpDir () { m_path = temp_directory_path();} ~ScopedTmpDir { fs::remove_all(m_path ); } fs::path getPath(); };
And use it in the static helper function.
This adds the benefit of deleting things ASAP, instead of waiting that the ZipBuffer object goes out of scope, which may be long after the end of the function (which I gues is why you kept deleting things at the end of the methods, instead of relying on the destructor, as I commented elsewhere).
What do you think of this approach ?

I like this approach :-) Made the modifications.

jim-bcom · 2026-09-02T15:32:44Z

+ try {
+ // Check working directory
+ fs::path wp(m_workingPath);
+ if (!fs::is_directory(wp)) {


If temp_directory_path this check is probably not needed (even in the current version). Unless we fear something might happen to this directory between the creation of ZipBuffer and the call to this.

Removed because temp_directory_path is now used.

jim-bcom · 2026-09-02T15:34:30Z

+ fs::path op(originalPath);
+ if (!fs::is_directory(op)) {
+ LOG_ERROR("The original path is not a directory: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


Maybe !exists => NOT_FOUND but !directory => ERROR (if we don't have BAD_ARGUMENT) ?

jim-bcom · 2026-09-02T15:35:45Z

+ }
+ if (fs::is_empty(op)) {
+ LOG_ERROR("The original path is empty: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


SUCCESS? You give an empty dir, you get an empty vector, job done ?

OK returns SUCCESS but with a warning (and clear the input buffer)

jim-bcom · 2026-09-02T15:42:05Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Can't we avoid the copy ? For example by invoking zip directly from originalPath ?
std::string command = "cd " + originalPath + ";zip -r " + m_workingPath+ "/data.zip .";

No, I prefer not to work on the original directory, to ensure I don't modify it.

Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

OK, I see the point. I will try to change this behavior.

jim-bcom · 2026-09-02T15:53:33Z

+ fs::remove(zipFile);
+
+ // Copy unzipped data in the destination directory
+ const auto copyOptions = fs::copy_options::recursive;


Could we avoid the copy by invoking zip from the destination directory?
std::string command = "cd " + destinationPath+ "; unzip " + m_workingPath + " /data.zip";

See previous comment

jim-bcom · 2026-09-02T15:53:49Z

+
+// private
+
+void ZipBuffer::cleanWorkingDirectory()


No longer useful at all: removed

jim-bcom · 2026-09-02T16:01:02Z

+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * FrameworkReturnCode::_NOT_FOUND if data is not found in original path
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode zipToBuffer(const std::string & originalPath,


nit: use directly fs::path i.o. string for path ?

I thought using std::string avoided forcing the caller to use std::filesystem?

jim-bcom · 2026-09-02T16:03:33Z

+ /// @return
+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode bufferToUnzip(const std::vector<unsigned char> & compressedZipBuffer,


nit: matter of taste, but with verbs for method ? ZipBuffer::extract()/ZipBuffer::compress()? Or something else.

jim-bcom · 2026-09-02T16:07:21Z

+ *
+ */
+
+class ZipBuffer {


Again naming things is hard... It does not per say represent a ZipBuffer, but more like utils functions... But I'm not sure about ZipBufferUtils... I don't have a better idea :). Something like ZipBufferExtractor (but it does also compress, so ...), or ZipBufferProcessor,...

What about just ZipUtils ?

Let's go for ZipBufferUtils class and extractand compressmethods

jim-bcom · 2026-09-03T12:05:57Z

+#include <vector>
+#include <filesystem>
+
+namespace fs = std::filesystem;


Remove, avoid aliasing in header, especially at this scope.

Yep, sorry!

jim-bcom · 2026-09-03T12:14:09Z

+ * @brief <B>Create a temporary working directory</B>
+ *
+ */
+ class ScopedWorkingDir {


This class is decorrelated to ZipBufferUtils, so 2 suggestions:
define in its own file for anyboady to reuse
define it in the cpp file, because it's not referenced here, so it does not have to be defined here.
It's only an implementation detail that does not have to surface here (except if this class is needed elsewhere, but then this would have to be defined in its own file, as suggested in 1.

Also maybe keep Tmp or Temp in the name (ScopedTmpDir), so that we know it uses the tmp folder API, and this is where the file is created ...?

jim-bcom · 2026-09-03T12:24:21Z

+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }
+ ~ScopedWorkingDir() { fs::remove_all(m_workingPath); }


Delete copy operations to prevent double deletion
ScopedWorkingDir(const ScopedWorkingDir&) = delete; ScopedWorkingDir& operator=(const ScopedWorkingDir&) = delete; ScopedWorkingDir(ScopedWorkingDir&&) = delete; ScopedWorkingDir& operator=(ScopedWorkingDir&&) = delete;
(move operations are already deleted by the fact of defining the destructor, but this way it's explicit)

jim-bcom · 2026-09-03T12:30:14Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


Make the subdir a (optional?) parameter of the (explicit) ctor?
explicitScopedWorkingDir(const std::string& subDir = "");

This class has moved to the .cpp file, and will only be used by ZipBufferUtils static methods, so I don't think it's usefull to make the sudir configurable

jim-bcom · 2026-09-03T12:32:10Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


nit: use operator / for path separator, example of syntax:
m_workingPath = m_workingPath / "solar"; m_workingPath /= "solar";

Both operations are available : why this one instead of the other one?

OK, changed

jim-bcom · 2026-09-03T12:58:38Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

jim-bcom · 2026-09-03T13:01:08Z

+ LOG_DEBUG("ZipBufferUtils::extract - Working temporary path: {}", workingDir.getStringPath());
+
+ // Check/create the working directory
+ if (!fs::exists(workingDir.getPath())) {


No longer needed?

Yes, still needed here: the subdir must be created here before creating the zip file (tested)

jim-bcom · 2026-09-03T13:03:19Z

+ }
+
+ try {
+ fs::path dp(destinationPath);


In the other function you clear the output buffer, here we could delete the content of the destination path as well ?

I don't think so: this destination directory can contain other files, we don't know.

jim-bcom · 2026-09-03T13:04:24Z

+ return FrameworkReturnCode::_ERROR_;
+ }
+
+ // Delete the zip file


No longer needed ?

Ah yes you do it because the zip is in the same dir as the one you copy later on, OK.

jim-bcom · 2026-09-03T13:06:30Z

+ file.close();
+
+ // Try to unzip the file content
+ std::string command = "cd " + workingDir.getStringPath() + "; unzip data.zip";


Here you could avoid the copy by cd in the dest dir, there's really no risk of messing with existing files, since it's supposed to receive ours, no?

Yes, I think you're right and I will change it

cutullic · 2026-09-02T15:07:55Z

No description provided.

jim-bcom · 2026-09-02T15:28:51Z

+ try {
+ // Create a working directory for zip/unzip features
+ fs::path wp(m_workingPath);
+ if (!fs::exists(wp)) {


Use temp_directory_pathhttps://en.cppreference.com/cpp/filesystem/temp_directory_path ?
This way you are guaranteed to get an existing directory, and it might enventually be removed when the machine restarts, if for some reason you were not able to delete it sooner.
Here, if the directory does exists, you might use it, and potentially overwrite user data, since it's not handled as an error.

Now that I think of it, concerning the two PR comment this seems to address: you might want to split the RAII part you did with the dtor of ZipBuffer, from the utility methods.
Make ZipBufferUtils (or whatever :)) have these 2 methods of extraction/compression as static helpers
ZipBufferUtils::extract() ZipBufferUtils::create/compress/...()
create a RAII object responsible to create the tmp dir and deleting it on destruction, something like:
classScopedTmpDir { ScopedTmpDir () { m_path = temp_directory_path();} ~ScopedTmpDir { fs::remove_all(m_path ); } fs::path getPath(); };
And use it in the static helper function.
This adds the benefit of deleting things ASAP, instead of waiting that the ZipBuffer object goes out of scope, which may be long after the end of the function (which I gues is why you kept deleting things at the end of the methods, instead of relying on the destructor, as I commented elsewhere).
What do you think of this approach ?

I like this approach :-) Made the modifications.

jim-bcom · 2026-09-02T15:32:44Z

+ try {
+ // Check working directory
+ fs::path wp(m_workingPath);
+ if (!fs::is_directory(wp)) {


If temp_directory_path this check is probably not needed (even in the current version). Unless we fear something might happen to this directory between the creation of ZipBuffer and the call to this.

Removed because temp_directory_path is now used.

jim-bcom · 2026-09-02T15:34:30Z

+ fs::path op(originalPath);
+ if (!fs::is_directory(op)) {
+ LOG_ERROR("The original path is not a directory: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


Maybe !exists => NOT_FOUND but !directory => ERROR (if we don't have BAD_ARGUMENT) ?

jim-bcom · 2026-09-02T15:35:45Z

+ }
+ if (fs::is_empty(op)) {
+ LOG_ERROR("The original path is empty: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


SUCCESS? You give an empty dir, you get an empty vector, job done ?

OK returns SUCCESS but with a warning (and clear the input buffer)

jim-bcom · 2026-09-02T15:42:05Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Can't we avoid the copy ? For example by invoking zip directly from originalPath ?
std::string command = "cd " + originalPath + ";zip -r " + m_workingPath+ "/data.zip .";

No, I prefer not to work on the original directory, to ensure I don't modify it.

Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

OK, I see the point. I will try to change this behavior.

jim-bcom · 2026-09-02T15:53:33Z

+ fs::remove(zipFile);
+
+ // Copy unzipped data in the destination directory
+ const auto copyOptions = fs::copy_options::recursive;


Could we avoid the copy by invoking zip from the destination directory?
std::string command = "cd " + destinationPath+ "; unzip " + m_workingPath + " /data.zip";

See previous comment

jim-bcom · 2026-09-02T15:53:49Z

+
+// private
+
+void ZipBuffer::cleanWorkingDirectory()


No longer useful at all: removed

jim-bcom · 2026-09-02T16:01:02Z

+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * FrameworkReturnCode::_NOT_FOUND if data is not found in original path
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode zipToBuffer(const std::string & originalPath,


nit: use directly fs::path i.o. string for path ?

I thought using std::string avoided forcing the caller to use std::filesystem?

jim-bcom · 2026-09-02T16:03:33Z

+ /// @return
+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode bufferToUnzip(const std::vector<unsigned char> & compressedZipBuffer,


nit: matter of taste, but with verbs for method ? ZipBuffer::extract()/ZipBuffer::compress()? Or something else.

jim-bcom · 2026-09-02T16:07:21Z

+ *
+ */
+
+class ZipBuffer {


Again naming things is hard... It does not per say represent a ZipBuffer, but more like utils functions... But I'm not sure about ZipBufferUtils... I don't have a better idea :). Something like ZipBufferExtractor (but it does also compress, so ...), or ZipBufferProcessor,...

What about just ZipUtils ?

Let's go for ZipBufferUtils class and extractand compressmethods

jim-bcom · 2026-09-03T12:05:57Z

+#include <vector>
+#include <filesystem>
+
+namespace fs = std::filesystem;


Remove, avoid aliasing in header, especially at this scope.

Yep, sorry!

jim-bcom · 2026-09-03T12:14:09Z

+ * @brief <B>Create a temporary working directory</B>
+ *
+ */
+ class ScopedWorkingDir {


This class is decorrelated to ZipBufferUtils, so 2 suggestions:
define in its own file for anyboady to reuse
define it in the cpp file, because it's not referenced here, so it does not have to be defined here.
It's only an implementation detail that does not have to surface here (except if this class is needed elsewhere, but then this would have to be defined in its own file, as suggested in 1.

Also maybe keep Tmp or Temp in the name (ScopedTmpDir), so that we know it uses the tmp folder API, and this is where the file is created ...?

jim-bcom · 2026-09-03T12:24:21Z

+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }
+ ~ScopedWorkingDir() { fs::remove_all(m_workingPath); }


Delete copy operations to prevent double deletion
ScopedWorkingDir(const ScopedWorkingDir&) = delete; ScopedWorkingDir& operator=(const ScopedWorkingDir&) = delete; ScopedWorkingDir(ScopedWorkingDir&&) = delete; ScopedWorkingDir& operator=(ScopedWorkingDir&&) = delete;
(move operations are already deleted by the fact of defining the destructor, but this way it's explicit)

jim-bcom · 2026-09-03T12:30:14Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


Make the subdir a (optional?) parameter of the (explicit) ctor?
explicitScopedWorkingDir(const std::string& subDir = "");

This class has moved to the .cpp file, and will only be used by ZipBufferUtils static methods, so I don't think it's usefull to make the sudir configurable

jim-bcom · 2026-09-03T12:32:10Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


nit: use operator / for path separator, example of syntax:
m_workingPath = m_workingPath / "solar"; m_workingPath /= "solar";

Both operations are available : why this one instead of the other one?

OK, changed

jim-bcom · 2026-09-03T12:58:38Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

jim-bcom · 2026-09-03T13:01:08Z

+ LOG_DEBUG("ZipBufferUtils::extract - Working temporary path: {}", workingDir.getStringPath());
+
+ // Check/create the working directory
+ if (!fs::exists(workingDir.getPath())) {


No longer needed?

Yes, still needed here: the subdir must be created here before creating the zip file (tested)

jim-bcom · 2026-09-03T13:03:19Z

+ }
+
+ try {
+ fs::path dp(destinationPath);


In the other function you clear the output buffer, here we could delete the content of the destination path as well ?

I don't think so: this destination directory can contain other files, we don't know.

jim-bcom · 2026-09-03T13:04:24Z

+ return FrameworkReturnCode::_ERROR_;
+ }
+
+ // Delete the zip file


No longer needed ?

Ah yes you do it because the zip is in the same dir as the one you copy later on, OK.

jim-bcom · 2026-09-03T13:06:30Z

+ file.close();
+
+ // Try to unzip the file content
+ std::string command = "cd " + workingDir.getStringPath() + "; unzip data.zip";


Here you could avoid the copy by cd in the dest dir, there's really no risk of messing with existing files, since it's supposed to receive ours, no?

Yes, I think you're right and I will change it

cutullic · 2026-09-02T15:07:55Z

No description provided.

jim-bcom · 2026-09-02T15:28:51Z

+ try {
+ // Create a working directory for zip/unzip features
+ fs::path wp(m_workingPath);
+ if (!fs::exists(wp)) {


Use temp_directory_pathhttps://en.cppreference.com/cpp/filesystem/temp_directory_path ?
This way you are guaranteed to get an existing directory, and it might enventually be removed when the machine restarts, if for some reason you were not able to delete it sooner.
Here, if the directory does exists, you might use it, and potentially overwrite user data, since it's not handled as an error.

Now that I think of it, concerning the two PR comment this seems to address: you might want to split the RAII part you did with the dtor of ZipBuffer, from the utility methods.
Make ZipBufferUtils (or whatever :)) have these 2 methods of extraction/compression as static helpers
ZipBufferUtils::extract() ZipBufferUtils::create/compress/...()
create a RAII object responsible to create the tmp dir and deleting it on destruction, something like:
classScopedTmpDir { ScopedTmpDir () { m_path = temp_directory_path();} ~ScopedTmpDir { fs::remove_all(m_path ); } fs::path getPath(); };
And use it in the static helper function.
This adds the benefit of deleting things ASAP, instead of waiting that the ZipBuffer object goes out of scope, which may be long after the end of the function (which I gues is why you kept deleting things at the end of the methods, instead of relying on the destructor, as I commented elsewhere).
What do you think of this approach ?

I like this approach :-) Made the modifications.

jim-bcom · 2026-09-02T15:32:44Z

+ try {
+ // Check working directory
+ fs::path wp(m_workingPath);
+ if (!fs::is_directory(wp)) {


If temp_directory_path this check is probably not needed (even in the current version). Unless we fear something might happen to this directory between the creation of ZipBuffer and the call to this.

Removed because temp_directory_path is now used.

jim-bcom · 2026-09-02T15:34:30Z

+ fs::path op(originalPath);
+ if (!fs::is_directory(op)) {
+ LOG_ERROR("The original path is not a directory: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


Maybe !exists => NOT_FOUND but !directory => ERROR (if we don't have BAD_ARGUMENT) ?

jim-bcom · 2026-09-02T15:35:45Z

+ }
+ if (fs::is_empty(op)) {
+ LOG_ERROR("The original path is empty: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


SUCCESS? You give an empty dir, you get an empty vector, job done ?

OK returns SUCCESS but with a warning (and clear the input buffer)

jim-bcom · 2026-09-02T15:42:05Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Can't we avoid the copy ? For example by invoking zip directly from originalPath ?
std::string command = "cd " + originalPath + ";zip -r " + m_workingPath+ "/data.zip .";

No, I prefer not to work on the original directory, to ensure I don't modify it.

Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

OK, I see the point. I will try to change this behavior.

jim-bcom · 2026-09-02T15:53:33Z

+ fs::remove(zipFile);
+
+ // Copy unzipped data in the destination directory
+ const auto copyOptions = fs::copy_options::recursive;


Could we avoid the copy by invoking zip from the destination directory?
std::string command = "cd " + destinationPath+ "; unzip " + m_workingPath + " /data.zip";

See previous comment

jim-bcom · 2026-09-02T15:53:49Z

+
+// private
+
+void ZipBuffer::cleanWorkingDirectory()


No longer useful at all: removed

jim-bcom · 2026-09-02T16:01:02Z

+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * FrameworkReturnCode::_NOT_FOUND if data is not found in original path
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode zipToBuffer(const std::string & originalPath,


nit: use directly fs::path i.o. string for path ?

I thought using std::string avoided forcing the caller to use std::filesystem?

jim-bcom · 2026-09-02T16:03:33Z

+ /// @return
+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode bufferToUnzip(const std::vector<unsigned char> & compressedZipBuffer,


nit: matter of taste, but with verbs for method ? ZipBuffer::extract()/ZipBuffer::compress()? Or something else.

jim-bcom · 2026-09-02T16:07:21Z

+ *
+ */
+
+class ZipBuffer {


Again naming things is hard... It does not per say represent a ZipBuffer, but more like utils functions... But I'm not sure about ZipBufferUtils... I don't have a better idea :). Something like ZipBufferExtractor (but it does also compress, so ...), or ZipBufferProcessor,...

What about just ZipUtils ?

Let's go for ZipBufferUtils class and extractand compressmethods

jim-bcom · 2026-09-03T12:05:57Z

+#include <vector>
+#include <filesystem>
+
+namespace fs = std::filesystem;


Remove, avoid aliasing in header, especially at this scope.

Yep, sorry!

jim-bcom · 2026-09-03T12:14:09Z

+ * @brief <B>Create a temporary working directory</B>
+ *
+ */
+ class ScopedWorkingDir {


This class is decorrelated to ZipBufferUtils, so 2 suggestions:
define in its own file for anyboady to reuse
define it in the cpp file, because it's not referenced here, so it does not have to be defined here.
It's only an implementation detail that does not have to surface here (except if this class is needed elsewhere, but then this would have to be defined in its own file, as suggested in 1.

Also maybe keep Tmp or Temp in the name (ScopedTmpDir), so that we know it uses the tmp folder API, and this is where the file is created ...?

jim-bcom · 2026-09-03T12:24:21Z

+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }
+ ~ScopedWorkingDir() { fs::remove_all(m_workingPath); }


Delete copy operations to prevent double deletion
ScopedWorkingDir(const ScopedWorkingDir&) = delete; ScopedWorkingDir& operator=(const ScopedWorkingDir&) = delete; ScopedWorkingDir(ScopedWorkingDir&&) = delete; ScopedWorkingDir& operator=(ScopedWorkingDir&&) = delete;
(move operations are already deleted by the fact of defining the destructor, but this way it's explicit)

jim-bcom · 2026-09-03T12:30:14Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


Make the subdir a (optional?) parameter of the (explicit) ctor?
explicitScopedWorkingDir(const std::string& subDir = "");

This class has moved to the .cpp file, and will only be used by ZipBufferUtils static methods, so I don't think it's usefull to make the sudir configurable

jim-bcom · 2026-09-03T12:32:10Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


nit: use operator / for path separator, example of syntax:
m_workingPath = m_workingPath / "solar"; m_workingPath /= "solar";

Both operations are available : why this one instead of the other one?

OK, changed

jim-bcom · 2026-09-03T12:58:38Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

jim-bcom · 2026-09-03T13:01:08Z

+ LOG_DEBUG("ZipBufferUtils::extract - Working temporary path: {}", workingDir.getStringPath());
+
+ // Check/create the working directory
+ if (!fs::exists(workingDir.getPath())) {


No longer needed?

Yes, still needed here: the subdir must be created here before creating the zip file (tested)

jim-bcom · 2026-09-03T13:03:19Z

+ }
+
+ try {
+ fs::path dp(destinationPath);


In the other function you clear the output buffer, here we could delete the content of the destination path as well ?

I don't think so: this destination directory can contain other files, we don't know.

jim-bcom · 2026-09-03T13:04:24Z

+ return FrameworkReturnCode::_ERROR_;
+ }
+
+ // Delete the zip file


No longer needed ?

Ah yes you do it because the zip is in the same dir as the one you copy later on, OK.

jim-bcom · 2026-09-03T13:06:30Z

+ file.close();
+
+ // Try to unzip the file content
+ std::string command = "cd " + workingDir.getStringPath() + "; unzip data.zip";


Here you could avoid the copy by cd in the dest dir, there's really no risk of messing with existing files, since it's supposed to receive ours, no?

Yes, I think you're right and I will change it

cutullic · 2026-09-02T15:07:55Z

No description provided.

jim-bcom · 2026-09-02T15:28:51Z

+ try {
+ // Create a working directory for zip/unzip features
+ fs::path wp(m_workingPath);
+ if (!fs::exists(wp)) {


Use temp_directory_pathhttps://en.cppreference.com/cpp/filesystem/temp_directory_path ?
This way you are guaranteed to get an existing directory, and it might enventually be removed when the machine restarts, if for some reason you were not able to delete it sooner.
Here, if the directory does exists, you might use it, and potentially overwrite user data, since it's not handled as an error.

Now that I think of it, concerning the two PR comment this seems to address: you might want to split the RAII part you did with the dtor of ZipBuffer, from the utility methods.
Make ZipBufferUtils (or whatever :)) have these 2 methods of extraction/compression as static helpers
ZipBufferUtils::extract() ZipBufferUtils::create/compress/...()
create a RAII object responsible to create the tmp dir and deleting it on destruction, something like:
classScopedTmpDir { ScopedTmpDir () { m_path = temp_directory_path();} ~ScopedTmpDir { fs::remove_all(m_path ); } fs::path getPath(); };
And use it in the static helper function.
This adds the benefit of deleting things ASAP, instead of waiting that the ZipBuffer object goes out of scope, which may be long after the end of the function (which I gues is why you kept deleting things at the end of the methods, instead of relying on the destructor, as I commented elsewhere).
What do you think of this approach ?

I like this approach :-) Made the modifications.

jim-bcom · 2026-09-02T15:32:44Z

+ try {
+ // Check working directory
+ fs::path wp(m_workingPath);
+ if (!fs::is_directory(wp)) {


If temp_directory_path this check is probably not needed (even in the current version). Unless we fear something might happen to this directory between the creation of ZipBuffer and the call to this.

Removed because temp_directory_path is now used.

jim-bcom · 2026-09-02T15:34:30Z

+ fs::path op(originalPath);
+ if (!fs::is_directory(op)) {
+ LOG_ERROR("The original path is not a directory: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


Maybe !exists => NOT_FOUND but !directory => ERROR (if we don't have BAD_ARGUMENT) ?

jim-bcom · 2026-09-02T15:35:45Z

+ }
+ if (fs::is_empty(op)) {
+ LOG_ERROR("The original path is empty: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


SUCCESS? You give an empty dir, you get an empty vector, job done ?

OK returns SUCCESS but with a warning (and clear the input buffer)

jim-bcom · 2026-09-02T15:42:05Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Can't we avoid the copy ? For example by invoking zip directly from originalPath ?
std::string command = "cd " + originalPath + ";zip -r " + m_workingPath+ "/data.zip .";

No, I prefer not to work on the original directory, to ensure I don't modify it.

Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

OK, I see the point. I will try to change this behavior.

jim-bcom · 2026-09-02T15:53:33Z

+ fs::remove(zipFile);
+
+ // Copy unzipped data in the destination directory
+ const auto copyOptions = fs::copy_options::recursive;


Could we avoid the copy by invoking zip from the destination directory?
std::string command = "cd " + destinationPath+ "; unzip " + m_workingPath + " /data.zip";

See previous comment

jim-bcom · 2026-09-02T15:53:49Z

+
+// private
+
+void ZipBuffer::cleanWorkingDirectory()


No longer useful at all: removed

jim-bcom · 2026-09-02T16:01:02Z

+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * FrameworkReturnCode::_NOT_FOUND if data is not found in original path
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode zipToBuffer(const std::string & originalPath,


nit: use directly fs::path i.o. string for path ?

I thought using std::string avoided forcing the caller to use std::filesystem?

jim-bcom · 2026-09-02T16:03:33Z

+ /// @return
+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode bufferToUnzip(const std::vector<unsigned char> & compressedZipBuffer,


nit: matter of taste, but with verbs for method ? ZipBuffer::extract()/ZipBuffer::compress()? Or something else.

jim-bcom · 2026-09-02T16:07:21Z

+ *
+ */
+
+class ZipBuffer {


Again naming things is hard... It does not per say represent a ZipBuffer, but more like utils functions... But I'm not sure about ZipBufferUtils... I don't have a better idea :). Something like ZipBufferExtractor (but it does also compress, so ...), or ZipBufferProcessor,...

What about just ZipUtils ?

Let's go for ZipBufferUtils class and extractand compressmethods

jim-bcom · 2026-09-03T12:05:57Z

+#include <vector>
+#include <filesystem>
+
+namespace fs = std::filesystem;


Remove, avoid aliasing in header, especially at this scope.

Yep, sorry!

jim-bcom · 2026-09-03T12:14:09Z

+ * @brief <B>Create a temporary working directory</B>
+ *
+ */
+ class ScopedWorkingDir {


This class is decorrelated to ZipBufferUtils, so 2 suggestions:
define in its own file for anyboady to reuse
define it in the cpp file, because it's not referenced here, so it does not have to be defined here.
It's only an implementation detail that does not have to surface here (except if this class is needed elsewhere, but then this would have to be defined in its own file, as suggested in 1.

Also maybe keep Tmp or Temp in the name (ScopedTmpDir), so that we know it uses the tmp folder API, and this is where the file is created ...?

jim-bcom · 2026-09-03T12:24:21Z

+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }
+ ~ScopedWorkingDir() { fs::remove_all(m_workingPath); }


Delete copy operations to prevent double deletion
ScopedWorkingDir(const ScopedWorkingDir&) = delete; ScopedWorkingDir& operator=(const ScopedWorkingDir&) = delete; ScopedWorkingDir(ScopedWorkingDir&&) = delete; ScopedWorkingDir& operator=(ScopedWorkingDir&&) = delete;
(move operations are already deleted by the fact of defining the destructor, but this way it's explicit)

jim-bcom · 2026-09-03T12:30:14Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


Make the subdir a (optional?) parameter of the (explicit) ctor?
explicitScopedWorkingDir(const std::string& subDir = "");

This class has moved to the .cpp file, and will only be used by ZipBufferUtils static methods, so I don't think it's usefull to make the sudir configurable

jim-bcom · 2026-09-03T12:32:10Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


nit: use operator / for path separator, example of syntax:
m_workingPath = m_workingPath / "solar"; m_workingPath /= "solar";

Both operations are available : why this one instead of the other one?

OK, changed

jim-bcom · 2026-09-03T12:58:38Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

jim-bcom · 2026-09-03T13:01:08Z

+ LOG_DEBUG("ZipBufferUtils::extract - Working temporary path: {}", workingDir.getStringPath());
+
+ // Check/create the working directory
+ if (!fs::exists(workingDir.getPath())) {


No longer needed?

Yes, still needed here: the subdir must be created here before creating the zip file (tested)

jim-bcom · 2026-09-03T13:03:19Z

+ }
+
+ try {
+ fs::path dp(destinationPath);


In the other function you clear the output buffer, here we could delete the content of the destination path as well ?

I don't think so: this destination directory can contain other files, we don't know.

jim-bcom · 2026-09-03T13:04:24Z

+ return FrameworkReturnCode::_ERROR_;
+ }
+
+ // Delete the zip file


No longer needed ?

Ah yes you do it because the zip is in the same dir as the one you copy later on, OK.

jim-bcom · 2026-09-03T13:06:30Z

+ file.close();
+
+ // Try to unzip the file content
+ std::string command = "cd " + workingDir.getStringPath() + "; unzip data.zip";


Here you could avoid the copy by cd in the dest dir, there's really no risk of messing with existing files, since it's supposed to receive ours, no?

Yes, I think you're right and I will change it

cutullic · 2026-09-02T15:07:55Z

No description provided.

jim-bcom · 2026-09-02T15:28:51Z

+ try {
+ // Create a working directory for zip/unzip features
+ fs::path wp(m_workingPath);
+ if (!fs::exists(wp)) {


Use temp_directory_pathhttps://en.cppreference.com/cpp/filesystem/temp_directory_path ?
This way you are guaranteed to get an existing directory, and it might enventually be removed when the machine restarts, if for some reason you were not able to delete it sooner.
Here, if the directory does exists, you might use it, and potentially overwrite user data, since it's not handled as an error.

Now that I think of it, concerning the two PR comment this seems to address: you might want to split the RAII part you did with the dtor of ZipBuffer, from the utility methods.
Make ZipBufferUtils (or whatever :)) have these 2 methods of extraction/compression as static helpers
ZipBufferUtils::extract() ZipBufferUtils::create/compress/...()
create a RAII object responsible to create the tmp dir and deleting it on destruction, something like:
classScopedTmpDir { ScopedTmpDir () { m_path = temp_directory_path();} ~ScopedTmpDir { fs::remove_all(m_path ); } fs::path getPath(); };
And use it in the static helper function.
This adds the benefit of deleting things ASAP, instead of waiting that the ZipBuffer object goes out of scope, which may be long after the end of the function (which I gues is why you kept deleting things at the end of the methods, instead of relying on the destructor, as I commented elsewhere).
What do you think of this approach ?

I like this approach :-) Made the modifications.

jim-bcom · 2026-09-02T15:32:44Z

+ try {
+ // Check working directory
+ fs::path wp(m_workingPath);
+ if (!fs::is_directory(wp)) {


If temp_directory_path this check is probably not needed (even in the current version). Unless we fear something might happen to this directory between the creation of ZipBuffer and the call to this.

Removed because temp_directory_path is now used.

jim-bcom · 2026-09-02T15:34:30Z

+ fs::path op(originalPath);
+ if (!fs::is_directory(op)) {
+ LOG_ERROR("The original path is not a directory: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


Maybe !exists => NOT_FOUND but !directory => ERROR (if we don't have BAD_ARGUMENT) ?

jim-bcom · 2026-09-02T15:35:45Z

+ }
+ if (fs::is_empty(op)) {
+ LOG_ERROR("The original path is empty: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


SUCCESS? You give an empty dir, you get an empty vector, job done ?

OK returns SUCCESS but with a warning (and clear the input buffer)

jim-bcom · 2026-09-02T15:42:05Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Can't we avoid the copy ? For example by invoking zip directly from originalPath ?
std::string command = "cd " + originalPath + ";zip -r " + m_workingPath+ "/data.zip .";

No, I prefer not to work on the original directory, to ensure I don't modify it.

Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

OK, I see the point. I will try to change this behavior.

jim-bcom · 2026-09-02T15:53:33Z

+ fs::remove(zipFile);
+
+ // Copy unzipped data in the destination directory
+ const auto copyOptions = fs::copy_options::recursive;


Could we avoid the copy by invoking zip from the destination directory?
std::string command = "cd " + destinationPath+ "; unzip " + m_workingPath + " /data.zip";

See previous comment

jim-bcom · 2026-09-02T15:53:49Z

+
+// private
+
+void ZipBuffer::cleanWorkingDirectory()


No longer useful at all: removed

jim-bcom · 2026-09-02T16:01:02Z

+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * FrameworkReturnCode::_NOT_FOUND if data is not found in original path
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode zipToBuffer(const std::string & originalPath,


nit: use directly fs::path i.o. string for path ?

I thought using std::string avoided forcing the caller to use std::filesystem?

jim-bcom · 2026-09-02T16:03:33Z

+ /// @return
+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode bufferToUnzip(const std::vector<unsigned char> & compressedZipBuffer,


nit: matter of taste, but with verbs for method ? ZipBuffer::extract()/ZipBuffer::compress()? Or something else.

jim-bcom · 2026-09-02T16:07:21Z

+ *
+ */
+
+class ZipBuffer {


Again naming things is hard... It does not per say represent a ZipBuffer, but more like utils functions... But I'm not sure about ZipBufferUtils... I don't have a better idea :). Something like ZipBufferExtractor (but it does also compress, so ...), or ZipBufferProcessor,...

What about just ZipUtils ?

Let's go for ZipBufferUtils class and extractand compressmethods

jim-bcom · 2026-09-03T12:05:57Z

+#include <vector>
+#include <filesystem>
+
+namespace fs = std::filesystem;


Remove, avoid aliasing in header, especially at this scope.

Yep, sorry!

jim-bcom · 2026-09-03T12:14:09Z

+ * @brief <B>Create a temporary working directory</B>
+ *
+ */
+ class ScopedWorkingDir {


This class is decorrelated to ZipBufferUtils, so 2 suggestions:
define in its own file for anyboady to reuse
define it in the cpp file, because it's not referenced here, so it does not have to be defined here.
It's only an implementation detail that does not have to surface here (except if this class is needed elsewhere, but then this would have to be defined in its own file, as suggested in 1.

Also maybe keep Tmp or Temp in the name (ScopedTmpDir), so that we know it uses the tmp folder API, and this is where the file is created ...?

jim-bcom · 2026-09-03T12:24:21Z

+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }
+ ~ScopedWorkingDir() { fs::remove_all(m_workingPath); }


Delete copy operations to prevent double deletion
ScopedWorkingDir(const ScopedWorkingDir&) = delete; ScopedWorkingDir& operator=(const ScopedWorkingDir&) = delete; ScopedWorkingDir(ScopedWorkingDir&&) = delete; ScopedWorkingDir& operator=(ScopedWorkingDir&&) = delete;
(move operations are already deleted by the fact of defining the destructor, but this way it's explicit)

jim-bcom · 2026-09-03T12:30:14Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


Make the subdir a (optional?) parameter of the (explicit) ctor?
explicitScopedWorkingDir(const std::string& subDir = "");

This class has moved to the .cpp file, and will only be used by ZipBufferUtils static methods, so I don't think it's usefull to make the sudir configurable

jim-bcom · 2026-09-03T12:32:10Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


nit: use operator / for path separator, example of syntax:
m_workingPath = m_workingPath / "solar"; m_workingPath /= "solar";

Both operations are available : why this one instead of the other one?

OK, changed

jim-bcom · 2026-09-03T12:58:38Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

jim-bcom · 2026-09-03T13:01:08Z

+ LOG_DEBUG("ZipBufferUtils::extract - Working temporary path: {}", workingDir.getStringPath());
+
+ // Check/create the working directory
+ if (!fs::exists(workingDir.getPath())) {


No longer needed?

Yes, still needed here: the subdir must be created here before creating the zip file (tested)

jim-bcom · 2026-09-03T13:03:19Z

+ }
+
+ try {
+ fs::path dp(destinationPath);


In the other function you clear the output buffer, here we could delete the content of the destination path as well ?

I don't think so: this destination directory can contain other files, we don't know.

jim-bcom · 2026-09-03T13:04:24Z

+ return FrameworkReturnCode::_ERROR_;
+ }
+
+ // Delete the zip file


No longer needed ?

Ah yes you do it because the zip is in the same dir as the one you copy later on, OK.

jim-bcom · 2026-09-03T13:06:30Z

+ file.close();
+
+ // Try to unzip the file content
+ std::string command = "cd " + workingDir.getStringPath() + "; unzip data.zip";


Here you could avoid the copy by cd in the dest dir, there's really no risk of messing with existing files, since it's supposed to receive ours, no?

Yes, I think you're right and I will change it

cutullic · 2026-09-02T15:07:55Z

No description provided.

jim-bcom · 2026-09-02T15:28:51Z

+ try {
+ // Create a working directory for zip/unzip features
+ fs::path wp(m_workingPath);
+ if (!fs::exists(wp)) {


Use temp_directory_pathhttps://en.cppreference.com/cpp/filesystem/temp_directory_path ?
This way you are guaranteed to get an existing directory, and it might enventually be removed when the machine restarts, if for some reason you were not able to delete it sooner.
Here, if the directory does exists, you might use it, and potentially overwrite user data, since it's not handled as an error.

Now that I think of it, concerning the two PR comment this seems to address: you might want to split the RAII part you did with the dtor of ZipBuffer, from the utility methods.
Make ZipBufferUtils (or whatever :)) have these 2 methods of extraction/compression as static helpers
ZipBufferUtils::extract() ZipBufferUtils::create/compress/...()
create a RAII object responsible to create the tmp dir and deleting it on destruction, something like:
classScopedTmpDir { ScopedTmpDir () { m_path = temp_directory_path();} ~ScopedTmpDir { fs::remove_all(m_path ); } fs::path getPath(); };
And use it in the static helper function.
This adds the benefit of deleting things ASAP, instead of waiting that the ZipBuffer object goes out of scope, which may be long after the end of the function (which I gues is why you kept deleting things at the end of the methods, instead of relying on the destructor, as I commented elsewhere).
What do you think of this approach ?

I like this approach :-) Made the modifications.

jim-bcom · 2026-09-02T15:32:44Z

+ try {
+ // Check working directory
+ fs::path wp(m_workingPath);
+ if (!fs::is_directory(wp)) {


If temp_directory_path this check is probably not needed (even in the current version). Unless we fear something might happen to this directory between the creation of ZipBuffer and the call to this.

Removed because temp_directory_path is now used.

jim-bcom · 2026-09-02T15:34:30Z

+ fs::path op(originalPath);
+ if (!fs::is_directory(op)) {
+ LOG_ERROR("The original path is not a directory: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


Maybe !exists => NOT_FOUND but !directory => ERROR (if we don't have BAD_ARGUMENT) ?

jim-bcom · 2026-09-02T15:35:45Z

+ }
+ if (fs::is_empty(op)) {
+ LOG_ERROR("The original path is empty: {}", originalPath);
+ return FrameworkReturnCode::_NOT_FOUND;


SUCCESS? You give an empty dir, you get an empty vector, job done ?

OK returns SUCCESS but with a warning (and clear the input buffer)

jim-bcom · 2026-09-02T15:42:05Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Can't we avoid the copy ? For example by invoking zip directly from originalPath ?
std::string command = "cd " + originalPath + ";zip -r " + m_workingPath+ "/data.zip .";

No, I prefer not to work on the original directory, to ensure I don't modify it.

Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

OK, I see the point. I will try to change this behavior.

jim-bcom · 2026-09-02T15:53:33Z

+ fs::remove(zipFile);
+
+ // Copy unzipped data in the destination directory
+ const auto copyOptions = fs::copy_options::recursive;


Could we avoid the copy by invoking zip from the destination directory?
std::string command = "cd " + destinationPath+ "; unzip " + m_workingPath + " /data.zip";

See previous comment

jim-bcom · 2026-09-02T15:53:49Z

+
+// private
+
+void ZipBuffer::cleanWorkingDirectory()


No longer useful at all: removed

jim-bcom · 2026-09-02T16:01:02Z

+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * FrameworkReturnCode::_NOT_FOUND if data is not found in original path
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode zipToBuffer(const std::string & originalPath,


nit: use directly fs::path i.o. string for path ?

I thought using std::string avoided forcing the caller to use std::filesystem?

jim-bcom · 2026-09-02T16:03:33Z

+ /// @return
+ /// * FrameworkReturnCode::_SUCCESS if the process succeeds
+ /// * else FrameworkReturnCode::_ERROR_
+ FrameworkReturnCode bufferToUnzip(const std::vector<unsigned char> & compressedZipBuffer,


nit: matter of taste, but with verbs for method ? ZipBuffer::extract()/ZipBuffer::compress()? Or something else.

jim-bcom · 2026-09-02T16:07:21Z

+ *
+ */
+
+class ZipBuffer {


Again naming things is hard... It does not per say represent a ZipBuffer, but more like utils functions... But I'm not sure about ZipBufferUtils... I don't have a better idea :). Something like ZipBufferExtractor (but it does also compress, so ...), or ZipBufferProcessor,...

What about just ZipUtils ?

Let's go for ZipBufferUtils class and extractand compressmethods

jim-bcom · 2026-09-03T12:05:57Z

+#include <vector>
+#include <filesystem>
+
+namespace fs = std::filesystem;


Remove, avoid aliasing in header, especially at this scope.

Yep, sorry!

jim-bcom · 2026-09-03T12:14:09Z

+ * @brief <B>Create a temporary working directory</B>
+ *
+ */
+ class ScopedWorkingDir {


This class is decorrelated to ZipBufferUtils, so 2 suggestions:
define in its own file for anyboady to reuse
define it in the cpp file, because it's not referenced here, so it does not have to be defined here.
It's only an implementation detail that does not have to surface here (except if this class is needed elsewhere, but then this would have to be defined in its own file, as suggested in 1.

Also maybe keep Tmp or Temp in the name (ScopedTmpDir), so that we know it uses the tmp folder API, and this is where the file is created ...?

jim-bcom · 2026-09-03T12:24:21Z

+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }
+ ~ScopedWorkingDir() { fs::remove_all(m_workingPath); }


Delete copy operations to prevent double deletion
ScopedWorkingDir(const ScopedWorkingDir&) = delete; ScopedWorkingDir& operator=(const ScopedWorkingDir&) = delete; ScopedWorkingDir(ScopedWorkingDir&&) = delete; ScopedWorkingDir& operator=(ScopedWorkingDir&&) = delete;
(move operations are already deleted by the fact of defining the destructor, but this way it's explicit)

jim-bcom · 2026-09-03T12:30:14Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


Make the subdir a (optional?) parameter of the (explicit) ctor?
explicitScopedWorkingDir(const std::string& subDir = "");

This class has moved to the .cpp file, and will only be used by ZipBufferUtils static methods, so I don't think it's usefull to make the sudir configurable

jim-bcom · 2026-09-03T12:32:10Z

+ */
+ class ScopedWorkingDir {
+ public:
+ ScopedWorkingDir() { m_workingPath = fs::temp_directory_path(); m_workingPath += "/solar"; }


nit: use operator / for path separator, example of syntax:
m_workingPath = m_workingPath / "solar"; m_workingPath /= "solar";

Both operations are available : why this one instead of the other one?

OK, changed

jim-bcom · 2026-09-03T12:58:38Z

+
+ // Copy data to zip in the working directory
+ const auto copyOptions = fs::copy_options::recursive;
+ fs::copy(op, wp, copyOptions);


Ok, but I'm not sure I see what you fear of invoking zip, if you specify a different destination dit for the zip file, it shouldn't touch the origin one.
It's just that it might be costly to copy a large amount of data. The current implementation is already not ideal performance wise (using a temporary zip file on disk i.o. zipping it in memory, which produces a lot of I/O), and this adds up.
OK we'll see if it becomes a bottleneck.

jim-bcom · 2026-09-03T13:01:08Z

+ LOG_DEBUG("ZipBufferUtils::extract - Working temporary path: {}", workingDir.getStringPath());
+
+ // Check/create the working directory
+ if (!fs::exists(workingDir.getPath())) {


No longer needed?

Yes, still needed here: the subdir must be created here before creating the zip file (tested)

jim-bcom · 2026-09-03T13:03:19Z

+ }
+
+ try {
+ fs::path dp(destinationPath);


In the other function you clear the output buffer, here we could delete the content of the destination path as well ?

I don't think so: this destination directory can contain other files, we don't know.

jim-bcom · 2026-09-03T13:04:24Z

+ return FrameworkReturnCode::_ERROR_;
+ }
+
+ // Delete the zip file


No longer needed ?

Ah yes you do it because the zip is in the same dir as the one you copy later on, OK.

jim-bcom · 2026-09-03T13:06:30Z

+ file.close();
+
+ // Try to unzip the file content
+ std::string command = "cd " + workingDir.getStringPath() + "; unzip data.zip";


Here you could avoid the copy by cd in the dest dir, there's really no risk of messing with existing files, since it's supposed to receive ours, no?

Yes, I think you're right and I will change it

cutullic added 2 commits September 2, 2026 14:53

feat: add new class ZipBuffer to manage zip/unzip data to/from buffer
a8470a8

feat: ZipBuffer methods return FrameworkReturnCode instead of boolean
1fbeac7

cutullic requested a review from jim-bcom September 2, 2026 15:07

jim-bcom reviewed Sep 2, 2026
View reviewed changes

refactor: changes following PR comments
d8c8c46

jim-bcom reviewed Sep 3, 2026
View reviewed changes

refactor: changes following PR comments
8a50ced

cutullic added 2 commits September 2, 2026 14:53

feat: add new class ZipBuffer to manage zip/unzip data to/from buffer
a8470a8

feat: ZipBuffer methods return FrameworkReturnCode instead of boolean
1fbeac7

cutullic requested a review from jim-bcom September 2, 2026 15:07

jim-bcom reviewed Sep 2, 2026
View reviewed changes

refactor: changes following PR comments
d8c8c46

jim-bcom reviewed Sep 3, 2026
View reviewed changes

refactor: changes following PR comments
8a50ced

cutullic added 2 commits September 2, 2026 14:53

feat: add new class ZipBuffer to manage zip/unzip data to/from buffer
a8470a8

feat: ZipBuffer methods return FrameworkReturnCode instead of boolean
1fbeac7

cutullic requested a review from jim-bcom September 2, 2026 15:07

jim-bcom reviewed Sep 2, 2026
View reviewed changes

refactor: changes following PR comments
d8c8c46

jim-bcom reviewed Sep 3, 2026
View reviewed changes

refactor: changes following PR comments
8a50ced

cutullic added 2 commits September 2, 2026 14:53

feat: add new class ZipBuffer to manage zip/unzip data to/from buffer
a8470a8

feat: ZipBuffer methods return FrameworkReturnCode instead of boolean
1fbeac7

cutullic requested a review from jim-bcom September 2, 2026 15:07

jim-bcom reviewed Sep 2, 2026
View reviewed changes

refactor: changes following PR comments
d8c8c46

jim-bcom reviewed Sep 3, 2026
View reviewed changes

refactor: changes following PR comments
8a50ced

cutullic added 2 commits September 2, 2026 14:53

feat: add new class ZipBuffer to manage zip/unzip data to/from buffer
a8470a8

feat: ZipBuffer methods return FrameworkReturnCode instead of boolean
1fbeac7

cutullic requested a review from jim-bcom September 2, 2026 15:07

jim-bcom reviewed Sep 2, 2026
View reviewed changes

refactor: changes following PR comments
d8c8c46

jim-bcom reviewed Sep 3, 2026
View reviewed changes

refactor: changes following PR comments
8a50ced

cutullic added 2 commits September 2, 2026 14:53

feat: add new class ZipBuffer to manage zip/unzip data to/from buffer
a8470a8

feat: ZipBuffer methods return FrameworkReturnCode instead of boolean
1fbeac7

cutullic requested a review from jim-bcom September 2, 2026 15:07

jim-bcom reviewed Sep 2, 2026
View reviewed changes

refactor: changes following PR comments
d8c8c46

jim-bcom reviewed Sep 3, 2026
View reviewed changes

refactor: changes following PR comments
8a50ced

cutullic added 2 commits September 2, 2026 14:53

feat: add new class ZipBuffer to manage zip/unzip data to/from buffer
a8470a8

feat: ZipBuffer methods return FrameworkReturnCode instead of boolean
1fbeac7

cutullic requested a review from jim-bcom September 2, 2026 15:07

jim-bcom reviewed Sep 2, 2026
View reviewed changes

refactor: changes following PR comments
d8c8c46

jim-bcom reviewed Sep 3, 2026
View reviewed changes

refactor: changes following PR comments
8a50ced

cutullic added 2 commits September 2, 2026 14:53

feat: add new class ZipBuffer to manage zip/unzip data to/from buffer
a8470a8

feat: ZipBuffer methods return FrameworkReturnCode instead of boolean
1fbeac7

cutullic requested a review from jim-bcom September 2, 2026 15:07

jim-bcom reviewed Sep 2, 2026
View reviewed changes

refactor: changes following PR comments
d8c8c46

jim-bcom reviewed Sep 3, 2026
View reviewed changes

refactor: changes following PR comments
8a50ced

Uh oh!

Conversation

cutullic commented Sep 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!