From fae7889fe3448e74404c741e6bf286dbc02b8e6e Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 13 Dec 2016 16:27:38 +0100 Subject: [PATCH 01/14] First take at producing exception-safe functions #314 --- .../25_exception_safe_public_functions.erb | 36 +++++++++++++++++++ .../renderer/templates/90_grant_execute.erb | 1 + 2 files changed, 37 insertions(+) create mode 100644 client/renderer/templates/25_exception_safe_public_functions.erb diff --git a/client/renderer/templates/25_exception_safe_public_functions.erb b/client/renderer/templates/25_exception_safe_public_functions.erb new file mode 100644 index 0000000..bd479c4 --- /dev/null +++ b/client/renderer/templates/25_exception_safe_public_functions.erb @@ -0,0 +1,36 @@ +-- +-- Exception-safe public DataServices API function +-- + +CREATE OR REPLACE FUNCTION <%= DATASERVICES_CLIENT_SCHEMA %>.<%= name %>_exception_safe (<%= params_with_type_and_default.join(' ,') %>) +RETURNS <%= return_type %> AS $$ +DECLARE + <% if not multi_row %>ret <%= return_type %>;<% end %> + username text; + orgname text; +BEGIN + IF session_user = 'publicuser' OR session_user ~ 'cartodb_publicuser_*' THEN + RAISE EXCEPTION 'The api_key must be provided'; + END IF; + SELECT u, o INTO username, orgname FROM <%= DATASERVICES_CLIENT_SCHEMA %>._cdb_entity_config() AS (u text, o text); + -- JSON value stored "" is taken as literal + IF username IS NULL OR username = '' OR username = '""' THEN + RAISE EXCEPTION 'Username is a mandatory argument, check it out'; + END IF; + BEGIN + <% if multi_row %> + RETURN QUERY + SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>); + <% elsif multi_field %> + SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>) INTO ret; + RETURN ret; + <% else %> + SELECT <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>) INTO ret; + RETURN ret; + <% end %> + EXCEPTION + WHEN OTHERS THEN + RAISE WARNING 'whatever'; + END; +END; +$$ LANGUAGE 'plpgsql' SECURITY DEFINER; diff --git a/client/renderer/templates/90_grant_execute.erb b/client/renderer/templates/90_grant_execute.erb index bde5944..bb00c6b 100644 --- a/client/renderer/templates/90_grant_execute.erb +++ b/client/renderer/templates/90_grant_execute.erb @@ -1 +1,2 @@ GRANT EXECUTE ON FUNCTION <%= DATASERVICES_CLIENT_SCHEMA %>.<%= name %>(<%= params_with_type.join(', ') %>) TO publicuser; +GRANT EXECUTE ON FUNCTION <%= DATASERVICES_CLIENT_SCHEMA %>.<%= name %>_exception_safe(<%= params_with_type.join(', ') %>) TO publicuser; From 4cd72616ca81c6077ce32027db325736945b0c41 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 13 Dec 2016 16:38:30 +0100 Subject: [PATCH 02/14] Fix the no multi_row, no multi_field case #314 Fix for the `ERROR: control reached end of function without RETURN` but now need to implement for the two other cases. --- .../25_exception_safe_public_functions.erb | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/client/renderer/templates/25_exception_safe_public_functions.erb b/client/renderer/templates/25_exception_safe_public_functions.erb index bd479c4..7bf7920 100644 --- a/client/renderer/templates/25_exception_safe_public_functions.erb +++ b/client/renderer/templates/25_exception_safe_public_functions.erb @@ -17,20 +17,22 @@ BEGIN IF username IS NULL OR username = '' OR username = '""' THEN RAISE EXCEPTION 'Username is a mandatory argument, check it out'; END IF; - BEGIN - <% if multi_row %> - RETURN QUERY - SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>); - <% elsif multi_field %> - SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>) INTO ret; - RETURN ret; - <% else %> + + <% if multi_row %> + RETURN QUERY + SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>); + <% elsif multi_field %> + SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>) INTO ret; + RETURN ret; + <% else %> + BEGIN SELECT <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>) INTO ret; RETURN ret; - <% end %> - EXCEPTION - WHEN OTHERS THEN - RAISE WARNING 'whatever'; - END; + EXCEPTION + WHEN OTHERS THEN + RAISE WARNING 'whatever'; + RETURN ret; + END; + <% end %> END; $$ LANGUAGE 'plpgsql' SECURITY DEFINER; From e53a39875e882c984108cb431ab0c29a2639e180 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 13 Dec 2016 16:51:11 +0100 Subject: [PATCH 03/14] Implement the multi_row case #314 --- .../templates/25_exception_safe_public_functions.erb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/client/renderer/templates/25_exception_safe_public_functions.erb b/client/renderer/templates/25_exception_safe_public_functions.erb index 7bf7920..04782d8 100644 --- a/client/renderer/templates/25_exception_safe_public_functions.erb +++ b/client/renderer/templates/25_exception_safe_public_functions.erb @@ -19,8 +19,13 @@ BEGIN END IF; <% if multi_row %> - RETURN QUERY - SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>); + BEGIN + RETURN QUERY + SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>); + EXCEPTION + WHEN OTHERS THEN + RAISE WARNING 'whatever'; + END; <% elsif multi_field %> SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>) INTO ret; RETURN ret; From dbd5911a2aef252b12ed86c617e89496b1ed5f7d Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 13 Dec 2016 17:00:55 +0100 Subject: [PATCH 04/14] Implement the multi_field case #314 --- .../templates/25_exception_safe_public_functions.erb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/client/renderer/templates/25_exception_safe_public_functions.erb b/client/renderer/templates/25_exception_safe_public_functions.erb index 04782d8..fe907e2 100644 --- a/client/renderer/templates/25_exception_safe_public_functions.erb +++ b/client/renderer/templates/25_exception_safe_public_functions.erb @@ -27,8 +27,14 @@ BEGIN RAISE WARNING 'whatever'; END; <% elsif multi_field %> - SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>) INTO ret; - RETURN ret; + BEGIN + SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>) INTO ret; + RETURN ret; + EXCEPTION + WHEN OTHERS THEN + RAISE WARNING 'whatever'; + RETURN ret; + END; <% else %> BEGIN SELECT <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>) INTO ret; From 842be0ba85d629be23c8a4e7b7c92aca776a4eb6 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 13 Dec 2016 19:19:03 +0100 Subject: [PATCH 05/14] Add diagnostics info to warning #314 --- .../templates/25_exception_safe_public_functions.erb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/client/renderer/templates/25_exception_safe_public_functions.erb b/client/renderer/templates/25_exception_safe_public_functions.erb index fe907e2..94208b9 100644 --- a/client/renderer/templates/25_exception_safe_public_functions.erb +++ b/client/renderer/templates/25_exception_safe_public_functions.erb @@ -27,12 +27,19 @@ BEGIN RAISE WARNING 'whatever'; END; <% elsif multi_field %> + DECLARE + _returned_sqlstate TEXT; + _message_text TEXT; + _pg_exception_context TEXT; BEGIN SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>) INTO ret; RETURN ret; EXCEPTION WHEN OTHERS THEN - RAISE WARNING 'whatever'; + GET STACKED DIAGNOSTICS _returned_sqlstate = RETURNED_SQLSTATE, + _message_text = MESSAGE_TEXT, + _pg_exception_context = PG_EXCEPTION_CONTEXT; + RAISE WARNING USING ERRCODE = _returned_sqlstate, MESSAGE = _message_text, DETAIL = _pg_exception_context; RETURN ret; END; <% else %> From 58b1713a0dcdd9d33769eefbaf5961c51f8591e1 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 14 Dec 2016 10:27:43 +0100 Subject: [PATCH 06/14] Make the functions private #314 --- ..._functions.erb => 25_exception_safe_private_functions.erb} | 4 ++-- client/renderer/templates/90_grant_execute.erb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) rename client/renderer/templates/{25_exception_safe_public_functions.erb => 25_exception_safe_private_functions.erb} (90%) diff --git a/client/renderer/templates/25_exception_safe_public_functions.erb b/client/renderer/templates/25_exception_safe_private_functions.erb similarity index 90% rename from client/renderer/templates/25_exception_safe_public_functions.erb rename to client/renderer/templates/25_exception_safe_private_functions.erb index 94208b9..b929ae9 100644 --- a/client/renderer/templates/25_exception_safe_public_functions.erb +++ b/client/renderer/templates/25_exception_safe_private_functions.erb @@ -1,8 +1,8 @@ -- --- Exception-safe public DataServices API function +-- Exception-safe private DataServices API function -- -CREATE OR REPLACE FUNCTION <%= DATASERVICES_CLIENT_SCHEMA %>.<%= name %>_exception_safe (<%= params_with_type_and_default.join(' ,') %>) +CREATE OR REPLACE FUNCTION <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>_exception_safe (<%= params_with_type_and_default.join(' ,') %>) RETURNS <%= return_type %> AS $$ DECLARE <% if not multi_row %>ret <%= return_type %>;<% end %> diff --git a/client/renderer/templates/90_grant_execute.erb b/client/renderer/templates/90_grant_execute.erb index bb00c6b..b6c797e 100644 --- a/client/renderer/templates/90_grant_execute.erb +++ b/client/renderer/templates/90_grant_execute.erb @@ -1,2 +1,2 @@ GRANT EXECUTE ON FUNCTION <%= DATASERVICES_CLIENT_SCHEMA %>.<%= name %>(<%= params_with_type.join(', ') %>) TO publicuser; -GRANT EXECUTE ON FUNCTION <%= DATASERVICES_CLIENT_SCHEMA %>.<%= name %>_exception_safe(<%= params_with_type.join(', ') %>) TO publicuser; +GRANT EXECUTE ON FUNCTION <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>_exception_safe(<%= params_with_type.join(', ') %>) TO publicuser; From 610cfaab5722b443c06990207ce9d95080a41e3f Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 14 Dec 2016 10:33:50 +0100 Subject: [PATCH 07/14] Move exception info vars to the top #314 --- .../templates/25_exception_safe_private_functions.erb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/client/renderer/templates/25_exception_safe_private_functions.erb b/client/renderer/templates/25_exception_safe_private_functions.erb index b929ae9..fa1ae6a 100644 --- a/client/renderer/templates/25_exception_safe_private_functions.erb +++ b/client/renderer/templates/25_exception_safe_private_functions.erb @@ -8,6 +8,9 @@ DECLARE <% if not multi_row %>ret <%= return_type %>;<% end %> username text; orgname text; + _returned_sqlstate TEXT; + _message_text TEXT; + _pg_exception_context TEXT; BEGIN IF session_user = 'publicuser' OR session_user ~ 'cartodb_publicuser_*' THEN RAISE EXCEPTION 'The api_key must be provided'; @@ -27,10 +30,6 @@ BEGIN RAISE WARNING 'whatever'; END; <% elsif multi_field %> - DECLARE - _returned_sqlstate TEXT; - _message_text TEXT; - _pg_exception_context TEXT; BEGIN SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>) INTO ret; RETURN ret; From cd653bc4960e70ec12b12862d71f6cfae261f218 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 14 Dec 2016 10:35:44 +0100 Subject: [PATCH 08/14] Add diagnostics info to rest of cases #314 --- .../templates/25_exception_safe_private_functions.erb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/client/renderer/templates/25_exception_safe_private_functions.erb b/client/renderer/templates/25_exception_safe_private_functions.erb index fa1ae6a..2dde980 100644 --- a/client/renderer/templates/25_exception_safe_private_functions.erb +++ b/client/renderer/templates/25_exception_safe_private_functions.erb @@ -27,7 +27,10 @@ BEGIN SELECT * FROM <%= DATASERVICES_CLIENT_SCHEMA %>._<%= name %>(<%= ['username', 'orgname'].concat(params).join(', ') %>); EXCEPTION WHEN OTHERS THEN - RAISE WARNING 'whatever'; + GET STACKED DIAGNOSTICS _returned_sqlstate = RETURNED_SQLSTATE, + _message_text = MESSAGE_TEXT, + _pg_exception_context = PG_EXCEPTION_CONTEXT; + RAISE WARNING USING ERRCODE = _returned_sqlstate, MESSAGE = _message_text, DETAIL = _pg_exception_context; END; <% elsif multi_field %> BEGIN @@ -47,7 +50,10 @@ BEGIN RETURN ret; EXCEPTION WHEN OTHERS THEN - RAISE WARNING 'whatever'; + GET STACKED DIAGNOSTICS _returned_sqlstate = RETURNED_SQLSTATE, + _message_text = MESSAGE_TEXT, + _pg_exception_context = PG_EXCEPTION_CONTEXT; + RAISE WARNING USING ERRCODE = _returned_sqlstate, MESSAGE = _message_text, DETAIL = _pg_exception_context; RETURN ret; END; <% end %> From c97f03b2e33207a806e08e3403ac274ae64d7d97 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 14 Dec 2016 12:06:51 +0100 Subject: [PATCH 09/14] Add generated file to .gitignore #314 --- client/.gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/.gitignore b/client/.gitignore index dff332e..4e31250 100644 --- a/client/.gitignore +++ b/client/.gitignore @@ -2,9 +2,10 @@ results/ regression.diffs regression.out 20_public_functions.sql +25_exception_safe_private_functions.sql 30_plproxy_functions.sql 90_grant_execute.sql cdb_geocoder_client--0.0.1.sql cdb_geocoder_client--0.1.0.sql cdb_geocoder_client--0.2.0.sql -cdb_geocoder_client--0.3.0.sql \ No newline at end of file +cdb_geocoder_client--0.3.0.sql From 99b76afc3354ae0321e4afafc243490f4ed8af09 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 14 Dec 2016 12:08:26 +0100 Subject: [PATCH 10/14] Some versioning facilities for make release #314 --- client/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/Makefile b/client/Makefile index fe6939d..3930d5d 100644 --- a/client/Makefile +++ b/client/Makefile @@ -57,9 +57,11 @@ all: $(DATA) .PHONY: release release: $(EXTENSION).control $(SOURCES_DATA) test -n "$(NEW_VERSION)" # $$NEW_VERSION VARIABLE MISSING. Eg. make release NEW_VERSION=0.x.0 - mv *.sql old_versions + git mv *.sql old_versions $(SED) $(REPLACEMENTS) $(EXTENSION).control + git add $(EXTENSION).control cat $(SOURCES_DATA_DIR)/*.sql > $(EXTENSION)--$(NEW_VERSION).sql + git add $(EXTENSION)--$(NEW_VERSION).sql $(ERB) version=$(NEW_VERSION) upgrade_downgrade_template.erb > $(EXTENSION)--$(EXTVERSION)--$(NEW_VERSION).sql $(ERB) version=$(EXTVERSION) upgrade_downgrade_template.erb > $(EXTENSION)--$(NEW_VERSION)--$(EXTVERSION).sql From 5c8dbe91eb62356ee50fcf230f1cd9e549b735a7 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 14 Dec 2016 12:39:56 +0100 Subject: [PATCH 11/14] Add test for simple interface case #314 --- ..._exception_safe_private_functions_test.out | 21 +++++++++++++++++++ ..._exception_safe_private_functions_test.sql | 16 ++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 client/test/expected/25_exception_safe_private_functions_test.out create mode 100644 client/test/sql/25_exception_safe_private_functions_test.sql diff --git a/client/test/expected/25_exception_safe_private_functions_test.out b/client/test/expected/25_exception_safe_private_functions_test.out new file mode 100644 index 0000000..01b79ed --- /dev/null +++ b/client/test/expected/25_exception_safe_private_functions_test.out @@ -0,0 +1,21 @@ +SET search_path TO public,cartodb,cdb_dataservices_client; +-- Mock the server function to fail +CREATE OR REPLACE FUNCTION cdb_dataservices_server.cdb_geocode_street_point (username text, orgname text, searchtext text, city text DEFAULT NULL, state_province text DEFAULT NULL, country text DEFAULT NULL) +RETURNS Geometry AS $$ +BEGIN + RAISE EXCEPTION 'Not enough quota or any other exception whatsoever.'; + RETURN NULL; +END; +$$ LANGUAGE 'plpgsql'; +-- Use regular user role +SET ROLE test_regular_user; +-- Exercise the public and the proxied function +SELECT _cdb_geocode_street_point_exception_safe('One street, 1'); +WARNING: cdb_dataservices_client._cdb_geocode_street_point(6): [contrib_regression] REMOTE ERROR: Not enough quota or any other exception whatsoever. +DETAIL: SQL statement "SELECT cdb_dataservices_client._cdb_geocode_street_point(username, orgname, searchtext, city, state_province, country)" +PL/pgSQL function _cdb_geocode_street_point_exception_safe(text,text,text,text) line 21 at SQL statement + _cdb_geocode_street_point_exception_safe +------------------------------------------ + +(1 row) + diff --git a/client/test/sql/25_exception_safe_private_functions_test.sql b/client/test/sql/25_exception_safe_private_functions_test.sql new file mode 100644 index 0000000..c5a1831 --- /dev/null +++ b/client/test/sql/25_exception_safe_private_functions_test.sql @@ -0,0 +1,16 @@ +SET search_path TO public,cartodb,cdb_dataservices_client; + +-- Mock the server function to fail +CREATE OR REPLACE FUNCTION cdb_dataservices_server.cdb_geocode_street_point (username text, orgname text, searchtext text, city text DEFAULT NULL, state_province text DEFAULT NULL, country text DEFAULT NULL) +RETURNS Geometry AS $$ +BEGIN + RAISE EXCEPTION 'Not enough quota or any other exception whatsoever.'; + RETURN NULL; +END; +$$ LANGUAGE 'plpgsql'; + +-- Use regular user role +SET ROLE test_regular_user; + +-- Exercise the public and the proxied function +SELECT _cdb_geocode_street_point_exception_safe('One street, 1'); From 4523b2e04d18993bea6d761523d064f3897933ba Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 14 Dec 2016 12:53:51 +0100 Subject: [PATCH 12/14] Add test for multi_row case #314 --- ...5_exception_safe_private_functions_test.out | 18 ++++++++++++++++-- ...5_exception_safe_private_functions_test.sql | 14 ++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/client/test/expected/25_exception_safe_private_functions_test.out b/client/test/expected/25_exception_safe_private_functions_test.out index 01b79ed..de19bd2 100644 --- a/client/test/expected/25_exception_safe_private_functions_test.out +++ b/client/test/expected/25_exception_safe_private_functions_test.out @@ -1,5 +1,6 @@ +SET client_min_messages TO warning; SET search_path TO public,cartodb,cdb_dataservices_client; --- Mock the server function to fail +-- Mock the server functions to raise exceptions CREATE OR REPLACE FUNCTION cdb_dataservices_server.cdb_geocode_street_point (username text, orgname text, searchtext text, city text DEFAULT NULL, state_province text DEFAULT NULL, country text DEFAULT NULL) RETURNS Geometry AS $$ BEGIN @@ -7,9 +8,15 @@ BEGIN RETURN NULL; END; $$ LANGUAGE 'plpgsql'; +CREATE OR REPLACE FUNCTION cdb_dataservices_server.cdb_isodistance(username text, orgname text, source geometry, mode text, range integer[], options text[] DEFAULT ARRAY[]::text[]) +RETURNS SETOF isoline AS $$ +BEGIN + RAISE EXCEPTION 'Not enough quota or any other exception whatsoever.'; +END; +$$ LANGUAGE 'plpgsql'; -- Use regular user role SET ROLE test_regular_user; --- Exercise the public and the proxied function +-- Exercise the exception safe and the proxied functions SELECT _cdb_geocode_street_point_exception_safe('One street, 1'); WARNING: cdb_dataservices_client._cdb_geocode_street_point(6): [contrib_regression] REMOTE ERROR: Not enough quota or any other exception whatsoever. DETAIL: SQL statement "SELECT cdb_dataservices_client._cdb_geocode_street_point(username, orgname, searchtext, city, state_province, country)" @@ -19,3 +26,10 @@ PL/pgSQL function _cdb_geocode_street_point_exception_safe(text,text,text,text) (1 row) +SELECT the_geom FROM _cdb_isodistance_exception_safe('POINT(-3.70568 40.42028)'::geometry, 'walk', ARRAY[300]::integer[]); +WARNING: cdb_dataservices_client._cdb_isodistance(6): [contrib_regression] REMOTE ERROR: Not enough quota or any other exception whatsoever. +DETAIL: PL/pgSQL function _cdb_isodistance_exception_safe(geometry,text,integer[],text[]) line 21 at RETURN QUERY + the_geom +---------- +(0 rows) + diff --git a/client/test/sql/25_exception_safe_private_functions_test.sql b/client/test/sql/25_exception_safe_private_functions_test.sql index c5a1831..c48a465 100644 --- a/client/test/sql/25_exception_safe_private_functions_test.sql +++ b/client/test/sql/25_exception_safe_private_functions_test.sql @@ -1,6 +1,7 @@ +SET client_min_messages TO warning; SET search_path TO public,cartodb,cdb_dataservices_client; --- Mock the server function to fail +-- Mock the server functions to raise exceptions CREATE OR REPLACE FUNCTION cdb_dataservices_server.cdb_geocode_street_point (username text, orgname text, searchtext text, city text DEFAULT NULL, state_province text DEFAULT NULL, country text DEFAULT NULL) RETURNS Geometry AS $$ BEGIN @@ -9,8 +10,17 @@ BEGIN END; $$ LANGUAGE 'plpgsql'; +CREATE OR REPLACE FUNCTION cdb_dataservices_server.cdb_isodistance(username text, orgname text, source geometry, mode text, range integer[], options text[] DEFAULT ARRAY[]::text[]) +RETURNS SETOF isoline AS $$ +BEGIN + RAISE EXCEPTION 'Not enough quota or any other exception whatsoever.'; +END; +$$ LANGUAGE 'plpgsql'; + + -- Use regular user role SET ROLE test_regular_user; --- Exercise the public and the proxied function +-- Exercise the exception safe and the proxied functions SELECT _cdb_geocode_street_point_exception_safe('One street, 1'); +SELECT the_geom FROM _cdb_isodistance_exception_safe('POINT(-3.70568 40.42028)'::geometry, 'walk', ARRAY[300]::integer[]); From 8692fb12cae38b01d6bbe8d85582c94f533788b5 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 14 Dec 2016 12:57:51 +0100 Subject: [PATCH 13/14] Add test case for multi_field #314 --- ..._exception_safe_private_functions_test.out | 21 +++++++++++++++++++ ..._exception_safe_private_functions_test.sql | 15 +++++++++++++ 2 files changed, 36 insertions(+) diff --git a/client/test/expected/25_exception_safe_private_functions_test.out b/client/test/expected/25_exception_safe_private_functions_test.out index de19bd2..f7a6c97 100644 --- a/client/test/expected/25_exception_safe_private_functions_test.out +++ b/client/test/expected/25_exception_safe_private_functions_test.out @@ -14,6 +14,18 @@ BEGIN RAISE EXCEPTION 'Not enough quota or any other exception whatsoever.'; END; $$ LANGUAGE 'plpgsql'; +CREATE OR REPLACE FUNCTION cdb_dataservices_server.cdb_route_point_to_point (username text, orgname text, origin geometry(Point, 4326), destination geometry(Point, 4326), mode TEXT, options text[] DEFAULT ARRAY[]::text[], units text DEFAULT 'kilometers') +RETURNS cdb_dataservices_client.simple_route AS $$ +DECLARE + ret cdb_dataservices_client.simple_route; +BEGIN + RAISE EXCEPTION 'Not enough quota or any other exception whatsoever.'; + + -- This code shall never be reached + SELECT NULL, 5.33, 100 INTO ret; + RETURN ret; +END; +$$ LANGUAGE 'plpgsql'; -- Use regular user role SET ROLE test_regular_user; -- Exercise the exception safe and the proxied functions @@ -33,3 +45,12 @@ DETAIL: PL/pgSQL function _cdb_isodistance_exception_safe(geometry,text,integer ---------- (0 rows) +SELECT shape FROM _cdb_route_point_to_point_exception_safe('POINT(-3.70237112 40.41706163)'::geometry,'POINT(-3.69909883 40.41236875)'::geometry, 'car', ARRAY['mode_type=shortest']::text[]); +WARNING: cdb_dataservices_client._cdb_route_point_to_point(7): [contrib_regression] REMOTE ERROR: Not enough quota or any other exception whatsoever. +DETAIL: SQL statement "SELECT * FROM cdb_dataservices_client._cdb_route_point_to_point(username, orgname, origin, destination, mode, options, units)" +PL/pgSQL function _cdb_route_point_to_point_exception_safe(geometry,geometry,text,text[],text) line 21 at SQL statement + shape +------- + +(1 row) + diff --git a/client/test/sql/25_exception_safe_private_functions_test.sql b/client/test/sql/25_exception_safe_private_functions_test.sql index c48a465..98fce99 100644 --- a/client/test/sql/25_exception_safe_private_functions_test.sql +++ b/client/test/sql/25_exception_safe_private_functions_test.sql @@ -17,6 +17,20 @@ BEGIN END; $$ LANGUAGE 'plpgsql'; +CREATE OR REPLACE FUNCTION cdb_dataservices_server.cdb_route_point_to_point (username text, orgname text, origin geometry(Point, 4326), destination geometry(Point, 4326), mode TEXT, options text[] DEFAULT ARRAY[]::text[], units text DEFAULT 'kilometers') +RETURNS cdb_dataservices_client.simple_route AS $$ +DECLARE + ret cdb_dataservices_client.simple_route; +BEGIN + RAISE EXCEPTION 'Not enough quota or any other exception whatsoever.'; + + -- This code shall never be reached + SELECT NULL, 5.33, 100 INTO ret; + RETURN ret; +END; +$$ LANGUAGE 'plpgsql'; + + -- Use regular user role SET ROLE test_regular_user; @@ -24,3 +38,4 @@ SET ROLE test_regular_user; -- Exercise the exception safe and the proxied functions SELECT _cdb_geocode_street_point_exception_safe('One street, 1'); SELECT the_geom FROM _cdb_isodistance_exception_safe('POINT(-3.70568 40.42028)'::geometry, 'walk', ARRAY[300]::integer[]); +SELECT shape FROM _cdb_route_point_to_point_exception_safe('POINT(-3.70237112 40.41706163)'::geometry,'POINT(-3.69909883 40.41236875)'::geometry, 'car', ARRAY['mode_type=shortest']::text[]); From 3c60f3e93ba0d492b0112b75a662d0cd4271e3ac Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 14 Dec 2016 13:30:28 +0100 Subject: [PATCH 14/14] Minor improvement for test #314 Make sure we return an empty record and that the mentioned code is never reached. --- .../25_exception_safe_private_functions_test.out | 14 +++++++------- .../25_exception_safe_private_functions_test.sql | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/client/test/expected/25_exception_safe_private_functions_test.out b/client/test/expected/25_exception_safe_private_functions_test.out index f7a6c97..6d18ab6 100644 --- a/client/test/expected/25_exception_safe_private_functions_test.out +++ b/client/test/expected/25_exception_safe_private_functions_test.out @@ -38,19 +38,19 @@ PL/pgSQL function _cdb_geocode_street_point_exception_safe(text,text,text,text) (1 row) -SELECT the_geom FROM _cdb_isodistance_exception_safe('POINT(-3.70568 40.42028)'::geometry, 'walk', ARRAY[300]::integer[]); +SELECT * FROM _cdb_isodistance_exception_safe('POINT(-3.70568 40.42028)'::geometry, 'walk', ARRAY[300]::integer[]); WARNING: cdb_dataservices_client._cdb_isodistance(6): [contrib_regression] REMOTE ERROR: Not enough quota or any other exception whatsoever. DETAIL: PL/pgSQL function _cdb_isodistance_exception_safe(geometry,text,integer[],text[]) line 21 at RETURN QUERY - the_geom ----------- + center | data_range | the_geom +--------+------------+---------- (0 rows) -SELECT shape FROM _cdb_route_point_to_point_exception_safe('POINT(-3.70237112 40.41706163)'::geometry,'POINT(-3.69909883 40.41236875)'::geometry, 'car', ARRAY['mode_type=shortest']::text[]); +SELECT * FROM _cdb_route_point_to_point_exception_safe('POINT(-3.70237112 40.41706163)'::geometry,'POINT(-3.69909883 40.41236875)'::geometry, 'car', ARRAY['mode_type=shortest']::text[]); WARNING: cdb_dataservices_client._cdb_route_point_to_point(7): [contrib_regression] REMOTE ERROR: Not enough quota or any other exception whatsoever. DETAIL: SQL statement "SELECT * FROM cdb_dataservices_client._cdb_route_point_to_point(username, orgname, origin, destination, mode, options, units)" PL/pgSQL function _cdb_route_point_to_point_exception_safe(geometry,geometry,text,text[],text) line 21 at SQL statement - shape -------- - + shape | length | duration +-------+--------+---------- + | | (1 row) diff --git a/client/test/sql/25_exception_safe_private_functions_test.sql b/client/test/sql/25_exception_safe_private_functions_test.sql index 98fce99..ba45130 100644 --- a/client/test/sql/25_exception_safe_private_functions_test.sql +++ b/client/test/sql/25_exception_safe_private_functions_test.sql @@ -37,5 +37,5 @@ SET ROLE test_regular_user; -- Exercise the exception safe and the proxied functions SELECT _cdb_geocode_street_point_exception_safe('One street, 1'); -SELECT the_geom FROM _cdb_isodistance_exception_safe('POINT(-3.70568 40.42028)'::geometry, 'walk', ARRAY[300]::integer[]); -SELECT shape FROM _cdb_route_point_to_point_exception_safe('POINT(-3.70237112 40.41706163)'::geometry,'POINT(-3.69909883 40.41236875)'::geometry, 'car', ARRAY['mode_type=shortest']::text[]); +SELECT * FROM _cdb_isodistance_exception_safe('POINT(-3.70568 40.42028)'::geometry, 'walk', ARRAY[300]::integer[]); +SELECT * FROM _cdb_route_point_to_point_exception_safe('POINT(-3.70237112 40.41706163)'::geometry,'POINT(-3.69909883 40.41236875)'::geometry, 'car', ARRAY['mode_type=shortest']::text[]);