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] 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()); + } +}