pdxcodemonkey commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r442566625
##########
File path: cppcache/integration/test/PdxJsonTypeTest.cpp
##########
@@ -142,4 +146,91 @@ TEST(PdxJsonTypeTest, testTwoConsecutiveGets) {
std::dynamic_pointer_cast<PdxInstance>(region2->get("simpleObject")));
}
+TEST(PdxJsonTypeTest, testTwoObjectsWithSameFieldsHaveTheSameHash) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+ cluster.start();
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("REPLICATE")
+ .execute();
+
+ auto cache = cluster.createCache();
+
+ auto properties = std::make_shared<Properties>();
+ CacheImpl cacheImpl(&cache, properties, false, false, nullptr);
+
+ PdxTypeRegistry pdxTypeRegistry(&cacheImpl);
+
+ apache::geode::client::PdxType m_pdxType1(pdxTypeRegistry,
gemfireJsonClassName,false);
+ apache::geode::client::PdxType m_pdxType2(pdxTypeRegistry,
gemfireJsonClassName,false);
+
+ m_pdxType1.addVariableLengthTypeField("bar0", "string",
PdxFieldTypes::STRING);
+ m_pdxType1.addVariableLengthTypeField("bar1", "string",
PdxFieldTypes::STRING);
+
+ m_pdxType2.addVariableLengthTypeField("bar0", "string",
PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar1", "string",
PdxFieldTypes::STRING);
+
+ EXPECT_EQ(std::hash<PdxType>(m_pdxType1),std::hash<PdxType>(m_pdxType2));
+}
+
+TEST(PdxJsonTypeTest, testTwoObjectsWithDifferentFieldsHaveDifferentHash) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+ cluster.start();
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("REPLICATE")
+ .execute();
+
+ auto cache = cluster.createCache();
+
+ auto properties = std::make_shared<Properties>();
+ CacheImpl cacheImpl(&cache, properties, false, false, nullptr);
+
+ PdxTypeRegistry pdxTypeRegistry(&cacheImpl);
+
+ apache::geode::client::PdxType m_pdxType1(pdxTypeRegistry,
gemfireJsonClassName,false);
+ apache::geode::client::PdxType m_pdxType2(pdxTypeRegistry,
gemfireJsonClassName,false);
+
+ m_pdxType1.addVariableLengthTypeField("bar0", "string",
PdxFieldTypes::STRING);
+ m_pdxType1.addVariableLengthTypeField("bar1", "string",
PdxFieldTypes::STRING);
+
+ m_pdxType2.addVariableLengthTypeField("bar2", "string",
PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar3", "string",
PdxFieldTypes::STRING);
+
+ EXPECT_NE(std::hash<PdxType>(m_pdxType1),std::hash<PdxType>(m_pdxType2));
Review comment:
And now I think we see why PdxTests::PdxType existed in the first place.
apache::geode::client::PdxType is hidden in the library. The following fixes
linker errors for me, in cppcache/src/PdxType.hpp:
```
- class PdxType
+ class APACHE_GEODE_EXPORT PdxType
```
But now we need to have the more philosophical question about whether or not
this is the right thing to do. If we _are_ going to export PdxType, the header
needs to move from cppcache/src to cppcache/include/geode. @pivotal-jbarrett
any opinion/suggestion?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
