QPID-6470: FieldValue::getFloatingPointValue() converts endian each time it is called.

When calling getFloatingPointValue multiple times, the octets are endian-converted each time.
Actually we need to make a copy first and then call convertIfRequired().

This fix is from a pull request by Pavel Pokutnev (see the JIRA).
commit 4ed0ce9c9b74b136c49735b19efb80489aa495a3

His original patch was correct, I made some additions:

- Added a unit test: qpid/cpp/src/tests/FieldValue.cpp
- Fixed some incorrect uses of "const" in nearby code.
- Replaced a for loop with std::copy, more readable and more optimizable.

There are still serious problems with float conversion shown up by the unit tests,
the relevant tests are commented out till these issues are fixed.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1671125 13f79535-47bb-0310-9956-ffa450edef68

1 files changed, 36 insertions, 34 deletions

diff --git a/qpid/cpp/src/tests/FieldValue.cpp b/qpid/cpp/src/tests/FieldValue.cpp
index 0ebd0d7d44..34faee2953 100644
--- a/qpid/cpp/src/tests/FieldValue.cpp
+++ b/qpid/cpp/src/tests/FieldValue.cpp
@@ -2,7 +2,7 @@
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
+ uni* regarding copyright ownership.  The ASF licenses this file
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
@@ -19,6 +19,7 @@
 #include "qpid/framing/FieldValue.h"
 
 #include "unit_test.h"
+#include <boost/test/floating_point_comparison.hpp>
 
 namespace qpid {
 namespace tests {
@@ -29,9 +30,8 @@ using namespace qpid::framing;
 
 Str16Value s("abc");
 IntegerValue i(42);
-//DecimalValue d(1234,2);
-//FieldTableValue ft;
-//EmptyValue e;
+FloatValue f(42.42);
+DoubleValue df(123.123);
 
 QPID_AUTO_TEST_CASE(testStr16ValueEquals)
 {
@@ -43,52 +43,54 @@ QPID_AUTO_TEST_CASE(testStr16ValueEquals)
     BOOST_CHECK(s.convertsTo<int>() == false);
     BOOST_CHECK(s.get<std::string>() == "abc");
     BOOST_CHECK_THROW(s.get<int>(), InvalidConversionException);
-//    BOOST_CHECK(s != ft);
 
 }
 
 QPID_AUTO_TEST_CASE(testIntegerValueEquals)
 {
+    BOOST_CHECK(i.get<int>() == 42);
     BOOST_CHECK(IntegerValue(42) == i);
     BOOST_CHECK(IntegerValue(5) != i);
     BOOST_CHECK(i != s);
     BOOST_CHECK(i.convertsTo<std::string>() == false);
+    BOOST_CHECK(i.convertsTo<float>() == false);
     BOOST_CHECK(i.convertsTo<int>() == true);
     BOOST_CHECK_THROW(i.get<std::string>(), InvalidConversionException);
-    BOOST_CHECK(i.get<int>() == 42);
-//    BOOST_CHECK(i != ft);
-}
 
-#if 0
-QPID_AUTO_TEST_CASE(testDecimalValueEquals)
-{
-    BOOST_CHECK(DecimalValue(1234, 2) == d);
-    BOOST_CHECK(DecimalValue(12345, 2) != d);
-    BOOST_CHECK(DecimalValue(1234, 3) != d);
-    BOOST_CHECK(d != s);
+    //FIXME aconway 2015-04-03: fails
+    //BOOST_CHECK_THROW(i.get<float>(), InvalidConversionException);
 }
 
-QPID_AUTO_TEST_CASE(testFieldTableValueEquals)
+QPID_AUTO_TEST_CASE(testFloatValueEquals)
 {
-    ft.getValue().setString("foo", "FOO");
-    ft.getValue().setInt("magic", 7);
-
-    BOOST_CHECK_EQUAL(std::string("FOO"),
-                            ft.getValue().getString("foo"));
-    BOOST_CHECK_EQUAL(7, ft.getValue().getInt("magic"));
-
-    FieldTableValue f2;
-    BOOST_CHECK(ft != f2);
-    f2.getValue().setString("foo", "FOO");
-    BOOST_CHECK(ft != f2);
-    f2.getValue().setInt("magic", 7);
-    BOOST_CHECK_EQUAL(ft,f2);
-    BOOST_CHECK(ft == f2);
-    f2.getValue().setString("foo", "BAR");
-    BOOST_CHECK(ft != f2);
-    BOOST_CHECK(ft != i);
+    // FIXME aconway 2015-04-03: The commented out tests are bug QPID-6470.
+    // The basic problems are:
+    // - allows meaningles conversion between int and float types.
+    // - does not allow expected conversion between float and double types.
+
+    // BOOST_CHECK(f.convertsTo<float>() == true);
+    BOOST_CHECK_EQUAL(FloatValue(42.42), f);
+    BOOST_CHECK_CLOSE(f.get<float>(), 42.42, 0.001);
+    // Check twice, regression test for QPID-6470 where the value was corrupted during get.
+    BOOST_CHECK_EQUAL(FloatValue(42.42), f);
+    BOOST_CHECK_CLOSE(f.get<float>(), 42.42, 0.001);
+
+    // Float to double conversion
+    // BOOST_CHECK(f.convertsTo<double>() == true);
+    // BOOST_CHECK_CLOSE(f.get<double>(), 42.42, 0.001); 
+
+    // Double value
+    BOOST_CHECK_CLOSE(df.get<double>(), 123.123, 0.001);
+    // BOOST_CHECK(f.convertsTo<float>() == true);
+    // BOOST_CHECK(f.convertsTo<double>() == true);
+    // BOOST_CHECK_CLOSE(df.get<float>(), 123.123, 0.001);
+
+    // Invalid conversions should fail.
+    BOOST_CHECK(!f.convertsTo<std::string>());
+    // BOOST_CHECK(!f.convertsTo<int>());
+    BOOST_CHECK_THROW(f.get<std::string>(), InvalidConversionException);
+    // BOOST_CHECK_THROW(f.get<int>(), InvalidConversionException);
 }
-#endif
 
 QPID_AUTO_TEST_SUITE_END()