Skip to content

Commit

Permalink
Add tests for native JSON options. Convert GreyhoundReader to this us…
Browse files Browse the repository at this point in the history 
…age instead of stringification. (#1444)
  • Loading branch information
@connormanning @hobu
connormanning authored and hobu committed Dec 15, 2016
1 parent b41fda6 commit 6b2646a
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 44 deletions.
25 changes: 7 additions & 18 deletions plugins/greyhound/io/GreyhoundReader.cpp
View file
Expand Up @@ -276,35 +276,24 @@ void GreyhoundReader::initialize(PointTableRef table)

m_queryBounds = toBounds(m_queryBox).intersection(m_fullBounds);

if (m_filterArg.size() && !parse(m_filterArg).isNull())
{
m_filterString = write(parse(m_filterArg));
}

if (m_pathsArg.size())
{
Json::Value json;
if (m_filterString.size()) json = parse(m_filterString);

if (m_pathsArg.size() == 1)
{
json["Path"] = m_pathsArg.front();
m_filter["Path"] = m_pathsArg.front();
}
else
{
for (const auto& p : m_pathsArg)
{
json["Path"].append(p);
m_filter["Path"].append(p);
}
}

m_filterString = write(json);
}

if (m_filterString.size())
if (!m_filter.isNull())
{
log()->get(LogLevel::Debug) << "Filter: " << parse(m_filterString) <<
std::endl;
log()->get(LogLevel::Debug) << "Filter: " << m_filter << std::endl;
}

m_dims = schemaToDims(m_info["schema"]);
Expand All @@ -322,7 +311,7 @@ void GreyhoundReader::addArgs(ProgramArgs& args)
args.add("depth_begin", "Beginning depth to query", m_depthBeginArg, 0u);
args.add("depth_end", "Ending depth to query", m_depthEndArg, 0u);
args.add("tile_path", "Index-optimized tile selection", m_pathsArg);
args.add("filter", "Query filter", m_filterArg);
args.add("filter", "Query filter", m_filter);
args.add("threads", "Number of threads for HTTP requests", m_threadsArg, 4);
}

