Skip to content

Commit db99caf

Browse files
abellgithubhobu
abellgithub
and
hobu
authored
Move JSON out of FileSpec header (PDAL#4728)
* Remove private functions from stac reader header. * Move interface functions out of EsriReader. * Make writeHierarchy a non-member. * Add missed file. * Move functions/data to Private. * Remove handleReaderArgs() and setReaderOptions() from header. * Move functions to cpp file. * Remove JSON from public FileSpec. * Add missed file. * Another missed file. * Use correct export tag. * Remove dup parseJson. * no-op JsonFwd.hpp * Revert "no-op JsonFwd.hpp" This reverts commit 54dcc42. --------- Co-authored-by: Howard Butler <[email protected]>
1 parent 86d027b commit db99caf

File tree

13 files changed

+297
-98
lines changed

13 files changed

+297
-98
lines changed

‎io/private/connector/Connector.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,6 @@
3939
#include <curl/curl.h>
4040
#include <pdal/util/FileUtils.hpp>
4141

42-
43-
44-
45-
4642
namespace pdal
4743
{
4844
namespace connector
@@ -97,9 +93,9 @@ Connector::Connector(const std::string& filename, const StringMap& headers,
9793

9894
Connector::Connector(const FileSpec& spec) :
9995
m_arbiter(new arbiter::Arbiter),
100-
m_headers(spec.m_headers),
101-
m_query(spec.m_query),
102-
m_filename(spec.m_path.string())
96+
m_headers(spec.headers()),
97+
m_query(spec.query()),
98+
m_filename(spec.filePath().string())
10399
{
104100
if (m_headers.find("User-Agent") == m_headers.end())
105101
{

‎pdal/FileSpec.cpp

Lines changed: 122 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,24 @@
3636

3737
#include <nlohmann/json.hpp>
3838

39+
#include <pdal/util/private/JsonSupport.hpp>
40+
#include <pdal/private/FileSpecHelper.hpp>
41+
3942
namespace pdal
4043
{
4144

45+
struct FileSpec::Private
46+
{
47+
std::filesystem::path m_path;
48+
StringMap m_headers;
49+
StringMap m_query;
50+
51+
Utils::StatusWithReason parse(NL::json& node);
52+
Utils::StatusWithReason extractPath(NL::json& node);
53+
Utils::StatusWithReason extractHeaders(NL::json& node);
54+
Utils::StatusWithReason extractQuery(NL::json& node);
55+
};
56+
4257
namespace
4358
{
4459

@@ -58,12 +73,94 @@ bool extractStringMap(NL::json& node, StringMap& map)
5873

5974
} // unnamed namespace
6075

76+
FileSpec::FileSpec() : m_p(new Private)
77+
{}
78+
79+
FileSpec::FileSpec(const std::string& pathOrJson) : m_p(new Private)
80+
{
81+
(void)ingest(pathOrJson);
82+
}
83+
84+
FileSpec::~FileSpec()
85+
{}
86+
87+
FileSpec::FileSpec(const FileSpec& other) : m_p(new Private)
88+
{
89+
*m_p = *other.m_p;
90+
}
91+
92+
FileSpec& FileSpec::operator=(const FileSpec& other)
93+
{
94+
*m_p = *other.m_p;
95+
return *this;
96+
}
97+
98+
FileSpec::FileSpec(FileSpec&& other)
99+
{
100+
m_p = std::move(other.m_p);
101+
}
102+
103+
FileSpec& FileSpec::operator=(FileSpec&& other)
104+
{
105+
m_p = std::move(other.m_p);
106+
return *this;
107+
}
108+
109+
bool FileSpec::valid() const
110+
{
111+
return !m_p->m_path.empty();
112+
}
113+
114+
bool FileSpec::onlyFilename() const
115+
{
116+
return m_p->m_headers.empty() && m_p->m_query.empty();
117+
}
118+
119+
std::filesystem::path FileSpec::filePath() const
120+
{
121+
return m_p->m_path;
122+
}
123+
124+
void FileSpec::setFilePath(const std::string& path)
125+
{
126+
m_p->m_path = path;
127+
}
128+
129+
void FileSpec::setFilePath(const std::filesystem::path& path)
130+
{
131+
m_p->m_path = path;
132+
}
133+
134+
StringMap FileSpec::query() const
135+
{
136+
return m_p->m_query;
137+
}
138+
139+
StringMap FileSpec::headers() const
140+
{
141+
return m_p->m_headers;
142+
}
61143

62144
Utils::StatusWithReason FileSpec::ingest(const std::string& pathOrJson)
63145
{
146+
auto isJson = [](const std::string& s) -> bool
147+
{
148+
static constexpr std::array<std::pair<char, char>, 3> delims
149+
{ { { '{', '}' }, { '[', ']' }, { '"', '"' } } };
150+
151+
std::string t = s;
152+
Utils::trim(t);
153+
154+
if (t.size() < 2)
155+
return false;
156+
for (const std::pair<char, char>& d : delims)
157+
if (t.front() == d.first && t.back() == d.second)
158+
return true;
159+
return false;
160+
};
161+
64162
NL::json json;
65-
size_t pos = Utils::extractSpaces(pathOrJson, 0);
66-
if (pathOrJson[pos] == '{' || pathOrJson[pos] == '[')
163+
if (isJson(pathOrJson))
67164
{
68165
auto status = Utils::parseJson(pathOrJson, json);
69166
if (!status)
@@ -73,10 +170,10 @@ Utils::StatusWithReason FileSpec::ingest(const std::string& pathOrJson)
73170
else
74171
json = NL::json(pathOrJson);
75172

76-
return parse(json);
173+
return m_p->parse(json);
77174
}
78175

79-
Utils::StatusWithReason FileSpec::parse(NL::json& node)
176+
Utils::StatusWithReason FileSpec::Private::parse(NL::json& node)
80177
{
81178
if (node.is_null())
82179
return { -1, "'filename' argument contains no data" };
@@ -101,7 +198,8 @@ Utils::StatusWithReason FileSpec::parse(NL::json& node)
101198
return true;
102199
}
103200

104-
Utils::StatusWithReason FileSpec::extractPath(NL::json& node)
201+
202+
Utils::StatusWithReason FileSpec::Private::extractPath(NL::json& node)
105203
{
106204
auto it = node.find("path");
107205
if (it == node.end())
@@ -118,50 +216,55 @@ Utils::StatusWithReason FileSpec::extractPath(NL::json& node)
118216
return true;
119217
}
120218

121-
Utils::StatusWithReason FileSpec::extractQuery(NL::json& node)
219+
Utils::StatusWithReason FileSpec::Private::extractHeaders(NL::json& node)
122220
{
123-
auto it = node.find("query");
221+
auto it = node.find("headers");
124222
if (it == node.end())
125223
return true;
126224
NL::json& val = *it;
127225
if (!val.is_null())
128226
{
129-
if (!extractStringMap(val, m_query))
130-
return { -1, "'filename' sub-argument 'query' must be an object of "
227+
if (!extractStringMap(val, m_headers))
228+
return { -1, "'filename' sub-argument 'headers' must be an object of "
131229
"string key-value pairs." };
132230
}
133231
node.erase(it);
134232
return true;
135233
}
136234

137-
Utils::StatusWithReason FileSpec::extractHeaders(NL::json& node)
235+
Utils::StatusWithReason FileSpec::Private::extractQuery(NL::json& node)
138236
{
139-
auto it = node.find("headers");
237+
auto it = node.find("query");
140238
if (it == node.end())
141239
return true;
142240
NL::json& val = *it;
143241
if (!val.is_null())
144242
{
145-
if (!extractStringMap(val, m_headers))
146-
return { -1, "'filename' sub-argument 'headers' must be an object of "
243+
if (!extractStringMap(val, m_query))
244+
return { -1, "'filename' sub-argument 'query' must be an object of "
147245
"string key-value pairs." };
148246
}
149247
node.erase(it);
150248
return true;
151249
}
152250

251+
// Provide access to the private 'parse' function.
252+
Utils::StatusWithReason FileSpecHelper::parse(FileSpec& spec, NL::json& node)
253+
{
254+
return spec.m_p->parse(node);
255+
}
256+
153257
std::ostream& operator << (std::ostream& out, const FileSpec& spec)
154258
{
155259
NL::json json;
156-
json["path"] = spec.m_path.string();
157-
if (!spec.m_headers.empty())
158-
json["headers"] = spec.m_headers;
159-
if (!spec.m_query.empty())
160-
json["query"] = spec.m_query;
260+
json["path"] = spec.m_p->m_path.string();
261+
if (!spec.m_p->m_headers.empty())
262+
json["headers"] = spec.m_p->m_headers;
263+
if (!spec.m_p->m_query.empty())
264+
json["query"] = spec.m_p->m_query;
161265

162266
out << json;
163267
return out;
164268
}
165269

166-
167270
} // namespace pdal

‎pdal/FileSpec.hpp

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -35,47 +35,48 @@
3535

3636
#include <filesystem>
3737

38-
#include <pdal/PDALUtils.hpp>
38+
#include <pdal/pdal_export.hpp>
3939
#include <pdal/pdal_types.hpp>
4040

41-
#include <pdal/JsonFwd.hpp>
42-
43-
using StringMap = std::map<std::string, std::string>;
44-
4541
namespace pdal
4642
{
4743

48-
class FileSpec
44+
class PDAL_EXPORT FileSpec
4945
{
46+
friend class FileSpecHelper;
47+
5048
public:
51-
FileSpec()
52-
{}
49+
FileSpec();
50+
FileSpec(const std::string& pathOrJson);
51+
~FileSpec();
52+
53+
FileSpec(const FileSpec&);
54+
FileSpec& operator=(const FileSpec&);
55+
FileSpec(FileSpec&&);
56+
FileSpec& operator=(FileSpec&&);
57+
58+
bool valid() const;
59+
bool onlyFilename() const;
60+
std::filesystem::path filePath() const;
61+
StringMap query() const;
62+
StringMap headers() const;
63+
void setFilePath(const std::string& path);
64+
void setFilePath(const std::filesystem::path& path);
5365

54-
bool valid() const
55-
{ return !m_path.empty(); }
56-
bool onlyFilename() const
57-
{ return m_headers.empty() && m_query.empty(); }
58-
Utils::StatusWithReason parse(NL::json& json);
5966
// parse a user input string that could be a json spec or filename
6067
Utils::StatusWithReason ingest(const std::string& pathOrJson);
6168

6269
friend std::ostream& operator << (std::ostream& out, const FileSpec& spec);
6370

6471
private:
65-
Utils::StatusWithReason extractPath(NL::json& node);
66-
Utils::StatusWithReason extractQuery(NL::json& node);
67-
Utils::StatusWithReason extractHeaders(NL::json& node);
68-
69-
public:
70-
std::filesystem::path m_path;
71-
StringMap m_headers;
72-
StringMap m_query;
72+
struct Private;
73+
std::unique_ptr<Private> m_p;
7374
};
7475

7576
namespace Utils
7677
{
7778
template<>
78-
inline StatusWithReason fromString(const std::string& s, FileSpec& spec)
79+
PDAL_EXPORT inline StatusWithReason fromString(const std::string& s, FileSpec& spec)
7980
{
8081
return spec.ingest(s);
8182
}

‎pdal/PDALUtils.cpp

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -562,24 +562,5 @@ std::vector<std::string> glob(const std::string& path)
562562
return FileUtils::glob(path);
563563
}
564564

565-
StatusWithReason parseJson(const std::string& s, NL::json& json)
566-
{
567-
try
568-
{
569-
json = NL::json::parse(s);
570-
}
571-
catch (NL::json::parse_error& err)
572-
{
573-
// Look for a right bracket -- this indicates the start of the
574-
// actual message from the parse error.
575-
std::string s(err.what());
576-
auto pos = s.find(']');
577-
if (pos != std::string::npos)
578-
s = s.substr(pos + 1);
579-
return { -1, s };
580-
}
581-
return true;
582-
}
583-
584565
} // namespace Utils
585566
} // namespace pdal

‎pdal/PDALUtils.hpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636

3737
#include <pdal/Metadata.hpp>
3838
#include <pdal/Dimension.hpp>
39-
#include <pdal/JsonFwd.hpp>
4039
#include <pdal/pdal_export.hpp>
4140
#include <pdal/util/Bounds.hpp>
4241
#include <pdal/util/Inserter.hpp>
@@ -47,7 +46,6 @@
4746
#include <unistd.h>
4847
#endif
4948

50-
5149
namespace pdal
5250
{
5351
class Options;
@@ -281,7 +279,6 @@ std::pair<double, double> PDAL_EXPORT computeHausdorffPair(PointViewPtr srcView,
281279
double PDAL_EXPORT computeChamfer(PointViewPtr srcView, PointViewPtr candView);
282280
std::string PDAL_EXPORT tempFilename(const std::string& path);
283281
std::vector<std::string> PDAL_EXPORT glob(const std::string& path);
284-
StatusWithReason PDAL_EXPORT parseJson(const std::string& s, NL::json& json);
285282

286283
} // namespace Utils
287284
} // namespace pdal

0 commit comments

Comments
 (0)