From edf39e00ddffb1f2624bd7e82fa756f1c878eeb9 Mon Sep 17 00:00:00 2001 From: Luis Diaz Date: Mon, 10 Jan 2022 13:06:37 +0100 Subject: [PATCH 01/14] Add new BMP tests --- include/exiv2/bmpimage.hpp | 15 +++------------ src/bmpimage.cpp | 10 +++++----- unitTests/CMakeLists.txt | 1 + unitTests/test_bmpimage.cpp | 21 +++++++++++++++++++++ unitTests/test_pngimage.cpp | 8 ++++++++ 5 files changed, 38 insertions(+), 17 deletions(-) create mode 100644 unitTests/test_bmpimage.cpp diff --git a/include/exiv2/bmpimage.hpp b/include/exiv2/bmpimage.hpp index 2dd03010..abd0ccf5 100644 --- a/include/exiv2/bmpimage.hpp +++ b/include/exiv2/bmpimage.hpp @@ -84,20 +84,11 @@ namespace Exiv2 { yet(?) implemented. Calling it will throw an Error(kerWritingImageFormatUnsupported). */ void writeMetadata() override; - /*! - @brief Todo: Not supported yet(?). Calling this function will throw - an instance of Error(kerInvalidSettingForImage). - */ + void setExifData(const ExifData& exifData) override; - /*! - @brief Todo: Not supported yet(?). Calling this function will throw - an instance of Error(kerInvalidSettingForImage). - */ + void setIptcData(const IptcData& iptcData) override; - /*! - @brief Not supported. Calling this function will throw an instance - of Error(kerInvalidSettingForImage). - */ + void setComment(const std::string& comment) override; //@} diff --git a/src/bmpimage.cpp b/src/bmpimage.cpp index 8c2f5f09..1aff6991 100644 --- a/src/bmpimage.cpp +++ b/src/bmpimage.cpp @@ -47,24 +47,22 @@ namespace Exiv2 std::string BmpImage::mimeType() const { - return "image/x-ms-bmp"; + // "image/bmp" is a Generic Bitmap + return "image/x-ms-bmp"; // Microsoft Bitmap } void BmpImage::setExifData(const ExifData& /*exifData*/) { - // Todo: implement me! throw(Error(kerInvalidSettingForImage, "Exif metadata", "BMP")); } void BmpImage::setIptcData(const IptcData& /*iptcData*/) { - // Todo: implement me! throw(Error(kerInvalidSettingForImage, "IPTC metadata", "BMP")); } void BmpImage::setComment(const std::string& /*comment*/) { - // not supported throw(Error(kerInvalidSettingForImage, "Image comment", "BMP")); } @@ -77,9 +75,11 @@ namespace Exiv2 throw Error(kerDataSourceOpenFailed, io_->path(), strError()); } IoCloser closer(*io_); + // Ensure that this is the correct image type if (!isBmpType(*io_, false)) { - if (io_->error() || io_->eof()) throw Error(kerFailedToReadImageData); + if (io_->error() || io_->eof()) + throw Error(kerFailedToReadImageData); throw Error(kerNotAnImage, "BMP"); } clearMetadata(); diff --git a/unitTests/CMakeLists.txt b/unitTests/CMakeLists.txt index 1933c922..375f9820 100644 --- a/unitTests/CMakeLists.txt +++ b/unitTests/CMakeLists.txt @@ -2,6 +2,7 @@ find_package(GTest REQUIRED) add_executable(unit_tests mainTestRunner.cpp + test_bmpimage.cpp test_DateValue.cpp test_TimeValue.cpp test_XmpKey.cpp diff --git a/unitTests/test_bmpimage.cpp b/unitTests/test_bmpimage.cpp new file mode 100644 index 00000000..92865de6 --- /dev/null +++ b/unitTests/test_bmpimage.cpp @@ -0,0 +1,21 @@ +#include + +#include + +#include + +using namespace Exiv2; + +TEST(BmpImage, canBeOpenedWithEmptyMemIo) +{ + auto memIo = std::make_unique(); + ASSERT_NO_THROW(BmpImage bmp(std::move(memIo))); +} + +TEST(BmpImage, mimeTypeIsBmp) +{ + auto memIo = std::make_unique(); + BmpImage bmp(std::move(memIo)); + + ASSERT_EQ("image/x-ms-bmp", bmp.mimeType()); +} diff --git a/unitTests/test_pngimage.cpp b/unitTests/test_pngimage.cpp index ccdbb893..51b7c61f 100644 --- a/unitTests/test_pngimage.cpp +++ b/unitTests/test_pngimage.cpp @@ -158,6 +158,14 @@ TEST(PngImage, cannotWriteMetadataToEmptyIo) } } +TEST(PngImage, canWriteMetadataFromCreatedPngImage) +{ + auto memIo = std::make_unique(); + const bool create {true}; + PngImage png(std::move(memIo), create); + ASSERT_NO_THROW(png.writeMetadata()); +} + TEST(PngImage, cannotWriteMetadataToIoWhichCannotBeOpened) { auto memIo = std::make_unique("NonExistingPath.png"); From a94d648d0594ba2877daa7487d3312c969d10c5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Mon, 10 Jan 2022 14:15:20 +0100 Subject: [PATCH 02/14] Rest of tests to have 100% coverage in BMP class --- include/exiv2/image.hpp | 5 +- src/bmpimage.cpp | 2 +- src/image.cpp | 5 +- unitTests/test_bmpimage.cpp | 179 +++++++++++++++++++++++++++++++++++- 4 files changed, 183 insertions(+), 8 deletions(-) diff --git a/include/exiv2/image.hpp b/include/exiv2/image.hpp index 937b4ccc..37b441f3 100644 --- a/include/exiv2/image.hpp +++ b/include/exiv2/image.hpp @@ -368,9 +368,8 @@ namespace Exiv2 { encoded. Initially, it is not set (\em invalidByteOrder). */ ByteOrder byteOrder() const; - /*! - @brief Check if the Image instance is valid. Use after object - construction. + + /*! @brief Check if the Image instance is valid. Use after object construction. @return true if the Image is in a valid state. */ bool good() const; diff --git a/src/bmpimage.cpp b/src/bmpimage.cpp index 1aff6991..d18979a7 100644 --- a/src/bmpimage.cpp +++ b/src/bmpimage.cpp @@ -105,7 +105,7 @@ namespace Exiv2 46 4 bytes color count 50 4 bytes important colors number of "important" colors */ - byte buf[54]; + byte buf[26]; if (io_->read(buf, sizeof(buf)) == sizeof(buf)) { pixelWidth_ = getLong(buf + 18, littleEndian); pixelHeight_ = getLong(buf + 22, littleEndian); diff --git a/src/image.cpp b/src/image.cpp index 8dfa7bb1..7002a99c 100644 --- a/src/image.cpp +++ b/src/image.cpp @@ -741,7 +741,8 @@ namespace Exiv2 { bool Image::good() const { - if (io_->open() != 0) return false; + if (io_->open() != 0) + return false; IoCloser closer(*io_); return ImageFactory::checkType(imageType_, *io_, false); } @@ -809,7 +810,7 @@ namespace Exiv2 { return r->isThisType_(io, advance); } return false; - } // ImageFactory::checkType + } int ImageFactory::getType(const std::string& path) { diff --git a/unitTests/test_bmpimage.cpp b/unitTests/test_bmpimage.cpp index 92865de6..86764fd8 100644 --- a/unitTests/test_bmpimage.cpp +++ b/unitTests/test_bmpimage.cpp @@ -1,8 +1,7 @@ -#include - #include #include +#include using namespace Exiv2; @@ -19,3 +18,179 @@ TEST(BmpImage, mimeTypeIsBmp) ASSERT_EQ("image/x-ms-bmp", bmp.mimeType()); } + +TEST(BmpImage, writeMetadataIsNotImplemented) +{ + auto memIo = std::make_unique(); + BmpImage bmp(std::move(memIo)); + + try { + bmp.writeMetadata(); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerWritingImageFormatUnsupported, e.code()); + ASSERT_STREQ("Writing to BMP images is not supported", e.what()); + } +} + +TEST(BmpImage, setExitDataIsNotImplemented) +{ + auto memIo = std::make_unique(); + BmpImage bmp(std::move(memIo)); + + try { + ExifData data; + bmp.setExifData(data); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerInvalidSettingForImage, e.code()); + ASSERT_STREQ("Setting Exif metadata in BMP images is not supported", e.what()); + } +} + +TEST(BmpImage, setIptcDataIsNotImplemented) +{ + auto memIo = std::make_unique(); + BmpImage bmp(std::move(memIo)); + + try { + IptcData data; + bmp.setIptcData(data); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerInvalidSettingForImage, e.code()); + ASSERT_STREQ("Setting IPTC metadata in BMP images is not supported", e.what()); + } +} + +TEST(BmpImage, setCommentIsNotImplemented) +{ + auto memIo = std::make_unique(); + BmpImage bmp(std::move(memIo)); + + try { + bmp.setComment("random comment"); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerInvalidSettingForImage, e.code()); + ASSERT_STREQ("Setting Image comment in BMP images is not supported", e.what()); + } +} + +TEST(BmpImage, readMetadataReadsImageDimensionsWhenDataIsAvailable) +{ + const std::array header{ + 'B', 'M', // Signature off:0 size:2 + 0x4E, 0x47, 0x0D, 0x0A, // Size of the BMP file in bytes off:2, size:4 + 0x1A, 0x0A, // Reserved off:6, size:2 + 0x00, 0x00, // Reserved off:8, size:2 + 0x00, 0x00, 0x00, 0x00, // Offset of the byte where the bitmap image data can be found off:10, size:4 + 0x00, 0x00, 0x00, 0x00, // Size of this header off:14, size:4 + 0x00, 0x05, 0x00, 0x00, // The bitmap width in pixels (unsigned 16 bit) off:18, size:4 + 0x20, 0x03, 0x00, 0x00, // The bitmap height in pixels (unsigned 16 bit) off:22, size:4 + }; + + auto memIo = std::make_unique(header.data(), header.size()); + BmpImage bmp(std::move(memIo)); + ASSERT_NO_THROW(bmp.readMetadata()); + ASSERT_EQ(1280, bmp.pixelWidth()); + ASSERT_EQ(800, bmp.pixelHeight()); +} + +TEST(BmpImage, readMetadataThrowsWhenImageIsNotBMP) +{ + const std::array header{ + 'B', 'A', // Signature off:0 size:2 + 0x4E, 0x47, 0x0D, 0x0A, // Size of the BMP file in bytes off:2, size:4 + 0x1A, 0x0A, // Reserved off:6, size:2 + 0x00, 0x00, // Reserved off:8, size:2 + 0x00, 0x00, 0x00, 0x00, // Offset of the byte where the bitmap image data can be found off:10, size:4 + 0x00, 0x00, 0x00, 0x00, // Size of this header off:14, size:4 + 0x00, 0x05, 0x00, 0x00, // The bitmap width in pixels (unsigned 16 bit) off:18, size:4 + 0x20, 0x03, 0x00, 0x00, // The bitmap height in pixels (unsigned 16 bit) off:22, size:4 + }; + + auto memIo = std::make_unique(header.data(), header.size()); + BmpImage bmp(std::move(memIo)); + try { + bmp.readMetadata(); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerNotAnImage, e.code()); + ASSERT_STREQ("This does not look like a BMP image", e.what()); + } +} + +TEST(BmpImage, readMetadataThrowsWhenThereIsNotEnoughInfoToRead) +{ + const std::array header{'B'}; + auto memIo = std::make_unique(header.data(), header.size()); + BmpImage bmp(std::move(memIo)); + try { + bmp.readMetadata(); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerFailedToReadImageData, e.code()); + ASSERT_STREQ("Failed to read image data", e.what()); + } +} + + +TEST(BmpImage, readMetadataThrowsWhenIoCannotBeOpened) +{ + auto fileIo = std::make_unique("NonExistingPath.png"); + BmpImage bmp(std::move(fileIo)); + try { + bmp.readMetadata(); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerDataSourceOpenFailed, e.code()); + } +} + +TEST(newBmpInstance, createsValidInstace) +{ + const std::array bitmapHeader{ + 'B', 'M', // Signature + 0x4E, 0x47, 0x0D, 0x0A, // Size of the BMP file in bytes + 0x1A, 0x0A, // Reserved + 0x00, 0x00, // Reserved + 0x00, 0x00, 0x00, 0x00 // Offset of the byte where the bitmap image data can be found + }; + auto memIo = std::make_unique(bitmapHeader.data(), bitmapHeader.size()); + auto img = newBmpInstance(std::move(memIo), false); + ASSERT_TRUE(img->good()); +} + +TEST(newBmpInstance, createsInvalidInstaceWithNonExistingFilePath) +{ + auto fileIo = std::make_unique("NonExistingPath.png"); + auto img = newBmpInstance(std::move(fileIo), false); + ASSERT_FALSE(img); +} + +TEST(isBmpType, withValidSignatureReturnsTrue) +{ + const std::array bitmapHeader{ + 'B', 'M', // Signature + 0x4E, 0x47, 0x0D, 0x0A, // Size of the BMP file in bytes + 0x1A, 0x0A, // Reserved + 0x00, 0x00, // Reserved + 0x00, 0x00, 0x00, 0x00 // Offset of the byte where the bitmap image data can be found + }; + MemIo memIo(bitmapHeader.data(), bitmapHeader.size()); + ASSERT_TRUE(isBmpType(memIo, false)); +} + +TEST(isBmpType, withInvalidSignatureReturnsFalse) +{ + const std::array bitmapHeader{ + 'B', 'A', // Signature + 0x4E, 0x47, 0x0D, 0x0A, // Size of the BMP file in bytes + 0x1A, 0x0A, // Reserved + 0x00, 0x00, // Reserved + 0x00, 0x00, 0x00, 0x00 // Offset of the byte where the bitmap image data can be found + }; + MemIo memIo(bitmapHeader.data(), bitmapHeader.size()); + ASSERT_FALSE(isBmpType(memIo, false)); +} From 071e73fa4d7605fe86b30f622be8ce32bee1f161 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Mon, 10 Jan 2022 17:53:16 +0100 Subject: [PATCH 03/14] Add many tests for datasets --- include/exiv2/datasets.hpp | 2 + include/exiv2/iptc.hpp | 5 +- src/datasets.cpp | 35 +++++--- unitTests/CMakeLists.txt | 1 + unitTests/test_datasets.cpp | 163 ++++++++++++++++++++++++++++++++++++ 5 files changed, 192 insertions(+), 14 deletions(-) create mode 100644 unitTests/test_datasets.cpp diff --git a/include/exiv2/datasets.hpp b/include/exiv2/datasets.hpp index 17916d54..5d98ef5a 100644 --- a/include/exiv2/datasets.hpp +++ b/include/exiv2/datasets.hpp @@ -97,6 +97,7 @@ namespace Exiv2 { static constexpr uint16_t UNO = 100; static constexpr uint16_t ARMId = 120; static constexpr uint16_t ARMVersion = 122; + static constexpr uint16_t RecordVersion = 0; static constexpr uint16_t ObjectType = 3; static constexpr uint16_t ObjectAttribute = 4; @@ -212,6 +213,7 @@ namespace Exiv2 { @throw Error if the \em dataSetName or \em recordId are invalid */ static uint16_t dataSet(const std::string& dataSetName, uint16_t recordId); + //! Return the type for dataSet number and Record id static TypeId dataSetType(uint16_t number, uint16_t recordId); /*! diff --git a/include/exiv2/iptc.hpp b/include/exiv2/iptc.hpp index 2b5aaf29..23f8a66f 100644 --- a/include/exiv2/iptc.hpp +++ b/include/exiv2/iptc.hpp @@ -45,6 +45,8 @@ namespace Exiv2 { /*! @brief An IPTC metadatum ("dataset"), consisting of an IptcKey and a Value and methods to manipulate these. + + This is referred in the standard as a property. */ class EXIV2API Iptcdatum : public Metadatum { public: @@ -156,8 +158,7 @@ namespace Exiv2 { typedef std::vector IptcMetadata; /*! - @brief A container for IPTC data. This is a top-level class of - the %Exiv2 library. + @brief A container for IPTC data. This is a top-level class of the %Exiv2 library. Provide high-level access to the IPTC data of an image: - read IPTC information from JPEG files diff --git a/src/datasets.cpp b/src/datasets.cpp index 9cd62df5..ac63df6a 100644 --- a/src/datasets.cpp +++ b/src/datasets.cpp @@ -424,26 +424,30 @@ namespace Exiv2 { int IptcDataSets::dataSetIdx(uint16_t number, uint16_t recordId) { - if( recordId != envelope && recordId != application2 ) return -1; + if( recordId != envelope && recordId != application2 ) + return -1; const DataSet* dataSet = records_[recordId]; if (dataSet == nullptr) return -1; int idx; for (idx = 0; dataSet[idx].number_ != number; ++idx) { - if (dataSet[idx].number_ == 0xffff) return -1; + if (dataSet[idx].number_ == 0xffff) + return -1; } return idx; } int IptcDataSets::dataSetIdx(const std::string& dataSetName, uint16_t recordId) { - if( recordId != envelope && recordId != application2 ) return -1; + if( recordId != envelope && recordId != application2 ) + return -1; const DataSet* dataSet = records_[recordId]; if (dataSet == nullptr) return -1; int idx; for (idx = 0; dataSet[idx].name_ != dataSetName; ++idx) { - if (dataSet[idx].number_ == 0xffff) return -1; + if (dataSet[idx].number_ == 0xffff) + return -1; } return idx; } @@ -451,14 +455,16 @@ namespace Exiv2 { TypeId IptcDataSets::dataSetType(uint16_t number, uint16_t recordId) { int idx = dataSetIdx(number, recordId); - if (idx == -1) return unknownDataSet.type_; + if (idx == -1) + return unknownDataSet.type_; return records_[recordId][idx].type_; } std::string IptcDataSets::dataSetName(uint16_t number, uint16_t recordId) { int idx = dataSetIdx(number, recordId); - if (idx != -1) return records_[recordId][idx].name_; + if (idx != -1) + return records_[recordId][idx].name_; std::ostringstream os; os << "0x" << std::setw(4) << std::setfill('0') << std::right @@ -469,28 +475,32 @@ namespace Exiv2 { const char* IptcDataSets::dataSetTitle(uint16_t number, uint16_t recordId) { int idx = dataSetIdx(number, recordId); - if (idx == -1) return unknownDataSet.title_; + if (idx == -1) + return unknownDataSet.title_; return records_[recordId][idx].title_; } const char* IptcDataSets::dataSetDesc(uint16_t number, uint16_t recordId) { int idx = dataSetIdx(number, recordId); - if (idx == -1) return unknownDataSet.desc_; + if (idx == -1) + return unknownDataSet.desc_; return records_[recordId][idx].desc_; } const char* IptcDataSets::dataSetPsName(uint16_t number, uint16_t recordId) { int idx = dataSetIdx(number, recordId); - if (idx == -1) return unknownDataSet.photoshop_; + if (idx == -1) + return unknownDataSet.photoshop_; return records_[recordId][idx].photoshop_; } bool IptcDataSets::dataSetRepeatable(uint16_t number, uint16_t recordId) { int idx = dataSetIdx(number, recordId); - if (idx == -1) return unknownDataSet.repeatable_; + if (idx == -1) + return unknownDataSet.repeatable_; return records_[recordId][idx].repeatable_; } @@ -504,7 +514,8 @@ namespace Exiv2 { dataSet = records_[recordId][idx].number_; } else { - if (!isHex(dataSetName, 4, "0x")) throw Error(kerInvalidDataset, dataSetName); + if (!isHex(dataSetName, 4, "0x")) + throw Error(kerInvalidDataset, dataSetName); std::istringstream is(dataSetName); is >> std::hex >> dataSet; } @@ -552,7 +563,7 @@ namespace Exiv2 { os << record[j] << "\n"; } } - } // IptcDataSets::dataSetList + } IptcKey::IptcKey(std::string key) : key_(std::move(key)) { diff --git a/unitTests/CMakeLists.txt b/unitTests/CMakeLists.txt index 375f9820..eb814f8d 100644 --- a/unitTests/CMakeLists.txt +++ b/unitTests/CMakeLists.txt @@ -3,6 +3,7 @@ find_package(GTest REQUIRED) add_executable(unit_tests mainTestRunner.cpp test_bmpimage.cpp + test_datasets.cpp test_DateValue.cpp test_TimeValue.cpp test_XmpKey.cpp diff --git a/unitTests/test_datasets.cpp b/unitTests/test_datasets.cpp new file mode 100644 index 00000000..e7425062 --- /dev/null +++ b/unitTests/test_datasets.cpp @@ -0,0 +1,163 @@ +#include +#include + +#include +#include + +#include + +using namespace Exiv2; +using ::testing::StartsWith; + +TEST(IptcDataSets, dataSetName_returnsValidNamesWhenRequestingNumbersAvailableInEnvelopeRecord) +{ + ASSERT_EQ("ModelVersion", IptcDataSets::dataSetName(IptcDataSets::ModelVersion, IptcDataSets::envelope)); + ASSERT_EQ("Destination", IptcDataSets::dataSetName(IptcDataSets::Destination, IptcDataSets::envelope)); + ASSERT_EQ("FileFormat", IptcDataSets::dataSetName(IptcDataSets::FileFormat, IptcDataSets::envelope)); + ASSERT_EQ("FileVersion", IptcDataSets::dataSetName(IptcDataSets::FileVersion, IptcDataSets::envelope)); + ASSERT_EQ("ServiceId", IptcDataSets::dataSetName(IptcDataSets::ServiceId, IptcDataSets::envelope)); + ASSERT_EQ("EnvelopeNumber", IptcDataSets::dataSetName(IptcDataSets::EnvelopeNumber, IptcDataSets::envelope)); +} + +TEST(IptcDataSets, dataSetName_returnsValidNamesWhenRequestingNumbersAvailableInApplicationRecord) +{ + ASSERT_EQ("ObjectType", IptcDataSets::dataSetName(IptcDataSets::ObjectType, IptcDataSets::application2)); + ASSERT_EQ("ObjectAttribute", IptcDataSets::dataSetName(IptcDataSets::ObjectAttribute, IptcDataSets::application2)); +} + +TEST(IptcDataSets, dataSetName_returnsWrongNamesWhenRequestingNumbersNotAvailableInEnvelopeRecord) +{ + ASSERT_EQ("0x0003", IptcDataSets::dataSetName(IptcDataSets::ObjectType, IptcDataSets::envelope)); + ASSERT_EQ("0x0004", IptcDataSets::dataSetName(IptcDataSets::ObjectAttribute, IptcDataSets::envelope)); +} + +TEST(IptcDataSets, dataSetTitle_returnsValidTitleWhenRequestingNumbersAvailableInRecord) +{ + ASSERT_STREQ("Model Version", IptcDataSets::dataSetTitle(IptcDataSets::ModelVersion, IptcDataSets::envelope)); + ASSERT_STREQ("Destination", IptcDataSets::dataSetTitle(IptcDataSets::Destination, IptcDataSets::envelope)); + ASSERT_STREQ("File Format", IptcDataSets::dataSetTitle(IptcDataSets::FileFormat, IptcDataSets::envelope)); + + ASSERT_STREQ("Object Type", IptcDataSets::dataSetTitle(IptcDataSets::ObjectType, IptcDataSets::application2)); + ASSERT_STREQ("Object Attribute", + IptcDataSets::dataSetTitle(IptcDataSets::ObjectAttribute, IptcDataSets::application2)); +} + +TEST(IptcDataSets, dataSetTitle_returnsUnknownStringWhenRequestingNumbersNotAvailableInEnvelopeRecord) +{ + ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetTitle(IptcDataSets::ObjectType, IptcDataSets::envelope)); + ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetTitle(IptcDataSets::ObjectAttribute, IptcDataSets::envelope)); +} + +// Unfortunately, some constants such as ModelVersion, Destination or FileFormat has the same values as other constants +// available for other records (RecordVersion, ObjectName & SuppCategory respectively) + +// TEST(IptcDataSets, dataSetTitle_returnsUnknownStringWhenRequestingNumbersNotAvailableInApplicationRecord) +//{ +// ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetTitle(IptcDataSets::ModelVersion, IptcDataSets::application2)); +// ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetTitle(IptcDataSets::Destination, IptcDataSets::application2)); +// ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetTitle(IptcDataSets::FileFormat, IptcDataSets::application2)); +//} + +TEST(IptcDataSets, dataSetTitle_returnsUnknownStringWhenRequestingNumbersNotAvailableInApplicationRecord) +{ + ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetTitle(1, IptcDataSets::envelope)); + ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetTitle(2, IptcDataSets::envelope)); +} + +// ---------------------- + +TEST(IptcDataSets, dataSetDescription_returnsValidDescriptionWhenRequestingNumbersAvailableInRecord) +{ + ASSERT_THAT(IptcDataSets::dataSetDesc(IptcDataSets::ModelVersion, IptcDataSets::envelope), + StartsWith("A binary number identifying the version of the Information Interchange Model")); + ASSERT_THAT(IptcDataSets::dataSetDesc(IptcDataSets::FileFormat, IptcDataSets::envelope), + StartsWith("A binary number representing the file format. The file format must be registered with")); + + ASSERT_THAT(IptcDataSets::dataSetDesc(IptcDataSets::RecordVersion, IptcDataSets::application2), + StartsWith("A binary number identifying the version of the Information Interchange Model")); + ASSERT_THAT(IptcDataSets::dataSetDesc(IptcDataSets::ObjectType, IptcDataSets::application2), + StartsWith("The Object Type is used to distinguish between different types of objects within the IIM")); +} + +TEST(IptcDataSets, dataSetDescription_returnsUnknownStringWhenRequestingNumbersNotAvailableInRecord) +{ + ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetDesc(1, IptcDataSets::envelope)); + ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetDesc(2, IptcDataSets::envelope)); + + ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetDesc(1, IptcDataSets::application2)); + ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetDesc(2, IptcDataSets::application2)); +} + +// ---------------------- + +TEST(IptcDataSets, dataSetPsName_returnsValidPsNameWhenRequestingNumbersAvailableInRecord) +{ + ASSERT_STREQ("", IptcDataSets::dataSetPsName(IptcDataSets::FileFormat, IptcDataSets::envelope)); + + ASSERT_STREQ("Document Title", IptcDataSets::dataSetPsName(IptcDataSets::ObjectName, IptcDataSets::application2)); + ASSERT_STREQ("Urgency", IptcDataSets::dataSetPsName(IptcDataSets::Urgency, IptcDataSets::application2)); +} + +TEST(IptcDataSets, dataSetPsName_returnsUnknownStringWhenRequestingNumbersNotAvailableInRecord) +{ + ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetPsName(1, IptcDataSets::envelope)); + ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetPsName(2, IptcDataSets::envelope)); + + ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetPsName(1, IptcDataSets::application2)); + ASSERT_STREQ("Unknown dataset", IptcDataSets::dataSetPsName(2, IptcDataSets::application2)); +} + +// ---------------------- + +TEST(IptcDataSets, dataSetRepeatable_returnsExpectedValueNameWhenRequestingNumbersAvailableInRecord) +{ + ASSERT_TRUE(IptcDataSets::dataSetRepeatable(IptcDataSets::Destination, IptcDataSets::envelope)); + ASSERT_FALSE(IptcDataSets::dataSetRepeatable(IptcDataSets::FileFormat, IptcDataSets::envelope)); + + ASSERT_FALSE(IptcDataSets::dataSetRepeatable(IptcDataSets::ObjectType, IptcDataSets::application2)); + ASSERT_TRUE(IptcDataSets::dataSetRepeatable(IptcDataSets::ObjectAttribute, IptcDataSets::application2)); +} + +/// \todo check if we want to return true in this case or throw an exception ... +TEST(IptcDataSets, dataSetRepeatable_returnsTrueWhenRequestingNumbersNotAvailableInRecord) +{ + ASSERT_TRUE(IptcDataSets::dataSetRepeatable(1, IptcDataSets::envelope)); + ASSERT_TRUE(IptcDataSets::dataSetRepeatable(2, IptcDataSets::envelope)); + + ASSERT_TRUE(IptcDataSets::dataSetRepeatable(1, IptcDataSets::application2)); + ASSERT_TRUE(IptcDataSets::dataSetRepeatable(2, IptcDataSets::application2)); +} + +// ---------------------- + +TEST(IptcDataSets, dataSet_returnsExpectedValueWhenRequestingValidDatasetName) +{ + ASSERT_EQ(IptcDataSets::ModelVersion, IptcDataSets::dataSet("ModelVersion", IptcDataSets::envelope)); + ASSERT_EQ(IptcDataSets::FileFormat, IptcDataSets::dataSet("FileFormat", IptcDataSets::envelope)); + + ASSERT_EQ(IptcDataSets::RecordVersion, IptcDataSets::dataSet("RecordVersion", IptcDataSets::application2)); + ASSERT_EQ(IptcDataSets::FixtureId, IptcDataSets::dataSet("FixtureId", IptcDataSets::application2)); +} + +TEST(IptcDataSets, dataSet_throwWithNonExistingDatasetName) +{ + try { + IptcDataSets::dataSet("NonExistingName", IptcDataSets::envelope); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerInvalidDataset, e.code()); + ASSERT_STREQ("Invalid dataset name `NonExistingName'", e.what()); + } +} + +/// \todo Weird error reporting here. It should indicate that the record specified does not exist +TEST(IptcDataSets, dataSet_throwWithNonExistingRecordId) +{ + try { + IptcDataSets::dataSet("ModelVersion", 5); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerInvalidDataset, e.code()); + ASSERT_STREQ("Invalid dataset name `ModelVersion'", e.what()); + } +} From 1d6bac61f2c3f2a15be6fdfaed4aa33682dd9034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Mon, 10 Jan 2022 18:08:31 +0100 Subject: [PATCH 04/14] Delete dead code --- include/exiv2/datasets.hpp | 6 +----- src/datasets.cpp | 10 ---------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/include/exiv2/datasets.hpp b/include/exiv2/datasets.hpp index 5d98ef5a..ed9ff58e 100644 --- a/include/exiv2/datasets.hpp +++ b/include/exiv2/datasets.hpp @@ -237,11 +237,7 @@ namespace Exiv2 { @throw Error if the record is not known; */ static uint16_t recordId(const std::string& recordName); - //! Return read-only list of built-in Envelope Record datasets - static const DataSet* envelopeRecordList(); - //! Return read-only list of built-in Application2 Record datasets - static const DataSet* application2RecordList(); - //! Print a list of all dataSets to output stream + static void dataSetList(std::ostream& os); private: diff --git a/src/datasets.cpp b/src/datasets.cpp index ac63df6a..c0f01378 100644 --- a/src/datasets.cpp +++ b/src/datasets.cpp @@ -125,11 +125,6 @@ namespace Exiv2 { "(Invalid)", false, false, 0, 0, Exiv2::unsignedShort, IptcDataSets::envelope, ""}, }; - const DataSet* IptcDataSets::envelopeRecordList() - { - return envelopeRecord; - } - constexpr DataSet application2Record[] = { {IptcDataSets::RecordVersion, "RecordVersion", N_("Record Version"), N_("A binary number identifying the version of the Information " @@ -402,11 +397,6 @@ namespace Exiv2 { false, false, 0, 0, Exiv2::unsignedShort, IptcDataSets::application2, ""}, }; - const DataSet* IptcDataSets::application2RecordList() - { - return application2Record; - } - constexpr DataSet unknownDataSet{0xffff, "Unknown dataset", N_("Unknown dataset"), N_("Unknown dataset"), false, true, 0, 0xffffffff, Exiv2::string, From 8f9a780375a05bc78cd9de31661abf7898603b35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Mon, 10 Jan 2022 18:09:31 +0100 Subject: [PATCH 05/14] clang-format on datasets.cpp --- src/datasets.cpp | 81 +++++++++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 46 deletions(-) diff --git a/src/datasets.cpp b/src/datasets.cpp index c0f01378..5e750d46 100644 --- a/src/datasets.cpp +++ b/src/datasets.cpp @@ -397,11 +397,10 @@ namespace Exiv2 { false, false, 0, 0, Exiv2::unsignedShort, IptcDataSets::application2, ""}, }; - constexpr DataSet unknownDataSet{0xffff, "Unknown dataset", N_("Unknown dataset"), - N_("Unknown dataset"), - false, true, 0, 0xffffffff, Exiv2::string, - IptcDataSets::invalidRecord, - N_("Unknown dataset"),}; + constexpr DataSet unknownDataSet{ + 0xffff, "Unknown dataset", N_("Unknown dataset"), N_("Unknown dataset"), false, true, 0, + 0xffffffff, Exiv2::string, IptcDataSets::invalidRecord, N_("Unknown dataset"), + }; // Dataset lookup lists.This is an array with pointers to one list per IIM4 Record. // The record id is used as the index into the array. @@ -414,7 +413,7 @@ namespace Exiv2 { int IptcDataSets::dataSetIdx(uint16_t number, uint16_t recordId) { - if( recordId != envelope && recordId != application2 ) + if (recordId != envelope && recordId != application2) return -1; const DataSet* dataSet = records_[recordId]; if (dataSet == nullptr) @@ -429,7 +428,7 @@ namespace Exiv2 { int IptcDataSets::dataSetIdx(const std::string& dataSetName, uint16_t recordId) { - if( recordId != envelope && recordId != application2 ) + if (recordId != envelope && recordId != application2) return -1; const DataSet* dataSet = records_[recordId]; if (dataSet == nullptr) @@ -457,8 +456,7 @@ namespace Exiv2 { return records_[recordId][idx].name_; std::ostringstream os; - os << "0x" << std::setw(4) << std::setfill('0') << std::right - << std::hex << number; + os << "0x" << std::setw(4) << std::setfill('0') << std::right << std::hex << number; return os.str(); } @@ -494,16 +492,14 @@ namespace Exiv2 { return records_[recordId][idx].repeatable_; } - uint16_t IptcDataSets::dataSet(const std::string& dataSetName, - uint16_t recordId) + uint16_t IptcDataSets::dataSet(const std::string& dataSetName, uint16_t recordId) { uint16_t dataSet = 0; int idx = dataSetIdx(dataSetName, recordId); if (idx != -1) { // dataSetIdx checks the range of recordId dataSet = records_[recordId][idx].number_; - } - else { + } else { if (!isHex(dataSetName, 4, "0x")) throw Error(kerInvalidDataset, dataSetName); std::istringstream is(dataSetName); @@ -519,8 +515,7 @@ namespace Exiv2 { } std::ostringstream os; - os << "0x" << std::setw(4) << std::setfill('0') << std::right - << std::hex << recordId; + os << "0x" << std::setw(4) << std::setfill('0') << std::right << std::hex << recordId; return os.str(); } @@ -536,10 +531,12 @@ namespace Exiv2 { { uint16_t i; for (i = application2; i > 0; --i) { - if (recordInfo_[i].name_ == recordName) break; + if (recordInfo_[i].name_ == recordName) + break; } if (i == 0) { - if (!isHex(recordName, 4, "0x")) throw Error(kerInvalidRecord, recordName); + if (!isHex(recordName, 4, "0x")) + throw Error(kerInvalidRecord, recordName); std::istringstream is(recordName); is >> std::hex >> i; } @@ -560,22 +557,19 @@ namespace Exiv2 { decomposeKey(); } - IptcKey::IptcKey(uint16_t tag, uint16_t record) - : tag_(tag), record_(record) + IptcKey::IptcKey(uint16_t tag, uint16_t record) : tag_(tag), record_(record) { makeKey(); } - IptcKey::IptcKey(const IptcKey& rhs) - : tag_(rhs.tag_) - , record_(rhs.record_) - , key_(rhs.key_) + IptcKey::IptcKey(const IptcKey& rhs) : tag_(rhs.tag_), record_(rhs.record_), key_(rhs.key_) { } IptcKey& IptcKey::operator=(const IptcKey& rhs) { - if (this == &rhs) return *this; + if (this == &rhs) + return *this; Key::operator=(rhs); tag_ = rhs.tag_; record_ = rhs.record_; @@ -637,14 +631,16 @@ namespace Exiv2 { { // Get the family name, record name and dataSet name parts of the key std::string::size_type pos1 = key_.find('.'); - if (pos1 == std::string::npos) throw Error(kerInvalidKey, key_); + if (pos1 == std::string::npos) + throw Error(kerInvalidKey, key_); std::string familyName = key_.substr(0, pos1); if (0 != strcmp(familyName.c_str(), familyName_)) { throw Error(kerInvalidKey, key_); } std::string::size_type pos0 = pos1 + 1; pos1 = key_.find('.', pos0); - if (pos1 == std::string::npos) throw Error(kerInvalidKey, key_); + if (pos1 == std::string::npos) + throw Error(kerInvalidKey, key_); std::string recordName = key_.substr(pos0, pos1 - pos0); if (recordName.empty()) throw Error(kerInvalidKey, key_); @@ -663,13 +659,12 @@ namespace Exiv2 { tag_ = dataSet; record_ = recId; key_ = familyName + "." + recordName + "." + dataSetName; - } // IptcKey::decomposeKey + } // IptcKey::decomposeKey void IptcKey::makeKey() { - key_ = std::string(familyName_) - + "." + IptcDataSets::recordName(record_) - + "." + IptcDataSets::dataSetName(tag_, record_); + key_ = std::string(familyName_) + "." + IptcDataSets::recordName(record_) + "." + + IptcDataSets::dataSetName(tag_, record_); } // ************************************************************************* @@ -677,27 +672,21 @@ namespace Exiv2 { std::ostream& operator<<(std::ostream& os, const DataSet& dataSet) { - std::ios::fmtflags f( os.flags() ); + std::ios::fmtflags f(os.flags()); IptcKey iptcKey(dataSet.number_, dataSet.recordId_); - os << dataSet.name_ << ", " - << std::dec << dataSet.number_ << ", " - << "0x" << std::setw(4) << std::setfill('0') - << std::right << std::hex << dataSet.number_ << ", " - << IptcDataSets::recordName(dataSet.recordId_) << ", " - << std::boolalpha << dataSet.mandatory_ << ", " - << dataSet.repeatable_ << ", " - << std::dec << dataSet.minbytes_ << ", " - << dataSet.maxbytes_ << ", " - << iptcKey.key() << ", " - << TypeInfo::typeName( - IptcDataSets::dataSetType(dataSet.number_, - dataSet.recordId_)) << ", "; + os << dataSet.name_ << ", " << std::dec << dataSet.number_ << ", " + << "0x" << std::setw(4) << std::setfill('0') << std::right << std::hex << dataSet.number_ << ", " + << IptcDataSets::recordName(dataSet.recordId_) << ", " << std::boolalpha << dataSet.mandatory_ << ", " + << dataSet.repeatable_ << ", " << std::dec << dataSet.minbytes_ << ", " << dataSet.maxbytes_ << ", " + << iptcKey.key() << ", " << TypeInfo::typeName(IptcDataSets::dataSetType(dataSet.number_, dataSet.recordId_)) + << ", "; // CSV encoded I am \"dead\" beat" => "I am ""dead"" beat" char Q = '"'; os << Q; - for ( size_t i = 0 ; i < ::strlen(dataSet.desc_) ; i++ ) { + for (size_t i = 0; i < ::strlen(dataSet.desc_); i++) { char c = dataSet.desc_[i]; - if ( c == Q ) os << Q; + if (c == Q) + os << Q; os << c; } os << Q; From df0b7c450ddfd64381417a2f6349754146d0b762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Mon, 10 Jan 2022 18:25:26 +0100 Subject: [PATCH 06/14] Hide public details --- include/exiv2/datasets.hpp | 8 -------- src/datasets.cpp | 40 ++++++++++++++++++++++---------------- 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/include/exiv2/datasets.hpp b/include/exiv2/datasets.hpp index ed9ff58e..48f50596 100644 --- a/include/exiv2/datasets.hpp +++ b/include/exiv2/datasets.hpp @@ -230,13 +230,6 @@ namespace Exiv2 { @return the description of the Record */ static const char* recordDesc(uint16_t recordId); - /*! - @brief Return the Id number of a record - @param recordName Name of a record type - @return the Id number of a Record - @throw Error if the record is not known; - */ - static uint16_t recordId(const std::string& recordName); static void dataSetList(std::ostream& os); @@ -245,7 +238,6 @@ namespace Exiv2 { static int dataSetIdx(const std::string& dataSetName, uint16_t recordId); static const DataSet* const records_[]; - static const RecordInfo recordInfo_[]; }; // class IptcDataSets diff --git a/src/datasets.cpp b/src/datasets.cpp index 5e750d46..53b6764c 100644 --- a/src/datasets.cpp +++ b/src/datasets.cpp @@ -38,8 +38,9 @@ // ***************************************************************************** // class member definitions + namespace Exiv2 { - constexpr RecordInfo IptcDataSets::recordInfo_[] = { + constexpr RecordInfo recordInfo_[] = { {IptcDataSets::invalidRecord, "(invalid)", N_("(invalid)")}, {IptcDataSets::envelope, "Envelope", N_("IIM envelope record")}, {IptcDataSets::application2, "Application2", N_("IIM application record 2")}, @@ -397,6 +398,22 @@ namespace Exiv2 { false, false, 0, 0, Exiv2::unsignedShort, IptcDataSets::application2, ""}, }; + uint16_t recordId(const std::string& recordName) + { + uint16_t i; + for (i = IptcDataSets::application2; i > 0; --i) { + if (recordInfo_[i].name_ == recordName) + break; + } + if (i == 0) { + if (!isHex(recordName, 4, "0x")) + throw Error(kerInvalidRecord, recordName); + std::istringstream is(recordName); + is >> std::hex >> i; + } + return i; + } + constexpr DataSet unknownDataSet{ 0xffff, "Unknown dataset", N_("Unknown dataset"), N_("Unknown dataset"), false, true, 0, 0xffffffff, Exiv2::string, IptcDataSets::invalidRecord, N_("Unknown dataset"), @@ -527,21 +544,6 @@ namespace Exiv2 { return recordInfo_[recordId].desc_; } - uint16_t IptcDataSets::recordId(const std::string& recordName) - { - uint16_t i; - for (i = application2; i > 0; --i) { - if (recordInfo_[i].name_ == recordName) - break; - } - if (i == 0) { - if (!isHex(recordName, 4, "0x")) - throw Error(kerInvalidRecord, recordName); - std::istringstream is(recordName); - is >> std::hex >> i; - } - return i; - } void IptcDataSets::dataSetList(std::ostream& os) { @@ -633,23 +635,27 @@ namespace Exiv2 { std::string::size_type pos1 = key_.find('.'); if (pos1 == std::string::npos) throw Error(kerInvalidKey, key_); + std::string familyName = key_.substr(0, pos1); if (0 != strcmp(familyName.c_str(), familyName_)) { throw Error(kerInvalidKey, key_); } + std::string::size_type pos0 = pos1 + 1; pos1 = key_.find('.', pos0); if (pos1 == std::string::npos) throw Error(kerInvalidKey, key_); + std::string recordName = key_.substr(pos0, pos1 - pos0); if (recordName.empty()) throw Error(kerInvalidKey, key_); + std::string dataSetName = key_.substr(pos1 + 1); if (dataSetName.empty()) throw Error(kerInvalidKey, key_); // Use the parts of the key to find dataSet and recordId - uint16_t recId = IptcDataSets::recordId(recordName); + uint16_t recId = recordId(recordName); uint16_t dataSet = IptcDataSets::dataSet(dataSetName, recId); // Possibly translate hex name parts (0xabcd) to real names From d8fcbc45623527de0569454e319df46d1b73b676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Mon, 10 Jan 2022 18:25:54 +0100 Subject: [PATCH 07/14] Add new tests for Iptc classes --- src/error.cpp | 6 ++--- unitTests/CMakeLists.txt | 1 + unitTests/test_IptcKey.cpp | 44 +++++++++++++++++++++++++++++++++++++ unitTests/test_datasets.cpp | 10 +++++++++ 4 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 unitTests/test_IptcKey.cpp diff --git a/src/error.cpp b/src/error.cpp index d6a80c7f..488b1faf 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -55,11 +55,11 @@ namespace { { Exiv2::kerNotAnImage, N_("This does not look like a %1 image") }, // %1=Image type { Exiv2::kerInvalidDataset, - N_("Invalid dataset name `%1'") }, // %1=dataset name + N_("Invalid dataset name '%1'") }, // %1=dataset name { Exiv2::kerInvalidRecord, - N_("Invalid record name `%1'") }, // %1=record name + N_("Invalid record name '%1'") }, // %1=record name { Exiv2::kerInvalidKey, - N_("Invalid key `%1'") }, // %1=key + N_("Invalid key '%1'") }, // %1=key { Exiv2::kerInvalidTag, N_("Invalid tag name or ifdId `%1', ifdId %2") }, // %1=tag name, %2=ifdId { Exiv2::kerValueNotSet, diff --git a/unitTests/CMakeLists.txt b/unitTests/CMakeLists.txt index eb814f8d..a8e4b852 100644 --- a/unitTests/CMakeLists.txt +++ b/unitTests/CMakeLists.txt @@ -14,6 +14,7 @@ add_executable(unit_tests test_futils.cpp test_helper_functions.cpp test_image_int.cpp + test_IptcKey.cpp test_pngimage.cpp test_safe_op.cpp test_slice.cpp diff --git a/unitTests/test_IptcKey.cpp b/unitTests/test_IptcKey.cpp new file mode 100644 index 00000000..e720a726 --- /dev/null +++ b/unitTests/test_IptcKey.cpp @@ -0,0 +1,44 @@ +#include + +#include +#include + +using namespace Exiv2; + +TEST(IptcKey, creationWithNonValidStringFormatThrows) +{ + try { + IptcKey key("Yeah"); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerInvalidKey, e.code()); + ASSERT_STREQ("Invalid key 'Yeah'", e.what()); + } +} + +TEST(IptcKey, creationWithNonValidRecordNameThrows) +{ + try { + IptcKey key("Iptc.WrongRecordName.ModelVersion"); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerInvalidRecord, e.code()); + ASSERT_STREQ("Invalid record name 'WrongRecordName'", e.what()); + } +} + +TEST(IptcKey, creationWithNonValidDatasetNameThrows) +{ + try { + IptcKey key("Iptc.Envelope.WrongDataset"); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerInvalidDataset, e.code()); + ASSERT_STREQ("Invalid dataset name 'WrongDataset'", e.what()); + } +} + +TEST(IptcKey, creationWithValidStringDoesNotThrow) +{ + ASSERT_NO_THROW(IptcKey ("Iptc.Envelope.ModelVersion")); +} diff --git a/unitTests/test_datasets.cpp b/unitTests/test_datasets.cpp index e7425062..0f755068 100644 --- a/unitTests/test_datasets.cpp +++ b/unitTests/test_datasets.cpp @@ -5,6 +5,7 @@ #include #include +#include using namespace Exiv2; using ::testing::StartsWith; @@ -161,3 +162,12 @@ TEST(IptcDataSets, dataSet_throwWithNonExistingRecordId) ASSERT_STREQ("Invalid dataset name `ModelVersion'", e.what()); } } + +// ---------------------- + +TEST(IptcDataSets, dataSetLists_printDatasetsIntoOstream) +{ + std::ostringstream stream; + ASSERT_NO_THROW(IptcDataSets::dataSetList(stream)); + ASSERT_FALSE(stream.str().empty()); +} From 15e8c75a389518479fcb5d7090435e325f0e65d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Mon, 10 Jan 2022 18:45:17 +0100 Subject: [PATCH 08/14] Delete unused IptcKey copy operator --- include/exiv2/datasets.hpp | 8 +------- src/datasets.cpp | 11 ----------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/include/exiv2/datasets.hpp b/include/exiv2/datasets.hpp index 48f50596..2c34b90f 100644 --- a/include/exiv2/datasets.hpp +++ b/include/exiv2/datasets.hpp @@ -268,17 +268,11 @@ namespace Exiv2 { IptcKey(uint16_t tag, uint16_t record); //! Copy constructor IptcKey(const IptcKey& rhs); + IptcKey& operator=(const IptcKey& rhs) = delete; //! Destructor ~IptcKey() override = default; //@} - //! @name Manipulators - //@{ - /*! - @brief Assignment operator. - */ - IptcKey& operator=(const IptcKey& rhs); - //@} //! @name Accessors //@{ diff --git a/src/datasets.cpp b/src/datasets.cpp index 53b6764c..556b0963 100644 --- a/src/datasets.cpp +++ b/src/datasets.cpp @@ -568,17 +568,6 @@ namespace Exiv2 { { } - IptcKey& IptcKey::operator=(const IptcKey& rhs) - { - if (this == &rhs) - return *this; - Key::operator=(rhs); - tag_ = rhs.tag_; - record_ = rhs.record_; - key_ = rhs.key_; - return *this; - } - std::string IptcKey::key() const { return key_; From 9b40f948eb7137c928f10419df5cb3ef5290cbd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Mon, 10 Jan 2022 18:47:47 +0100 Subject: [PATCH 09/14] More tests --- src/datasets.cpp | 3 +- unitTests/test_IptcKey.cpp | 72 +++++++++++++++++++++++++++++++++++++ unitTests/test_datasets.cpp | 4 +-- 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/datasets.cpp b/src/datasets.cpp index 556b0963..f7c6dfb4 100644 --- a/src/datasets.cpp +++ b/src/datasets.cpp @@ -620,6 +620,7 @@ namespace Exiv2 { void IptcKey::decomposeKey() { + /// \todo Use regex to check the expected format. Then process the 3 expected chunks // Get the family name, record name and dataSet name parts of the key std::string::size_type pos1 = key_.find('.'); if (pos1 == std::string::npos) @@ -654,7 +655,7 @@ namespace Exiv2 { tag_ = dataSet; record_ = recId; key_ = familyName + "." + recordName + "." + dataSetName; - } // IptcKey::decomposeKey + } void IptcKey::makeKey() { diff --git a/unitTests/test_IptcKey.cpp b/unitTests/test_IptcKey.cpp index e720a726..2b12536c 100644 --- a/unitTests/test_IptcKey.cpp +++ b/unitTests/test_IptcKey.cpp @@ -38,7 +38,79 @@ TEST(IptcKey, creationWithNonValidDatasetNameThrows) } } +TEST(IptcKey, creationWithNonValidFamiltyNameThrows) +{ + try { + IptcKey key("JOJO.Envelope.WrongDataset"); + FAIL(); + } catch (const Exiv2::Error& e) { + ASSERT_EQ(kerInvalidKey, e.code()); + ASSERT_STREQ("Invalid key 'JOJO.Envelope.WrongDataset'", e.what()); + } +} + TEST(IptcKey, creationWithValidStringDoesNotThrow) { ASSERT_NO_THROW(IptcKey ("Iptc.Envelope.ModelVersion")); } + +TEST(IptcKey, copyConstructor) +{ + IptcKey key ("Iptc.Envelope.ModelVersion"); + IptcKey keyCopy (key); +} + +TEST(IptcKey, clone) +{ + IptcKey key ("Iptc.Envelope.ModelVersion"); + auto keyClone = key.clone(); + ASSERT_EQ("Iptc.Envelope.ModelVersion", keyClone->key()); +} + +TEST(IptcKey, keyReturnsTheFullString) +{ + IptcKey key ("Iptc.Envelope.ModelVersion"); + ASSERT_EQ("Iptc.Envelope.ModelVersion", key.key()); +} + +TEST(IptcKey, familyNameReturnsTheFullString) +{ + IptcKey key ("Iptc.Envelope.ModelVersion"); + ASSERT_STREQ("Iptc", key.familyName()); +} + +TEST(IptcKey, groupNameReturnsTheRecordName) +{ + IptcKey key ("Iptc.Envelope.ModelVersion"); + ASSERT_EQ("Envelope", key.groupName()); +} + +TEST(IptcKey, recordNameReturnsTheRecordName) +{ + IptcKey key ("Iptc.Envelope.ModelVersion"); + ASSERT_EQ("Envelope", key.recordName()); +} + +TEST(IptcKey, tagNameReturnsTheDatasetName) +{ + IptcKey key ("Iptc.Envelope.ModelVersion"); + ASSERT_EQ("ModelVersion", key.tagName()); +} + +TEST(IptcKey, tagLabelReturnsTheDatasetTitle) +{ + IptcKey key ("Iptc.Envelope.ModelVersion"); + ASSERT_EQ("Model Version", key.tagLabel()); +} + +TEST(IptcKey, tag) +{ + IptcKey key ("Iptc.Envelope.ModelVersion"); + ASSERT_EQ(IptcDataSets::ModelVersion, key.tag()); +} + +TEST(IptcKey, record) +{ + IptcKey key ("Iptc.Envelope.ModelVersion"); + ASSERT_EQ(IptcDataSets::envelope, key.record()); +} diff --git a/unitTests/test_datasets.cpp b/unitTests/test_datasets.cpp index 0f755068..6eb9588b 100644 --- a/unitTests/test_datasets.cpp +++ b/unitTests/test_datasets.cpp @@ -147,7 +147,7 @@ TEST(IptcDataSets, dataSet_throwWithNonExistingDatasetName) FAIL(); } catch (const Exiv2::Error& e) { ASSERT_EQ(kerInvalidDataset, e.code()); - ASSERT_STREQ("Invalid dataset name `NonExistingName'", e.what()); + ASSERT_STREQ("Invalid dataset name 'NonExistingName'", e.what()); } } @@ -159,7 +159,7 @@ TEST(IptcDataSets, dataSet_throwWithNonExistingRecordId) FAIL(); } catch (const Exiv2::Error& e) { ASSERT_EQ(kerInvalidDataset, e.code()); - ASSERT_STREQ("Invalid dataset name `ModelVersion'", e.what()); + ASSERT_STREQ("Invalid dataset name 'ModelVersion'", e.what()); } } From 9573f1edcc2a4cb41cb5434424f225e410b6fdbe Mon Sep 17 00:00:00 2001 From: Luis Diaz Date: Tue, 11 Jan 2022 08:52:30 +0100 Subject: [PATCH 10/14] Fix MSVC warnings by casting integer values --- unitTests/test_bmpimage.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/unitTests/test_bmpimage.cpp b/unitTests/test_bmpimage.cpp index 86764fd8..30a9eaa9 100644 --- a/unitTests/test_bmpimage.cpp +++ b/unitTests/test_bmpimage.cpp @@ -90,7 +90,7 @@ TEST(BmpImage, readMetadataReadsImageDimensionsWhenDataIsAvailable) 0x20, 0x03, 0x00, 0x00, // The bitmap height in pixels (unsigned 16 bit) off:22, size:4 }; - auto memIo = std::make_unique(header.data(), header.size()); + auto memIo = std::make_unique(header.data(), static_cast(header.size())); BmpImage bmp(std::move(memIo)); ASSERT_NO_THROW(bmp.readMetadata()); ASSERT_EQ(1280, bmp.pixelWidth()); @@ -110,7 +110,7 @@ TEST(BmpImage, readMetadataThrowsWhenImageIsNotBMP) 0x20, 0x03, 0x00, 0x00, // The bitmap height in pixels (unsigned 16 bit) off:22, size:4 }; - auto memIo = std::make_unique(header.data(), header.size()); + auto memIo = std::make_unique(header.data(), static_cast(header.size())); BmpImage bmp(std::move(memIo)); try { bmp.readMetadata(); @@ -124,7 +124,7 @@ TEST(BmpImage, readMetadataThrowsWhenImageIsNotBMP) TEST(BmpImage, readMetadataThrowsWhenThereIsNotEnoughInfoToRead) { const std::array header{'B'}; - auto memIo = std::make_unique(header.data(), header.size()); + auto memIo = std::make_unique(header.data(), static_cast(header.size())); BmpImage bmp(std::move(memIo)); try { bmp.readMetadata(); @@ -157,7 +157,7 @@ TEST(newBmpInstance, createsValidInstace) 0x00, 0x00, // Reserved 0x00, 0x00, 0x00, 0x00 // Offset of the byte where the bitmap image data can be found }; - auto memIo = std::make_unique(bitmapHeader.data(), bitmapHeader.size()); + auto memIo = std::make_unique(bitmapHeader.data(), static_cast(bitmapHeader.size())); auto img = newBmpInstance(std::move(memIo), false); ASSERT_TRUE(img->good()); } @@ -178,7 +178,7 @@ TEST(isBmpType, withValidSignatureReturnsTrue) 0x00, 0x00, // Reserved 0x00, 0x00, 0x00, 0x00 // Offset of the byte where the bitmap image data can be found }; - MemIo memIo(bitmapHeader.data(), bitmapHeader.size()); + MemIo memIo(bitmapHeader.data(), static_cast(bitmapHeader.size())); ASSERT_TRUE(isBmpType(memIo, false)); } @@ -191,6 +191,6 @@ TEST(isBmpType, withInvalidSignatureReturnsFalse) 0x00, 0x00, // Reserved 0x00, 0x00, 0x00, 0x00 // Offset of the byte where the bitmap image data can be found }; - MemIo memIo(bitmapHeader.data(), bitmapHeader.size()); + MemIo memIo(bitmapHeader.data(), static_cast(bitmapHeader.size())); ASSERT_FALSE(isBmpType(memIo, false)); } From 476a254dfcd7b031487d8cb38612a272a93e93e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Tue, 11 Jan 2022 09:25:37 +0100 Subject: [PATCH 11/14] Remove dead code No way to obtain nullptr DataSet* when we are checking already the 2 only existing DataSets above. --- include/exiv2/datasets.hpp | 7 +++++++ src/datasets.cpp | 4 ---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/include/exiv2/datasets.hpp b/include/exiv2/datasets.hpp index 2c34b90f..09c512e0 100644 --- a/include/exiv2/datasets.hpp +++ b/include/exiv2/datasets.hpp @@ -173,6 +173,7 @@ namespace Exiv2 { dataset. */ static std::string dataSetName(uint16_t number, uint16_t recordId); + /*! @brief Return the title (label) of the dataset. @param number The dataset number @@ -180,6 +181,7 @@ namespace Exiv2 { @return The title (label) of the dataset */ static const char* dataSetTitle(uint16_t number, uint16_t recordId); + /*! @brief Return the description of the dataset. @param number The dataset number @@ -187,6 +189,7 @@ namespace Exiv2 { @return The description of the dataset */ static const char* dataSetDesc(uint16_t number, uint16_t recordId); + /*! @brief Return the Photoshop name of a given dataset. @param number The dataset number @@ -195,6 +198,7 @@ namespace Exiv2 { string if Photoshop does not use the dataset. */ static const char* dataSetPsName(uint16_t number, uint16_t recordId); + /*! @brief Check if a given dataset is repeatable @param number The dataset number @@ -202,6 +206,7 @@ namespace Exiv2 { @return true if the given dataset is repeatable otherwise false */ static bool dataSetRepeatable(uint16_t number, uint16_t recordId); + /*! @brief Return the dataSet number for dataset name and record id @@ -216,6 +221,7 @@ namespace Exiv2 { //! Return the type for dataSet number and Record id static TypeId dataSetType(uint16_t number, uint16_t recordId); + /*! @brief Return the name of the Record @param recordId The record id @@ -224,6 +230,7 @@ namespace Exiv2 { unknown record. */ static std::string recordName(uint16_t recordId); + /*! @brief Return the description of a record @param recordId Record Id number diff --git a/src/datasets.cpp b/src/datasets.cpp index f7c6dfb4..9ab765a4 100644 --- a/src/datasets.cpp +++ b/src/datasets.cpp @@ -433,8 +433,6 @@ namespace Exiv2 { if (recordId != envelope && recordId != application2) return -1; const DataSet* dataSet = records_[recordId]; - if (dataSet == nullptr) - return -1; int idx; for (idx = 0; dataSet[idx].number_ != number; ++idx) { if (dataSet[idx].number_ == 0xffff) @@ -448,8 +446,6 @@ namespace Exiv2 { if (recordId != envelope && recordId != application2) return -1; const DataSet* dataSet = records_[recordId]; - if (dataSet == nullptr) - return -1; int idx; for (idx = 0; dataSet[idx].name_ != dataSetName; ++idx) { if (dataSet[idx].number_ == 0xffff) From c531c4abf5ef27dd376f0e20516135d7c684ae62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Thu, 27 Jan 2022 15:04:03 +0100 Subject: [PATCH 12/14] More tests for IptcDataSets --- include/exiv2/datasets.hpp | 4 ---- src/datasets.cpp | 31 ++++++++++++------------- unitTests/test_datasets.cpp | 45 +++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 21 deletions(-) diff --git a/include/exiv2/datasets.hpp b/include/exiv2/datasets.hpp index 09c512e0..3fcaedea 100644 --- a/include/exiv2/datasets.hpp +++ b/include/exiv2/datasets.hpp @@ -322,10 +322,6 @@ namespace Exiv2 { //! Internal virtual copy constructor. IptcKey* clone_() const override; - // DATA -#ifndef SWIG - static constexpr auto familyName_ = "Iptc"; -#endif uint16_t tag_; //!< Tag value uint16_t record_; //!< Record value std::string key_; //!< Key diff --git a/src/datasets.cpp b/src/datasets.cpp index 9ab765a4..8654e00e 100644 --- a/src/datasets.cpp +++ b/src/datasets.cpp @@ -34,12 +34,14 @@ #include #include +#include #include // ***************************************************************************** // class member definitions namespace Exiv2 { + constexpr const char *familyName_{"Iptc"}; constexpr RecordInfo recordInfo_[] = { {IptcDataSets::invalidRecord, "(invalid)", N_("(invalid)")}, {IptcDataSets::envelope, "Envelope", N_("IIM envelope record")}, @@ -616,29 +618,24 @@ namespace Exiv2 { void IptcKey::decomposeKey() { - /// \todo Use regex to check the expected format. Then process the 3 expected chunks + // Check that the key has the expected format with RE + static const std::regex re(R"((\w+)(\.\w+){2})"); + std::smatch sm; + if (!std::regex_match(key_, sm, re)) { + throw Error(kerInvalidKey, key_); + } + // Get the family name, record name and dataSet name parts of the key - std::string::size_type pos1 = key_.find('.'); - if (pos1 == std::string::npos) - throw Error(kerInvalidKey, key_); + auto posDot1 = key_.find('.'); + auto posDot2 = key_.find('.', posDot1+1); - std::string familyName = key_.substr(0, pos1); + const std::string familyName = key_.substr(0, posDot1); if (0 != strcmp(familyName.c_str(), familyName_)) { throw Error(kerInvalidKey, key_); } - std::string::size_type pos0 = pos1 + 1; - pos1 = key_.find('.', pos0); - if (pos1 == std::string::npos) - throw Error(kerInvalidKey, key_); - - std::string recordName = key_.substr(pos0, pos1 - pos0); - if (recordName.empty()) - throw Error(kerInvalidKey, key_); - - std::string dataSetName = key_.substr(pos1 + 1); - if (dataSetName.empty()) - throw Error(kerInvalidKey, key_); + std::string recordName = key_.substr(posDot1+1, posDot2 - posDot1 - 1); + std::string dataSetName = key_.substr(posDot2+1); // Use the parts of the key to find dataSet and recordId uint16_t recId = recordId(recordName); diff --git a/unitTests/test_datasets.cpp b/unitTests/test_datasets.cpp index 6eb9588b..781bdda8 100644 --- a/unitTests/test_datasets.cpp +++ b/unitTests/test_datasets.cpp @@ -165,6 +165,51 @@ TEST(IptcDataSets, dataSet_throwWithNonExistingRecordId) // ---------------------- +TEST(IptcDataSets, dataSetType_returnsExpectedTypeWhenRequestingValidDataset) +{ + ASSERT_EQ(unsignedShort, IptcDataSets::dataSetType(IptcDataSets::ModelVersion, IptcDataSets::envelope)); + ASSERT_EQ(Exiv2::string, IptcDataSets::dataSetType(IptcDataSets::Destination, IptcDataSets::envelope)); + + ASSERT_EQ(unsignedShort, IptcDataSets::dataSetType(IptcDataSets::RecordVersion, IptcDataSets::application2)); + ASSERT_EQ(Exiv2::string, IptcDataSets::dataSetType(IptcDataSets::ObjectType, IptcDataSets::application2)); +} + +/// \todo probably better to throw exception here? +TEST(IptcDataSets, dataSetType_returnsStringTypeWhenRecordIdDoesNotExist) +{ + ASSERT_EQ(Exiv2::string, IptcDataSets::dataSetType(1, 5)); +} + +// ---------------------- + +TEST(IptcDataSets, recordName_returnsExpectedNameWhenRequestingValidRecordId) +{ + ASSERT_EQ("Envelope", IptcDataSets::recordName(IptcDataSets::envelope)); + ASSERT_EQ("Application2", IptcDataSets::recordName(IptcDataSets::application2)); +} + +TEST(IptcDataSets, recordName_returnsHexStringWhenRecordIdDoesNotExist) +{ + ASSERT_EQ("0x0000", IptcDataSets::recordName(0)); + ASSERT_EQ("0x0003", IptcDataSets::recordName(3)); +} + +// ---------------------- + +TEST(IptcDataSets, recordDesc_returnsExpectedDescriptionWhenRequestingValidRecordId) +{ + ASSERT_STREQ("IIM envelope record", IptcDataSets::recordDesc(IptcDataSets::envelope)); + ASSERT_STREQ("IIM application record 2", IptcDataSets::recordDesc(IptcDataSets::application2)); +} + +TEST(IptcDataSets, recordDesc_) +{ + ASSERT_STREQ("Unknown dataset", IptcDataSets::recordDesc(0)); + ASSERT_STREQ("Unknown dataset", IptcDataSets::recordDesc(3)); +} + +// ---------------------- + TEST(IptcDataSets, dataSetLists_printDatasetsIntoOstream) { std::ostringstream stream; From 3ed696ac8e3c45509508b81ecd1888a214584691 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Thu, 27 Jan 2022 15:21:02 +0100 Subject: [PATCH 13/14] upgrade coverage scripts --- .../workflows/on_PR_linux_special_builds.yml | 23 +++++++++++++++---- cmake/gcovr.cmake | 1 + 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/.github/workflows/on_PR_linux_special_builds.yml b/.github/workflows/on_PR_linux_special_builds.yml index ddb7243c..092f8a4d 100644 --- a/.github/workflows/on_PR_linux_special_builds.yml +++ b/.github/workflows/on_PR_linux_special_builds.yml @@ -17,6 +17,7 @@ jobs: run: | sudo apt-get install ninja-build pip3 install conan==1.43.0 + pip3 install gcovr - name: Conan common config run: | @@ -32,16 +33,27 @@ jobs: - name: Build run: | - cd build - cmake -GNinja -DCMAKE_BUILD_TYPE=Debug -DBUILD_SHARED_LIBS=ON -DEXIV2_ENABLE_PNG=ON -DEXIV2_ENABLE_WEBREADY=ON -DEXIV2_ENABLE_CURL=ON -DEXIV2_BUILD_UNIT_TESTS=ON -DEXIV2_ENABLE_BMFF=ON -DEXIV2_TEAM_WARNINGS_AS_ERRORS=ON -DBUILD_WITH_COVERAGE=ON -DCMAKE_INSTALL_PREFIX=install .. + cd build && \ + cmake -GNinja \ + -DCMAKE_BUILD_TYPE=Debug \ + -DBUILD_SHARED_LIBS=ON \ + -DEXIV2_ENABLE_PNG=ON \ + -DEXIV2_ENABLE_WEBREADY=ON \ + -DEXIV2_ENABLE_CURL=ON \ + -DEXIV2_BUILD_UNIT_TESTS=ON \ + -DEXIV2_ENABLE_BMFF=ON \ + -DEXIV2_TEAM_WARNINGS_AS_ERRORS=ON \ + -DEXIV2_BUILD_SAMPLES=ON \ + -DBUILD_WITH_COVERAGE=ON \ + -DCMAKE_INSTALL_PREFIX=install \ + .. && \ cmake --build . - name: Tests + Upload coverage run: | cd build ctest --output-on-failure - pip install gcovr - gcovr -r ./../ -x --exclude-unreachable-branches --exclude-throw-branches -o coverage.xml . + gcovr --root .. --object-dir . --exclude-directories xmpsdk --exclude-directories unitTests --exclude-directories samples --exclude '.*xmpsdk.*' --exclude '.*unitTests.*' --exclude '.*samples.*' --exclude-unreachable-branches --exclude-throw-branches --xml -o coverage.xml curl https://keybase.io/codecovsecurity/pgp_keys.asc | gpg --import curl -Os https://uploader.codecov.io/latest/linux/codecov curl -Os https://uploader.codecov.io/latest/linux/codecov.SHA256SUM @@ -49,7 +61,8 @@ jobs: gpg --verify codecov.SHA256SUM.sig codecov.SHA256SUM shasum -a 256 -c codecov.SHA256SUM chmod +x codecov - ./codecov -f build/coverage.xml + ls -lh + ./codecov -f coverage.xml special_releaseValgrind: name: 'Ubuntu 20.04 - GCC - Release+Valgrind' diff --git a/cmake/gcovr.cmake b/cmake/gcovr.cmake index ad069563..75d16e8a 100644 --- a/cmake/gcovr.cmake +++ b/cmake/gcovr.cmake @@ -14,6 +14,7 @@ if(BUILD_WITH_COVERAGE) COMMAND ${GCOVR} --root ${CMAKE_SOURCE_DIR} --object-dir=${CMAKE_BINARY_DIR} --html --html-details -o coverage_output/coverage.html --exclude-directories xmpsdk --exclude-directories unitTests --exclude-directories samples --exclude '.*xmpsdk.*' --exclude '.*unitTests.*' --exclude '.*samples.*' + --exclude-unreachable-branches --exclude-throw-branches WORKING_DIRECTORY ${CMAKE_BINARY_DIR} ) From a42501864ae32f72d214a8fa4ccac8a92a0ea7a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Fri, 4 Feb 2022 17:35:44 +0100 Subject: [PATCH 14/14] Recover some documentation strings --- include/exiv2/bmpimage.hpp | 10 +++++----- src/bmpimage.cpp | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/exiv2/bmpimage.hpp b/include/exiv2/bmpimage.hpp index abd0ccf5..3e93615e 100644 --- a/include/exiv2/bmpimage.hpp +++ b/include/exiv2/bmpimage.hpp @@ -79,16 +79,17 @@ namespace Exiv2 { //! @name Manipulators //@{ void readMetadata() override; - /*! - @brief Todo: Write metadata back to the image. This method is not - yet(?) implemented. Calling it will throw an Error(kerWritingImageFormatUnsupported). - */ + + /// @throws Error(kerWritingImageFormatUnsupported). void writeMetadata() override; + /// @throws Error(kerInvalidSettingForImage) void setExifData(const ExifData& exifData) override; + /// @throws Error(kerInvalidSettingForImage) void setIptcData(const IptcData& iptcData) override; + /// @throws Error(kerInvalidSettingForImage) void setComment(const std::string& comment) override; //@} @@ -96,7 +97,6 @@ namespace Exiv2 { //@{ std::string mimeType() const override; //@} - }; // class BmpImage // ***************************************************************************** diff --git a/src/bmpimage.cpp b/src/bmpimage.cpp index d18979a7..4c4bac98 100644 --- a/src/bmpimage.cpp +++ b/src/bmpimage.cpp @@ -114,7 +114,7 @@ namespace Exiv2 void BmpImage::writeMetadata() { - // Todo: implement me! + /// \todo implement me! throw(Error(kerWritingImageFormatUnsupported, "BMP")); }