Expand Down Expand Up @@ -650,9 +639,9 @@ point_count_t GreyhoundReader::fetchData(
#ifdef PDAL_HAVE_LAZPERF
url << "&compress=true";
#endif
if (m_filterString.size())
if (!m_filter.isNull())
{
url << "&filter=" << arbiter::http::sanitize(m_filterString);
url << "&filter=" << arbiter::http::sanitize(write(m_filter));
}

{
Expand Down
3 changes: 1 addition & 2 deletions plugins/greyhound/io/GreyhoundReader.hpp
View file
Expand Up @@ -77,7 +77,6 @@ class PDAL_DLL GreyhoundReader : public pdal::Reader
std::size_t m_sparseDepth;
Json::Value m_info;
Json::Value m_schema;
std::string m_filterString;
std::unique_ptr<greyhound::Point> m_scale;
std::unique_ptr<greyhound::Point> m_offset;

Expand All @@ -100,7 +99,7 @@ class PDAL_DLL GreyhoundReader : public pdal::Reader
uint32_t m_depthBeginArg;
uint32_t m_depthEndArg;
std::vector<std::string> m_pathsArg;
std::string m_filterArg;
Json::Value m_filter;
int32_t m_threadsArg;

virtual void initialize(PointTableRef table) override;
Expand Down
68 changes: 49 additions & 19 deletions plugins/greyhound/test/GreyhoundReaderTest.cpp
View file
Expand Up @@ -164,27 +164,57 @@ TEST_F(GreyhoundReaderTest, filter)
{
if (!doTests()) return;

pdal::GreyhoundReader reader;
auto options(greyhoundOptions(&stadiumBounds, 0, 12));
options.add("filter", "{ \"Z\": { \"$gte\": 500 } }");

reader.setOptions(options);

pdal::PointTable table;
reader.prepare(table);
PointViewSet viewSet = reader.execute(table);
PointViewPtr view = *viewSet.begin();
ASSERT_LT(view->size(), 188260u);

greyhound::Point p;
for (std::size_t i(0); i < view->size(); ++i)
{
p.x = view->getFieldAs<double>(Dimension::Id::X, i);
p.y = view->getFieldAs<double>(Dimension::Id::Y, i);
p.z = view->getFieldAs<double>(Dimension::Id::Z, i);
pdal::GreyhoundReader reader;
auto options(greyhoundOptions(&stadiumBounds, 0, 12));
options.add("filter", "{ \"Z\": { \"$gte\": 500 } }");

reader.setOptions(options);

pdal::PointTable table;
reader.prepare(table);
PointViewSet viewSet = reader.execute(table);
PointViewPtr view = *viewSet.begin();
ASSERT_LT(view->size(), 188260u);

greyhound::Point p;
for (std::size_t i(0); i < view->size(); ++i)
{
p.x = view->getFieldAs<double>(Dimension::Id::X, i);
p.y = view->getFieldAs<double>(Dimension::Id::Y, i);
p.z = view->getFieldAs<double>(Dimension::Id::Z, i);

ASSERT_TRUE(stadiumBounds.contains(p));
ASSERT_GE(p.z, 500);
}
}

ASSERT_TRUE(stadiumBounds.contains(p));
ASSERT_GE(p.z, 500);
{
pdal::GreyhoundReader reader;
auto options(greyhoundOptions(&stadiumBounds, 0, 12));

Json::Value filter;
filter["Z"]["$gte"] = 500;
options.add("filter", filter);

reader.setOptions(options);

pdal::PointTable table;
reader.prepare(table);
PointViewSet viewSet = reader.execute(table);
PointViewPtr view = *viewSet.begin();
ASSERT_LT(view->size(), 188260u);

greyhound::Point p;
for (std::size_t i(0); i < view->size(); ++i)
{
p.x = view->getFieldAs<double>(Dimension::Id::X, i);
p.y = view->getFieldAs<double>(Dimension::Id::Y, i);
p.z = view->getFieldAs<double>(Dimension::Id::Z, i);

ASSERT_TRUE(stadiumBounds.contains(p));
ASSERT_GE(p.z, 500);
}
}
}

Expand Down
25 changes: 20 additions & 5 deletions test/unit/OptionsTest.cpp
View file
Expand Up @@ -34,6 +34,8 @@

#include <pdal/pdal_test_main.hpp>

#include <json/json.h>

#include <pdal/Options.hpp>
#include <pdal/PDALUtils.hpp>
#include <io/FauxReader.hpp>
Expand All @@ -53,11 +55,6 @@ namespace pdal

TEST(OptionsTest, test_option_writing)
{
std::ostringstream ostr_i;
const std::string ref_i = xml_header + xml_int_ref;
std::ostringstream ostr_s;
const std::string ref_s = xml_header + xml_str_ref;

const Option option_i("my_int", (uint16_t)17);
EXPECT_TRUE(option_i.getName() == "my_int");
EXPECT_TRUE(option_i.getValue() == "17");
Expand All @@ -69,6 +66,24 @@ TEST(OptionsTest, test_option_writing)
}


TEST(OptionsTest, json)
{
// Test that a JSON option will be stringified into the option's underlying
// value.
Json::Value inJson;
inJson["key"] = 42;
const Option option_j("my_json", inJson);
EXPECT_TRUE(option_j.getName() == "my_json");

// Don't string-compare, test JSON-equality, since we don't care exactly
// how it's stringified.
Json::Value outJson;
Json::Reader().parse(option_j.getValue(), outJson);

EXPECT_EQ(inJson, outJson) << inJson << " != " << outJson;
}


TEST(OptionsTest, conditional)
{
CropFilter s;
Expand Down
100 changes: 100 additions & 0 deletions test/unit/ProgramArgsTest.cpp
View file
Expand Up @@ -34,6 +34,8 @@

#include <pdal/pdal_test_main.hpp>

#include <json/json.h>

// No wordexp() on windows and I don't feel like doing something special.
#ifndef _WIN32

Expand Down Expand Up @@ -427,4 +429,102 @@ TEST(ProgramArgsTest, parseSimple)
EXPECT_THROW(args.parse(s), arg_error);
}

TEST(ProgramArgsTest, json)
{
// Test JSON arguments. If the arg refers to an underlying Json::Value,
// it should be parsed into that Json::Value. If a string, then that
// string should contain the stringified JSON (which should be parsable
// into equivalent JSON).
Json::Value m_json;
std::string m_string;
Json::Value verify;
verify["key"] = 42;

const std::string stringified("\"{ \\\"key\\\": 42 }\"");

ProgramArgs args;
args.add("json,j", "JSON description", m_json);
args.add("string,s", "String description", m_string);

StringList s;
Json::Reader reader;

// An unset Json::Value should be null.
{
s = toStringList("");
EXPECT_NO_THROW(args.parse(s));
EXPECT_TRUE(m_json.isNull());
}

// Test an object.
{
StringList s = toStringList(
"--json " + stringified + " --string " + stringified);
EXPECT_NO_THROW(args.parse(s));

// Verify the JSON value.
EXPECT_EQ(m_json, verify) << m_json << " != " << verify;

// Verify that the string value parses into the proper JSON value.
Json::Value fromString;
EXPECT_TRUE(reader.parse(m_string, fromString));
EXPECT_EQ(fromString, verify) << fromString << " != " << verify;
}

// Test non-objects.
{
args.reset();
s = toStringList("--json [1,2,3]");
EXPECT_NO_THROW(args.parse(s));
ASSERT_TRUE(reader.parse("[1, 2, 3]", verify));
EXPECT_EQ(m_json, verify);

args.reset();
s = toStringList("--json 2");
EXPECT_NO_THROW(args.parse(s));
verify = 2;
EXPECT_TRUE(m_json.isNumeric());
EXPECT_EQ(m_json.asInt(), 2);

args.reset();
s = toStringList("--json 3.14");
EXPECT_NO_THROW(args.parse(s));
verify = 3.14;
EXPECT_TRUE(m_json.isDouble());
EXPECT_EQ(m_json.asDouble(), 3.14);
}
}

TEST(ProgramArgsTest, invalidJson)
{
Json::Value m_json;
std::string m_string;
Json::Value verify;
verify["key"] = 42;

ProgramArgs args;
args.add("json,j", "JSON description", m_json);
args.add("string,s", "String description", m_string);

// If the underlying is a string, then invalid JSON is fine during argument
// parsing, but will fail later if parsed as JSON.
args.reset();
StringList s = toStringList("--string \"{ invalid JSON here }\"");
EXPECT_NO_THROW(args.parse(s));

// The string was successfully parsed, but contains invalid JSON.
Json::Value invalidParse;
EXPECT_FALSE(Json::Reader().parse(m_string, invalidParse));

// If the underlying is a Json::Value, then argument parsing should fail
// up front.
args.reset();
s = toStringList("--json \"{ invalid JSON here }\"");
EXPECT_ANY_THROW(args.parse(s));

args.reset();
s = toStringList("--json");
EXPECT_ANY_THROW(args.parse(s));
}

#endif

0 comments on commit 6b2646a

Please sign in to comment.