From 8b0f3f325575e98677cc32e2b99fdf462cee9c3b Mon Sep 17 00:00:00 2001 From: coderhyme Date: Fri, 24 Jul 2015 22:10:39 +0900 Subject: [PATCH] Add a comparison helper class for ResourceAttributes::Value The hepler class is needed to avoid implicitly converting and comparing raw values, because ResourceAttributes::Value has implicit constructors to make it easy to use. Change-Id: Ie72524d9000f930e01e63b0f0e0384295ed4f41d Signed-off-by: coderhyme Reviewed-on: https://gerrit.iotivity.org/gerrit/1890 Tested-by: jenkins-iotivity Reviewed-by: Uze Choi --- .../include/ResourceAttributes.h | 77 ++++++++++++++++------ .../primitiveResource/src/ResourceAttributes.cpp | 45 +++++++------ .../src/serverBuilder/SConscript | 6 +- .../unittests/ResourceClient_Test.cpp | 11 ++-- 4 files changed, 86 insertions(+), 53 deletions(-) diff --git a/service/resource-encapsulation/include/ResourceAttributes.h b/service/resource-encapsulation/include/ResourceAttributes.h index 56fa3ae..8f406e5 100644 --- a/service/resource-encapsulation/include/ResourceAttributes.h +++ b/service/resource-encapsulation/include/ResourceAttributes.h @@ -221,6 +221,8 @@ namespace OIC class Value { public: + class ComparisonHelper; + Value(); Value(const Value&); Value(Value&&); @@ -295,14 +297,6 @@ namespace OIC void swap(Value&); //! @cond - friend bool operator==(const Value&, const Value&); - - template< typename T > - friend typename std::enable_if< ResourceAttributes::is_supported_type< T >::value, - bool >::type operator==(const Value&, const T&); - - bool operator==(const char*) const; - friend class ResourceAttributes; //! @endcond @@ -508,6 +502,38 @@ namespace OIC //! @endcond }; + /** + * A helper class to avoid obscure comparisons of values which are supported + * by ResourceAttributes::Value caused by implicitly converting a value + * to a ResourceAttributes::Value. + * + * @see Value + * @see ResourceAttributes + * @see is_supported_type + */ + class ResourceAttributes::Value::ComparisonHelper + { + public: + ComparisonHelper(const Value&); + + template< typename T > + typename std::enable_if< is_supported_type< T >::value, bool >::type equals( + const T& v) const + { + return m_valueRef.equals< T >(v); + } + + bool equals(const std::string& v) const + { + return m_valueRef.equals< std::string >(v); + } + + bool operator==(const ComparisonHelper&) const; + + private: + const Value& m_valueRef; + }; + template< typename T > struct ResourceAttributes::IsSupportedTypeHelper { @@ -541,7 +567,6 @@ namespace OIC */ bool operator!=(const ResourceAttributes::Type&, const ResourceAttributes::Type&); - /** * @relates ResourceAttributes::Value * @@ -550,7 +575,8 @@ namespace OIC * * @return true if the contents are equal, false otherwise. */ - bool operator==(const ResourceAttributes::Value&, const ResourceAttributes::Value&); + bool operator==(const ResourceAttributes::Value::ComparisonHelper&, + const ResourceAttributes::Value::ComparisonHelper&); /** * @relates ResourceAttributes::Value @@ -560,32 +586,41 @@ namespace OIC * * @return true if the contents are not equal, false otherwise. */ - bool operator!=(const ResourceAttributes::Value&, const ResourceAttributes::Value&); + bool operator!=(const ResourceAttributes::Value::ComparisonHelper&, + const ResourceAttributes::Value::ComparisonHelper&); //! @cond template< typename T > - typename std::enable_if< ResourceAttributes::is_supported_type< T >::value, bool >::type - operator==(const ResourceAttributes::Value& lhs, const T& rhs) + typename std::enable_if< ResourceAttributes::is_supported_type< T >::value || + std::is_constructible< std::string, T >::value, bool >::type + operator==(const ResourceAttributes::Value::ComparisonHelper& lhs, const T& rhs) { - return lhs.equals< T >(rhs); + return lhs.equals(rhs); } template< typename T > - bool operator==(const T& lhs, const ResourceAttributes::Value& rhs) + typename std::enable_if< ResourceAttributes::is_supported_type< T >::value || + std::is_constructible< std::string, T >::value, bool >::type + operator==(const T& lhs, const ResourceAttributes::Value::ComparisonHelper& rhs) { return rhs == lhs; } template< typename T > - typename std::enable_if< ResourceAttributes::is_supported_type< T >::value, bool >::type - operator!=(const T& lhs, const ResourceAttributes::Value& rhs) + typename std::enable_if< ResourceAttributes::is_supported_type< T >::value || + std::is_constructible< std::string, T >::value, bool >::type + operator!=(const ResourceAttributes::Value::ComparisonHelper& lhs, const T& rhs) { - return !(rhs == lhs); + return !(lhs == rhs); } - bool operator!=(const char*, const ResourceAttributes::Value&); - - bool operator==(const char* lhs, const ResourceAttributes::Value& rhs); + template< typename T > + typename std::enable_if< ResourceAttributes::is_supported_type< T >::value || + std::is_constructible< std::string, T >::value, bool >::type + operator!=(const T& lhs, const ResourceAttributes::Value::ComparisonHelper& rhs) + { + return !(rhs == lhs); + } //! @endcond /** diff --git a/service/resource-encapsulation/src/common/primitiveResource/src/ResourceAttributes.cpp b/service/resource-encapsulation/src/common/primitiveResource/src/ResourceAttributes.cpp index b8b5739..4b6250d 100644 --- a/service/resource-encapsulation/src/common/primitiveResource/src/ResourceAttributes.cpp +++ b/service/resource-encapsulation/src/common/primitiveResource/src/ResourceAttributes.cpp @@ -143,12 +143,13 @@ namespace template< typename VARIANT, int POS > static constexpr TypeInfo get() { - return TypeInfo( - TypeInfoConverter< - typename boost::mpl::deref< - typename boost::mpl::advance< - typename boost::mpl::begin< typename VARIANT::types >::type, - boost::mpl::int_< POS > >::type >::type >{ }); + return TypeInfo(TypeInfoConverter< + typename boost::mpl::deref< + typename boost::mpl::advance< + typename boost::mpl::begin< typename VARIANT::types>::type, + boost::mpl::int_< POS > + >::type + >::type >{ }); } }; @@ -184,34 +185,37 @@ namespace OIC namespace Service { - bool operator==(const ResourceAttributes::Type& lhs, const ResourceAttributes::Type& rhs) + ResourceAttributes::Value::ComparisonHelper::ComparisonHelper(const Value& v) : + m_valueRef(v) { - return lhs.m_which == rhs.m_which; } - bool operator!=(const ResourceAttributes::Type& lhs, const ResourceAttributes::Type& rhs) + bool ResourceAttributes::Value::ComparisonHelper::operator== + (const Value::ComparisonHelper& rhs) const { - return !(lhs == rhs); + return *m_valueRef.m_data == *rhs.m_valueRef.m_data; } - bool operator!=(const ResourceAttributes::Value& lhs, const ResourceAttributes::Value& rhs) + bool operator==(const ResourceAttributes::Type& lhs, const ResourceAttributes::Type& rhs) { - return !(lhs == rhs); + return lhs.m_which == rhs.m_which; } - bool operator!=(const char* lhs, const ResourceAttributes::Value& rhs) + bool operator!=(const ResourceAttributes::Type& lhs, const ResourceAttributes::Type& rhs) { - return !(rhs == lhs); + return !(lhs == rhs); } - bool operator==(const char* lhs, const ResourceAttributes::Value& rhs) + bool operator==(const ResourceAttributes::Value::ComparisonHelper& lhs, + const ResourceAttributes::Value::ComparisonHelper& rhs) { - return rhs == lhs; + return lhs.operator==(rhs); } - bool operator==(const ResourceAttributes::Value& lhs, const ResourceAttributes::Value& rhs) + bool operator!=(const ResourceAttributes::Value::ComparisonHelper& lhs, + const ResourceAttributes::Value::ComparisonHelper& rhs) { - return *lhs.m_data == *rhs.m_data; + return !lhs.operator==(rhs); } bool operator==(const ResourceAttributes& lhs, const ResourceAttributes& rhs) @@ -275,11 +279,6 @@ namespace OIC return *this; } - bool ResourceAttributes::Value::operator==(const char* rhs) const - { - return equals< std::string >(rhs); - } - auto ResourceAttributes::Value::getType() const -> Type { return boost::apply_visitor(TypeVisitor(), *m_data); diff --git a/service/resource-encapsulation/src/serverBuilder/SConscript b/service/resource-encapsulation/src/serverBuilder/SConscript index 4b93547..ec6bdd4 100644 --- a/service/resource-encapsulation/src/serverBuilder/SConscript +++ b/service/resource-encapsulation/src/serverBuilder/SConscript @@ -107,6 +107,6 @@ server_builder_test_env.PrependUnique(LIBS = [ server_builder_test_src = env.Glob('unittests/*.cpp') -server_builder_test = server_builder_test_env.Program('server_builder_test', server_builder_test_src) -Alias("server_builder_test", server_builder_test) -env.AppendTarget('server_builder_test') +server_builder_test = server_builder_test_env.Program('rcs_server_test', server_builder_test_src) +Alias("rcs_server_test", server_builder_test) +env.AppendTarget('rcs_server_test') diff --git a/service/resource-encapsulation/unittests/ResourceClient_Test.cpp b/service/resource-encapsulation/unittests/ResourceClient_Test.cpp index 80183b6..3372ce1 100644 --- a/service/resource-encapsulation/unittests/ResourceClient_Test.cpp +++ b/service/resource-encapsulation/unittests/ResourceClient_Test.cpp @@ -250,7 +250,7 @@ TEST(ResourceClientTest, testGetCachedAttributesWithoutCallback) { createResource(); ResourceAttributes result = object->getCachedAttributes(); - EXPECT_TRUE(result != NULL); + EXPECT_TRUE(result.empty()); destroyResource(); } @@ -259,8 +259,7 @@ TEST(ResourceClientTest, testGetCachedAttributeWithInvalidAttribute) { createResource(); ResourceAttributes::Value result = object->getCachedAttribute(""); - std::nullptr_t null; - EXPECT_TRUE(result == null); + EXPECT_TRUE(result == nullptr); destroyResource(); } @@ -278,7 +277,7 @@ TEST(ResourceClientTest, testGetUri) { createResource(); std::string result = object->getUri(); - EXPECT_TRUE(result != NULL); + EXPECT_TRUE(!result.empty()); destroyResource(); } @@ -286,7 +285,7 @@ TEST(ResourceClientTest, testGetAddress) { createResource(); std::string result = object->getAddress(); - EXPECT_TRUE(result != NULL); + EXPECT_TRUE(!result.empty()); destroyResource(); } @@ -311,7 +310,7 @@ TEST(ResourceClientTest, testGetCachedAttribute) { createResource(); ResourceAttributes::Value result = object->getCachedAttribute("Temperature"); - EXPECT_TRUE(result != NULL); + EXPECT_TRUE(result != nullptr); destroyResource(); } -- 2.7.4