diff --git a/CMakeLists.txt b/CMakeLists.txt index 13aa01dc..a9bf5ec3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.15) cmake_policy(SET CMP0091 NEW) # enable new "MSVC runtime library selection" (https://cmake.org/cmake/help/latest/variable/CMAKE_MSVC_RUNTIME_LIBRARY.html) project(libCZI - VERSION 0.53.0 + VERSION 0.53.1 HOMEPAGE_URL "https://github.com/ZEISS/libczi" DESCRIPTION "libCZI is an Open Source Cross-Platform C++ library to read and write CZI") diff --git a/Src/libCZI/CziAttachmentsDirectory.cpp b/Src/libCZI/CziAttachmentsDirectory.cpp index ac759e43..fc8c55fc 100644 --- a/Src/libCZI/CziAttachmentsDirectory.cpp +++ b/Src/libCZI/CziAttachmentsDirectory.cpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: LGPL-3.0-or-later #include "CziAttachmentsDirectory.h" +#include #include /*static*/bool CCziAttachmentsDirectoryBase::CompareForEquality_Id(const AttachmentEntry& a, const AttachmentEntry& b) @@ -62,14 +63,22 @@ bool CCziAttachmentsDirectory::TryGetAttachment(int index, AttachmentEntry& entr bool CWriterCziAttachmentsDirectory::TryAddAttachment(const AttachmentEntry& entry) { - auto insert = this->attachments.insert(entry); - return insert.second; + if (std::find_if( + this->attachments_.cbegin(), + this->attachments_.cend(), + [&entry](const AttachmentEntry& x)->bool { return CCziAttachmentsDirectoryBase::CompareForEquality_Id(x, entry); }) != this->attachments_.cend()) + { + return false; + } + + this->attachments_.push_back(entry); + return true; } bool CWriterCziAttachmentsDirectory::EnumEntries(const std::function& func) const { int index = 0; - for (auto it = this->attachments.cbegin(); it != this->attachments.cend(); ++it) + for (auto it = this->attachments_.cbegin(); it != this->attachments_.cend(); ++it) { if (!func(index++, *it)) { @@ -82,39 +91,7 @@ bool CWriterCziAttachmentsDirectory::EnumEntries(const std::functionattachments.size(); -} - -bool CWriterCziAttachmentsDirectory::AttachmentEntriesCompare::operator()(const AttachmentEntry& a, const AttachmentEntry& b) const -{ - // we shall return true if a is considered to go before b in the strict weak ordering the function defines - int r = a.ContentGuid.compare(b.ContentGuid); - if (r < 0) - { - return true; - } - else if (r > 0) - { - return false; - } - - r = memcmp(&a.ContentFileType, &b.ContentFileType, sizeof(a.ContentFileType)); - if (r < 0) - { - return true; - } - else if (r > 0) - { - return false; - } - - r = memcmp(&a.Name, &b.Name, sizeof(a.Name)); - if (r < 0) - { - return true; - } - - return false; + return (int)this->attachments_.size(); } //----------------------------------------------------------------------------- @@ -199,12 +176,12 @@ bool CReaderWriterCziAttachmentsDirectory::TryRemoveAttachment(int key, Attachme bool CReaderWriterCziAttachmentsDirectory::TryAddAttachment(const AttachmentEntry& entry, int* key) { - for (auto it = this->attchmnts.cbegin(); it != this->attchmnts.cend(); ++it) + if (std::find_if( + this->attchmnts.cbegin(), + this->attchmnts.cend(), + [&entry](const std::pair& x)->bool { return CCziAttachmentsDirectoryBase::CompareForEquality_Id(x.second, entry); }) != this->attchmnts.cend()) { - if (CCziAttachmentsDirectoryBase::CompareForEquality_Id(it->second, entry)) - { - return false; - } + return false; } this->AddAttachment(entry, key); diff --git a/Src/libCZI/CziAttachmentsDirectory.h b/Src/libCZI/CziAttachmentsDirectory.h index e8de9ad7..df0571f9 100644 --- a/Src/libCZI/CziAttachmentsDirectory.h +++ b/Src/libCZI/CziAttachmentsDirectory.h @@ -25,6 +25,13 @@ class CCziAttachmentsDirectoryBase std::int64_t allocatedSize; }; + /// Compare the properties which make up the identity of an AttachmentEntry, + /// i.e. the GUID, the name and the content file type, for equality. + /// + /// \param a The first AttachmentEntry to process. + /// \param b The second AttachmentEntry to process. + /// + /// \returns True if the identifier-properties compare as equal; false otherwise. static bool CompareForEquality_Id(const AttachmentEntry& a, const AttachmentEntry& b); }; @@ -51,11 +58,7 @@ class CWriterCziAttachmentsDirectory : public CCziAttachmentsDirectoryBase int GetAttachmentCount() const; bool EnumEntries(const std::function& func) const; private: - struct AttachmentEntriesCompare { - bool operator() (const AttachmentEntry& a, const AttachmentEntry& b) const; - }; - - std::set attachments; + std::vector attachments_; }; class CReaderWriterCziAttachmentsDirectory : public CCziAttachmentsDirectoryBase diff --git a/Src/libCZI/CziWriter.cpp b/Src/libCZI/CziWriter.cpp index 8b9949aa..dbd67d1b 100644 --- a/Src/libCZI/CziWriter.cpp +++ b/Src/libCZI/CziWriter.cpp @@ -1160,7 +1160,7 @@ void CCziWriter::WriteAttachmentDirectory() } } -std::tuple CCziWriter::WriteCurrentAttachmentsDirectory() +std::tuple CCziWriter::WriteCurrentAttachmentsDirectory() { CWriterUtils::AttachmentDirWriteInfo info; if (this->attachmentDirectorySegment.IsValid()) diff --git a/Src/libCZI_UnitTests/test_readerwriter.cpp b/Src/libCZI_UnitTests/test_readerwriter.cpp index 94c66948..9e2c9414 100644 --- a/Src/libCZI_UnitTests/test_readerwriter.cpp +++ b/Src/libCZI_UnitTests/test_readerwriter.cpp @@ -1352,3 +1352,38 @@ INSTANTIATE_TEST_SUITE_P( SubBlockPyramidType::None, SubBlockPyramidType::SingleSubBlock, SubBlockPyramidType::MultiSubBlock)); + +TEST(CziReaderWriter, TryAddingDuplicateAttachmentToCziReaderWriterAndExpectError) +{ + const auto in_out_stream = make_shared(0); + const auto reader_writer = CreateCZIReaderWriter(); + reader_writer->Create(in_out_stream); + + constexpr uint8_t data[] = { 1,2,3 }; + + AddAttachmentInfo add_attachment_info; + add_attachment_info.contentGuid = GUID{ 1, 2, 3, {4, 5, 6, 7, 8, 9, 10, 11} }; + add_attachment_info.SetContentFileType("ABC"); + add_attachment_info.SetName("Test"); + add_attachment_info.ptrData = data; + add_attachment_info.dataSize = sizeof(data); + reader_writer->SyncAddAttachment(add_attachment_info); + + // now, try to add it a second time + EXPECT_THROW(reader_writer->SyncAddAttachment(add_attachment_info), LibCZIException); + + // a duplicate attachment means - same Guid, same name, same content-type, + // so now let's try to add it with a different name (which should work) + add_attachment_info.SetName("Test2"); + reader_writer->SyncAddAttachment(add_attachment_info); + + // now with a different content-type (which should also work) + add_attachment_info.SetName("Test"); + add_attachment_info.SetContentFileType("ABCD"); + reader_writer->SyncAddAttachment(add_attachment_info); + + // and finally with a different GUID + add_attachment_info.SetContentFileType("ABC"); + add_attachment_info.contentGuid = GUID{ 111, 2, 3, {4, 5, 6, 7, 8, 9, 10, 11} }; + reader_writer->SyncAddAttachment(add_attachment_info); +} diff --git a/Src/libCZI_UnitTests/test_writer.cpp b/Src/libCZI_UnitTests/test_writer.cpp index 0831c33b..cbd321e8 100644 --- a/Src/libCZI_UnitTests/test_writer.cpp +++ b/Src/libCZI_UnitTests/test_writer.cpp @@ -2121,3 +2121,26 @@ TEST(CziWriter, WriteReadCompressedZStd1ImageBrg48LowPacking) constexpr PixelType pixelType = PixelType::Bgr48; _testWriteReadCompressedImageZStd1LowPacking(61, 61, pixelType, 2, true); } + +TEST(CziWriter, TryAddingDuplicateAttachmentToCziWriterAndExpectError) +{ + const auto writer = CreateCZIWriter(); + const auto output_stream = make_shared(0); + + const auto czi_writer_info = std::make_shared(GUID{ 0, 0, 0, {0, 0, 0, 0, 0, 0, 0, 0} }); + + writer->Create(output_stream, czi_writer_info); + + constexpr uint8_t data[] = { 1,2,3 }; + + AddAttachmentInfo add_attachment_info; + add_attachment_info.contentGuid = GUID{ 1, 2, 3, {4, 5, 6, 7, 8, 9, 10, 11} }; + add_attachment_info.SetContentFileType("ABC"); + add_attachment_info.SetName("Test"); + add_attachment_info.ptrData = data; + add_attachment_info.dataSize = sizeof(data); + writer->SyncAddAttachment(add_attachment_info); + + // now, try to add it a second time + EXPECT_THROW(writer->SyncAddAttachment(add_attachment_info), LibCZIException); +}