Merge lp:~zorba-coders/zorba/csv-fixes into lp:zorba

Proposed by Paul J. Lucas on 2014-01-16
Status: Merged
Approved by: Paul J. Lucas on 2014-01-16
Approved revision: 11411
Merged at revision: 11687
Proposed branch: lp:~zorba-coders/zorba/csv-fixes
Merge into: lp:zorba
Diff against target: 438 lines (+134/-45)
9 files modified
NOTICE.txt (+2/-1)
include/zorba/internal/ztd.h (+17/-13)
include/zorba/pregenerated/diagnostic_list.h (+2/-0)
modules/json/json-csv.jq (+13/-3)
src/diagnostics/diagnostic_en.xml (+12/-3)
src/diagnostics/pregenerated/diagnostic_list.cpp (+3/-0)
src/diagnostics/pregenerated/dict_en.cpp (+3/-0)
src/diagnostics/pregenerated/dict_zed_keys.h (+2/-0)
src/runtime/csv/csv_impl.cpp (+80/-25)
To merge this branch: bzr merge lp:~zorba-coders/zorba/csv-fixes
Reviewer Review Type Date Requested Status
Matthias Brantner 2014-01-16 Approve on 2014-01-16
Paul J. Lucas Approve on 2014-01-16
Review via email: mp+202019@code.launchpad.net

Commit message

1. Now reporting error location for objects that are not convertible to string.
2. Now checking to ensure field-names is an array.
3. Better error messages.
4. Fixed some comments.

Description of the change

1. Now reporting error location for objects that are not convertible to string.
2. Now checking to ensure field-names is an array.
3. Better error messages.
4. Fixed some comments.

To post a comment you must log in.
Paul J. Lucas (paul-lucas) :
review: Approve
Zorba Build Bot (zorba-buildbot) wrote :

Validation queue starting for the following merge proposals:
https://code.launchpad.net/~zorba-coders/zorba/csv-fixes/+merge/202019

Progress dashboard at http://jenkins.lambda.nu/view/ValidationQueue

review: Approve
Zorba Build Bot (zorba-buildbot) wrote :

Validation queue succeeded - proposal merged!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NOTICE.txt'
2--- NOTICE.txt 2014-01-10 02:36:58 +0000
3+++ NOTICE.txt 2014-01-16 23:23:15 +0000
4@@ -482,7 +482,8 @@
5 Copyright: 2000 D. J. Bernstein
6 Website: http://cr.yp.to/ftpparse.html
7
8- Commercial use is fine, if you let me know what programs you're using this in.
9+ Commercial use is fine, if you let me know what programs you're
10+ using this in.
11
12
13 External libraries used by this project:
14
15=== modified file 'include/zorba/internal/ztd.h'
16--- include/zorba/internal/ztd.h 2013-12-14 00:45:54 +0000
17+++ include/zorba/internal/ztd.h 2014-01-16 23:23:15 +0000
18@@ -92,8 +92,9 @@
19 typedef char yes[2];
20
21 /**
22- * This dummy class is used to make the matching of the dummy operator<<()
23- * \e worse than the global \c operator<<(), if any.
24+ * This dummy class is used to make the matching of the dummy
25+ * \c operator&lt;&lt;() \e worse than the global \c operator&lt;&lt;(),
26+ * if any.
27 */
28 struct any_t {
29 template<typename T> any_t( T const& );
30@@ -101,23 +102,24 @@
31
32 /**
33 * This dummy operator is matched only when there is \e no global
34- * operator<<() otherwise declared for type \c T.
35+ * \c operator&lt;&lt;() otherwise declared for type \c T.
36 *
37 * @return Returns a \c no that selects defined(no).
38 */
39 no operator<<( std::ostream const&, any_t const& );
40
41 /**
42- * This function is matched only when there \e is a global \c operator<<()
43- * declared for type \c T because \c operator<<()'s return type is
44- * \c std::ostream&.
45+ * This function is matched only when there \e is a global
46+ * \c operator&lt;&lt;() declared for type \c T because
47+ * \c operator&lt;&lt;()'s return type is \c std::ostream&.
48 *
49 * @return Returns a yes& whose \c sizeof() equals \c sizeof(yes).
50 */
51 yes& defined( std::ostream& );
52
53 /**
54- * This function is matched only when the dummy \c operator<<() is matched.
55+ * This function is matched only when the dummy \c operator&lt;&lt;() is
56+ * matched.
57 *
58 * @return Returns a no whose \c sizeof() equals \c sizeof(no).
59 */
60@@ -125,8 +127,10 @@
61
62 /**
63 * The implementation class that can be used to determine whether a given
64- * type \c T has a global <code>std::ostream& operator<<(std::ostream&,T
65- * const&)</code> defined for it. However, do not use this class directly.
66+ * type \c T has a global
67+ * <code>std::ostream& operator&lt;&lt;(std::ostream&,T const&)</code>
68+ * defined for it.
69+ * However, do not use this class directly.
70 *
71 * @tparam T The type to check.
72 */
73@@ -136,8 +140,8 @@
74 static T const &t;
75 public:
76 /**
77- * This is \c true only when the type \c T has a global \c operator<<()
78- * declared for it.
79+ * This is \c true only when the type \c T has a global
80+ * \c operator&lt;&lt;() declared for it.
81 * \hideinitializer
82 */
83 static bool const value = sizeof( defined( s << t ) ) == sizeof( yes );
84@@ -147,8 +151,8 @@
85 /**
86 * \internal
87 * A class that can be used to determine whether a given type \c T has a global
88- * <code>std::ostream& operator<<(std::ostream&,T const&)</code> defined for
89- * it.
90+ * <code>std::ostream& operator&lt;&lt;(std::ostream&,T const&)</code> defined
91+ * for it.
92 * For example:
93 * \code
94 * template<typename T> inline
95
96=== modified file 'include/zorba/pregenerated/diagnostic_list.h'
97--- include/zorba/pregenerated/diagnostic_list.h 2013-12-18 06:15:17 +0000
98+++ include/zorba/pregenerated/diagnostic_list.h 2014-01-16 23:23:15 +0000
99@@ -1001,6 +1001,8 @@
100
101 extern ZORBA_DLL_PUBLIC ZorbaCSVErrorCode INVALID_OPTION;
102
103+extern ZORBA_DLL_PUBLIC ZorbaCSVErrorCode INVALID_CSV_VALUE;
104+
105 extern ZORBA_DLL_PUBLIC ZorbaCSVErrorCode MISSING_VALUE;
106
107 extern ZORBA_DLL_PUBLIC ZorbaCSVErrorCode EXTRA_VALUE;
108
109=== modified file 'modules/json/json-csv.jq'
110--- modules/json/json-csv.jq 2013-09-26 23:15:11 +0000
111+++ modules/json/json-csv.jq 2014-01-16 23:23:15 +0000
112@@ -134,9 +134,14 @@
113 : </dl>
114 : @return a sequence of zero or more JSON objects where each key is a field
115 : name and each value is a parsed value.
116- : @error csv:INVALID_OPTION if the <code>quote-char</code>,
117- : <code>quote-escape</code>, or <code>separator</code> option is given
118- : and it's not a single ASCII character.
119+ : @error csv:INVALID_OPTION if a value of <code>field-names</code> is not an
120+ : array or one of the array values is not a string;
121+ : if the <code>quote-char</code>, <code>quote-escape</code>, or
122+ : <code>separator</code> option is given and it's not a single ASCII
123+ : character.
124+ : @error csv:INVALID_CSV_VALUE if a value of some key is not castable to
125+ : string.
126+ : <code>field-names</code> option is not a string.
127 : @error csv:MISSING_VALUE if a missing value is detected and the
128 : <code>missing-value</code> option is "<code>error</code>".
129 : @error csv:EXTRA_VALUE if an extra value is detected and the
130@@ -294,6 +299,11 @@
131 : </dl>
132 : @return a sequence of strings where each string corresponds to a JSON
133 : object, aka, "record."
134+ : @error csv:INVALID_OPTION if a value of <code>field-names</code> is not an
135+ : array or one of the array values is not a string;
136+ : if the <code>quote-char</code>, <code>quote-escape</code>, or
137+ : <code>separator</code> option is given and it's not a single ASCII
138+ : character.
139 :)
140 declare function csv:serialize( $obj as object()*, $options as object() )
141 as string* external;
142
143=== modified file 'src/diagnostics/diagnostic_en.xml'
144--- src/diagnostics/diagnostic_en.xml 2013-12-18 06:15:17 +0000
145+++ src/diagnostics/diagnostic_en.xml 2014-01-16 23:23:15 +0000
146@@ -3777,6 +3777,9 @@
147
148 <diagnostic code="INVALID_OPTION">
149 <value>${"1": }invalid value for "$2" option${; 3}</value>
150+ <entry key="MustBeArray">
151+ <value>must be JSON array</value>
152+ </entry>
153 <entry key="MustBeASCIIChar">
154 <value>must be single ASCII character</value>
155 </entry>
156@@ -3789,6 +3792,13 @@
157 <entry key="MustBeString">
158 <value>must be string</value>
159 </entry>
160+ <entry key="ArrayElementsMustBeString">
161+ <value>array elements must be string</value>
162+ </entry>
163+ </diagnostic>
164+
165+ <diagnostic code="INVALID_CSV_VALUE">
166+ <value>"$1": invalid type for value of key "$2"</value>
167 </diagnostic>
168
169 <diagnostic code="MISSING_VALUE">
170@@ -3807,7 +3817,7 @@
171
172 </namespace>
173
174- <!--////////// dateTime Module Errors //////////////////////////////////////////-->
175+ <!--////////// dateTime Module Errors ////////////////////////////////////-->
176
177 <namespace prefix="dt">
178 <diagnostic code="INVALID_SPECIFICATION">
179@@ -3843,7 +3853,7 @@
180 </diagnostic>
181 </namespace>
182
183- <!-- /////////// URI procesing errors ///////////////////////////////// -->
184+ <!-- /////////// URI procesing errors /////////////////////////////////// -->
185
186 <namespace prefix="zuri">
187
188@@ -3874,7 +3884,6 @@
189
190 </namespace>
191
192-
193 <!--////////// Subvalues /////////////////////////////////////////////////-->
194
195 <subvalues>
196
197=== modified file 'src/diagnostics/pregenerated/diagnostic_list.cpp'
198--- src/diagnostics/pregenerated/diagnostic_list.cpp 2013-12-18 06:15:17 +0000
199+++ src/diagnostics/pregenerated/diagnostic_list.cpp 2014-01-16 23:23:15 +0000
200@@ -1473,6 +1473,9 @@
201 ZorbaCSVErrorCode INVALID_OPTION( "INVALID_OPTION" );
202
203
204+ZorbaCSVErrorCode INVALID_CSV_VALUE( "INVALID_CSV_VALUE" );
205+
206+
207 ZorbaCSVErrorCode MISSING_VALUE( "MISSING_VALUE" );
208
209
210
211=== modified file 'src/diagnostics/pregenerated/dict_en.cpp'
212--- src/diagnostics/pregenerated/dict_en.cpp 2013-12-18 06:15:17 +0000
213+++ src/diagnostics/pregenerated/dict_en.cpp 2014-01-16 23:23:15 +0000
214@@ -116,6 +116,7 @@
215 { "INCOMPLETE_DATE_OR_TIME", "'$1': incomplete date, time, or dateTime format" },
216 { "INSUFFICIENT_BUFFER", "\"$1\": insufficient value to parse for \"$2\"" },
217 { "INVALID_ABSOLUTE_PATH", "path component of absolute URI must begin with /" },
218+ { "INVALID_CSV_VALUE", "\"$1\": invalid type for value of key \"$2\"" },
219 { "INVALID_LOCALE", "\"$1\": invalid locale" },
220 { "INVALID_OPTION", "${\"1\": }invalid value for \"$2\" option${; 3}" },
221 { "INVALID_OPTION", "${\"1\": }invalid value for \"$2\" option${; 3}" },
222@@ -697,7 +698,9 @@
223 { "~ILLEGAL_KEY_FieldDescriptor", "field descriptor" },
224 { "~ILLEGAL_KEY_Type_34o", "$3 type${ \"4\"}" },
225 { "~ILLEGAL_KEY_UnionNoBaseType", "union type: unions may not have base types" },
226+ { "~INVALID_OPTION_ArrayElementsMustBeString", "array elements must be string" },
227 { "~INVALID_OPTION_MustBeASCIIChar", "must be single ASCII character" },
228+ { "~INVALID_OPTION_MustBeArray", "must be JSON array" },
229 { "~INVALID_OPTION_MustBeBoolean", "must be boolean" },
230 { "~INVALID_OPTION_MustBeString", "must be string" },
231 { "~INVALID_OPTION_MustBeTrueFalse", "must be sub-object with \"true\" and \"false\" keys" },
232
233=== modified file 'src/diagnostics/pregenerated/dict_zed_keys.h'
234--- src/diagnostics/pregenerated/dict_zed_keys.h 2013-12-18 06:15:17 +0000
235+++ src/diagnostics/pregenerated/dict_zed_keys.h 2014-01-16 23:23:15 +0000
236@@ -236,10 +236,12 @@
237 #define ZED_ZWST0009_JSONIQ_EMPTY_SEQUENCE "~ZWST0009_JSONIQ_EMPTY_SEQUENCE"
238 #define ZED_ZWST0009_OBJECT_KEY_NOT_QUOTED "~ZWST0009_OBJECT_KEY_NOT_QUOTED"
239 #define ZED_ZWST0009_AXIS_STEP "~ZWST0009_AXIS_STEP"
240+#define ZED_INVALID_OPTION_MustBeArray "~INVALID_OPTION_MustBeArray"
241 #define ZED_INVALID_OPTION_MustBeASCIIChar "~INVALID_OPTION_MustBeASCIIChar"
242 #define ZED_INVALID_OPTION_MustBeTrueFalse "~INVALID_OPTION_MustBeTrueFalse"
243 #define ZED_INVALID_OPTION_MustBeBoolean "~INVALID_OPTION_MustBeBoolean"
244 #define ZED_INVALID_OPTION_MustBeString "~INVALID_OPTION_MustBeString"
245+#define ZED_INVALID_OPTION_ArrayElementsMustBeString "~INVALID_OPTION_ArrayElementsMustBeString"
246 #define ZED_MISSING_VALUE_Default "~MISSING_VALUE_Default"
247 #define ZED_MISSING_VALUE_EmptyHeader "~MISSING_VALUE_EmptyHeader"
248 #define ZED_AllMatchesHasExcludes "~AllMatchesHasExcludes"
249
250=== modified file 'src/runtime/csv/csv_impl.cpp'
251--- src/runtime/csv/csv_impl.cpp 2013-12-18 06:15:17 +0000
252+++ src/runtime/csv/csv_impl.cpp 2014-01-16 23:23:15 +0000
253@@ -54,6 +54,39 @@
254 #define IS_JSON_NULL(ITEM) \
255 ( (ITEM)->isAtomic() && (ITEM)->getTypeCode() == store::JS_NULL )
256
257+static bool is_stringable( store::Item_t const &item ) {
258+ switch ( item->getKind() ) {
259+ case store::Item::ATOMIC:
260+ case store::Item::NODE:
261+ return true;
262+ default:
263+ return false;
264+ }
265+}
266+
267+inline zstring get_string_value( store::Item_t const &item ) {
268+ return is_stringable( item ) ? item->getStringValue() : "";
269+}
270+
271+static bool get_array_opt( store::Item_t const &object,
272+ char const *opt_name, store::Item_t *result,
273+ QueryLoc const &loc ) {
274+ if ( get_json_option( object, opt_name, result ) ) {
275+ if ( (*result)->getKind() != store::Item::ARRAY )
276+ throw XQUERY_EXCEPTION(
277+ csv::INVALID_OPTION,
278+ ERROR_PARAMS(
279+ get_string_value( *result ),
280+ opt_name,
281+ ZED( INVALID_OPTION_MustBeArray )
282+ ),
283+ ERROR_LOC( loc )
284+ );
285+ return true;
286+ }
287+ return false;
288+}
289+
290 static bool get_bool_opt( store::Item_t const &object,
291 char const *opt_name, bool *result,
292 QueryLoc const &loc ) {
293@@ -63,7 +96,7 @@
294 throw XQUERY_EXCEPTION(
295 csv::INVALID_OPTION,
296 ERROR_PARAMS(
297- opt_item->getStringValue(),
298+ get_string_value( opt_item ),
299 opt_name,
300 ZED( INVALID_OPTION_MustBeBoolean )
301 ),
302@@ -80,15 +113,23 @@
303 QueryLoc const &loc ) {
304 store::Item_t opt_item;
305 if ( get_json_option( object, opt_name, &opt_item ) ) {
306+ if ( !IS_ATOMIC_TYPE( opt_item, XS_STRING ) )
307+ throw XQUERY_EXCEPTION(
308+ csv::INVALID_OPTION,
309+ ERROR_PARAMS(
310+ get_string_value( opt_item ),
311+ opt_name,
312+ ZED( INVALID_OPTION_MustBeASCIIChar )
313+ ),
314+ ERROR_LOC( loc )
315+ );
316 zstring const value( opt_item->getStringValue() );
317- if ( !IS_ATOMIC_TYPE( opt_item, XS_STRING ) ||
318- value.size() != 1 || !ascii::is_ascii( value[0] ) ) {
319+ if ( value.size() != 1 || !ascii::is_ascii( value[0] ) )
320 throw XQUERY_EXCEPTION(
321 csv::INVALID_OPTION,
322 ERROR_PARAMS( value, opt_name, ZED( INVALID_OPTION_MustBeASCIIChar ) ),
323 ERROR_LOC( loc )
324 );
325- }
326 *result = value[0];
327 return true;
328 }
329@@ -104,7 +145,7 @@
330 throw XQUERY_EXCEPTION(
331 csv::INVALID_OPTION,
332 ERROR_PARAMS(
333- opt_item->getStringValue(),
334+ get_string_value( opt_item ),
335 opt_name,
336 ZED( INVALID_OPTION_MustBeString )
337 ),
338@@ -125,6 +166,30 @@
339 json::map_type( ptoken->get_type() ) : json::none;
340 }
341
342+static void set_keys( store::Item_t const &item, vector<store::Item_t> *keys,
343+ QueryLoc const &loc ) {
344+ store::Item_t opt_item;
345+ if ( get_array_opt( item, "field-names", &opt_item, loc ) ) {
346+ store::Iterator_t i( opt_item->getArrayValues() );
347+ i->open();
348+ store::Item_t name_item;
349+ while ( i->next( name_item ) ) {
350+ if ( !IS_ATOMIC_TYPE( name_item, XS_STRING ) )
351+ throw XQUERY_EXCEPTION(
352+ csv::INVALID_OPTION,
353+ ERROR_PARAMS(
354+ get_string_value( name_item ),
355+ "field-names",
356+ ZED( INVALID_OPTION_ArrayElementsMustBeString )
357+ ),
358+ ERROR_LOC( loc )
359+ );
360+ keys->push_back( name_item );
361+ } // while
362+ i->close();
363+ }
364+}
365+
366 ///////////////////////////////////////////////////////////////////////////////
367
368 void CsvParseIterator::set_input( store::Item_t const &item,
369@@ -147,14 +212,7 @@
370
371 get_bool_opt( item, "cast-unquoted-values", &state->cast_unquoted_, loc );
372 get_string_opt( item, "extra-name", &state->extra_name_, loc );
373- if ( get_json_option( item, "field-names", &opt_item ) ) {
374- store::Iterator_t i( opt_item->getArrayValues() );
375- i->open();
376- store::Item_t name_item;
377- while ( i->next( name_item ) )
378- state->keys_.push_back( name_item );
379- i->close();
380- }
381+ set_keys( item, &state->keys_, loc );
382 if ( get_string_opt( item, "missing-value", &value, loc ) ) {
383 if ( value == "error" )
384 state->missing_ = missing::error;
385@@ -488,14 +546,7 @@
386
387 // $options as object()
388 consumeNext( item, theChildren[1], plan_state );
389- if ( get_json_option( item, "field-names", &opt_item ) ) {
390- store::Iterator_t i( opt_item->getArrayValues() );
391- i->open();
392- store::Item_t name_item;
393- while ( i->next( name_item ) )
394- state->keys_.push_back( name_item );
395- i->close();
396- }
397+ set_keys( item, &state->keys_, loc );
398 if ( !get_char_opt( item, "quote-char", &state->quote_, loc ) )
399 state->quote_ = '"';
400 if ( get_char_opt( item, "quote-escape", &char_opt, loc ) ) {
401@@ -515,7 +566,7 @@
402 throw XQUERY_EXCEPTION(
403 csv::INVALID_OPTION,
404 ERROR_PARAMS(
405- "", "serialize-boolea-as",
406+ "", "serialize-boolean-as",
407 ZED( INVALID_OPTION_MustBeTrueFalse )
408 ),
409 ERROR_LOC( loc )
410@@ -582,7 +633,7 @@
411 line += state->boolean_string_[ value_item->getBooleanValue() ];
412 else if ( IS_JSON_NULL( value_item ) )
413 line += state->null_string_;
414- else {
415+ else if ( is_stringable( value_item ) ) {
416 value_item->getStringValue2( value );
417 bool const quote =
418 value.find_first_of( state->must_quote_ ) != zstring::npos;
419@@ -592,14 +643,18 @@
420 line += value;
421 if ( quote )
422 line += state->quote_;
423- }
424+ } else
425+ throw XQUERY_EXCEPTION(
426+ csv::INVALID_CSV_VALUE,
427+ ERROR_PARAMS( value_item->getKind(), (*key)->getStringValue() ),
428+ ERROR_LOC( loc )
429+ );
430 }
431 } // for
432 line += "\r\n";
433 GENV_ITEMFACTORY->createString( result, line );
434 STACK_PUSH( true, state );
435 } // while
436-
437 STACK_END( state );
438 }
439

Subscribers

People subscribed via source and target branches