2600hz changes #42

lazedo · 2017-02-24T14:54:40Z

some changes that fix/enhance jesse

fixes mixing draft3/draft4 schemas
export current_schema_id
allows extra validator
explicit error for enum
allow a setter_fun in options
set defaults

the fixes the mix between draft3/draft4 schemas

wraps the logic of validating referenced schemas

this will extra validations for schema elements example : lookup some value in a database an provide an error

allows to set values during validation

persistence depends on setter_fun option

default error handler

andreineculau · 2017-03-12T00:17:03Z

src/jesse_state.erl

@@ -192,7 +192,7 @@ resolve_ref(State, Reference) ->
      Path = jesse_json_path:parse(Pointer),
      case load_local_schema(State#state.root_schema, Path) of
        ?not_found ->
-          jesse_error:handle_schema_invalid(?schema_invalid, State);
+          jesse_error:handle_schema_invalid(Reference, State);


isn't the $ref already part of the error?
I've cherry-picked your change, and instead output the canonical reference (the output from combine_id). That is slightly more valuable i.e. see how the $ref was expanding, and see what jesse tried to find and load.

andreineculau · 2017-03-12T00:18:54Z

src/jesse_schema_validator.hrl

@@ -112,6 +112,7 @@
 -define(not_one_schema_valid,        'not_one_schema_valid').
 -define(not_schema_valid,            'not_schema_valid').
 -define(wrong_not_schema,            'wrong_not_schema').
+-define(external_error,              'external_error').


I can see the sugar-power for this change, and I've been tempted to do similar, but explain to me why wouldn't you first validate with jesse and then validate for more (e.g. realtime semantics), rather than call a function for all nested values (while I assume you're only interested in validate a handful) ?

andreineculau · 2017-03-12T00:25:50Z

src/jesse_error.erl

@@ -64,7 +64,7 @@
                           ) -> [error_reason()] | no_return().
 default_error_handler(Error, ErrorList, AllowedErrors) ->
  case AllowedErrors > length(ErrorList) orelse AllowedErrors =:= 'infinity' of
-    true  -> [Error | ErrorList];
+    true  -> ErrorList ++ [Error];
    false -> throw([Error | ErrorList])


shouldn't this fix also apply to the false branch ?

andreineculau · 2017-03-12T00:26:42Z

would you care to give some detailed rationale for

allow a setter_fun in schema validator
defaults

thanks

lazedo · 2017-03-13T09:26:13Z

@andreineculau
for defaults
if you give an empty object {} for a schema with defaults in its properties, it will fill up the object with the defaults and continue validation.

for setter_fun
when applying default, setter_fun will allow to actually set the value and return the value as part of validation.

example below for getting defaults for a schema

Options = [{'schema_loader_fun', fun kz_json_schema:fload/1}
                 ,{'allowed_errors', 'infinity'}
                 ,{'extra_validator', fun kz_json_schema_extensions:extra_validator/2}
                 ,{'setter_fun', fun kz_json:set_value/3}
                 ],
{ok, JObj} = kz_json_schema:validate(<<"system_config.conferences">>, kz_json:new(), Options),
io:format("~s~n", [kz_json:encode(JObj, ['pretty'])]).

{
  "entry_tone" : "tone_stream://v=-7;>=2;+=.1;%(300,0,523,659);v=-7;>=3;+=.1;%(800,0,659,783)",
  "exit_tone" : "tone_stream://v=-7;>=2;+=.1;%(300,0,523,440);v=-7;>=3;+=.1;%(800,0,349,440)",
  "moderator_entry_tone" : "tone_stream://v=-7;>=2;+=.1;%(300,0,523,659);v=-7;>=3;+=.1;%(800,0,659,783)",
  "moderator_exit_tone" : "tone_stream://v=-7;>=2;+=.1;%(300,0,523,440);v=-7;>=3;+=.1;%(800,0,349,440)",
  "number_timeout" : 5000,
  "pin_timeout" : 5000,
  "profiles" : {
    "default" : {
      "alone-sound" : "prompt://system_media/conf-alone/en-us",
      "caller-controls" : "default",
      "comfort-noise" : 1000,
      "deaf-sound" : "prompt://system_media/conf-deaf/en-us",
      "energy-level" : 20,
      "enter-sound" : "tone_stream://v=-7;>=2;+=.1;%(300,0,523,659);v=-7;>=3;+=.1;%(800,0,659,783)",
      "exit-sound" : "tone_stream://v=-7;>=2;+=.1;%(300,0,523,440);v=-7;>=3;+=.1;%(800,0,349,440)",
      "interval" : 20,
      "locked-sound" : "prompt://system_media/conf-max_participants/en-us",
      "max-member" : 200,
      "max-members-sound" : "prompt://system_media/conf-max_participants/en-us",
      "member-enter-sound" : "prompt://system_media/conf-joining_conference/en-us",
      "moh-sound" : "$${hold_music}",
      "muted-sound" : "prompt://system_media/conf-muted/en-us",
      "rate" : 8000,
      "undeaf-sound" : "prompt://system_media/conf-undeaf/en-us",
      "unmuted-sound" : "prompt://system_media/conf-unmuted/en-us"
    },
    "page" : {
      "caller-controls" : "default",
      "comfort-noise" : 1000,
      "energy-level" : 20,
      "enter-sound" : "",
      "interval" : 20,
      "moh-sound" : "",
      "rate" : 8000
    }
  },
  "review_name" : false,
  "route_win_timeout" : 3000,
  "support_name_announcement" : true
}

andreineculau · 2017-03-13T14:16:47Z

@lazedo thanks for the clear explanation!

I can only think of two requests:

can you add an option for this behaviour e.g. use_default, which would be on by default
setter_fun maybe renamed to default_setter_fun ?!

Wdyt?

lazedo · 2017-03-13T14:37:36Z

@andreineculau i'm ok with adding the use_default and we can default it to false to preserve current behaviour. thoughts?
as for the the setter_fun, why the rename ? if its not provided, there's no default to set the values, and since the library can act on different json implementations (jsx and other) there's really no easy way to set a default_setter_fun

lazedo · 2017-03-13T15:58:51Z

@andreineculau i ended up adding a 'validator_options' to control the defaults behaviour.
use_defaults -> to control applying defaults
'apply_defaults_to_empty_objects' -> to control if we should iterate object shcema properties and apply defaults.

this last is to bypass a recursive default {} with {$ref, #}

let me know if you agree or if there's anything else needed to wok on.
Thanks

lazedo · 2017-03-27T17:08:32Z

@andreineculau hi, anything else required here ?

needed when allowed_errors is `infinity`

andreineculau · 2017-03-29T06:49:40Z

yes, I'm required :( and I'm finding it hard to get time on my hands

…

On Mon, Mar 27, 2017 at 7:08 PM, lazedo ***@***.***> wrote: @andreineculau <https://github.com/andreineculau> hi, anything else required here ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#42 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AArOQWLNfrnhFCMw7ENPjjsfCkP99pF_ks5rp-0QgaJpZM4MLSoq> .

-- andreineculau.com <http://www.andreineculau.com>

lazedo · 2017-05-10T22:47:21Z

@andreineculau ping. we would love to create a 1.5 release after this merge

andreineculau · 2017-05-18T22:32:10Z

src/jesse_validator_draft3.erl

+  when is_binary(Type) ->
+    check_default_for_type(Default, State)
+        andalso is_type_valid(Default, Type, State);
+is_valid_default(Types, Default, State)


Types - typo?

expressing that in this clause we're dealing with a list used in lists:any

andreineculau · 2017-05-20T22:43:04Z

closing in favour of #51 and #52 (other commits already merged onto master). it was impossible for me to review properly as a mash. thanks!

lazedo added 7 commits February 24, 2017 14:20

restore default_schema_ver in undo_resolve_ref

fc761d9

the fixes the mix between draft3/draft4 schemas

create validate_ref

0f50fc3

wraps the logic of validating referenced schemas

allow to get current schema id

4997f84

provide schema id to error

91f2525

allow extra validator

ea502a6

this will extra validations for schema elements example : lookup some value in a database an provide an error

more explicit error for enums

58fac47

allow a setter_fun in schema validator

46f3d9a

allows to set values during validation

lazedo closed this Feb 24, 2017

lazedo reopened this Feb 24, 2017

set default values for properties

a4bf90f

persistence depends on setter_fun option

lazedo force-pushed the 2600hz-patches-1 branch from 11536f6 to a4bf90f Compare February 26, 2017 23:03

lazedo added 2 commits February 27, 2017 15:13

return errors in the order they occur

984635f

default error handler

fix path with default

46399e1

andreineculau reviewed Mar 12, 2017

View reviewed changes

lazedo added 2 commits March 13, 2017 08:59

fix throwing error in order

67df6d6

Merge branch 'master' into 2600hz-patches-1

5ba3b1c

lazedo added 2 commits March 13, 2017 15:53

add condition to apply defaults

d17bc19

fix setter test

e8460e1

handle invalid refs

c651c5e

needed when allowed_errors is `infinity`

fix default check when multiples types are provided

3c455d1

andreineculau reviewed May 18, 2017

View reviewed changes

This was referenced May 20, 2017

validator #51

Merged

default #52

Open

andreineculau closed this May 20, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2600hz changes #42

2600hz changes #42

lazedo commented Feb 24, 2017

andreineculau Mar 12, 2017

andreineculau Mar 12, 2017

andreineculau Mar 12, 2017

andreineculau commented Mar 12, 2017 •

edited

Loading

lazedo commented Mar 13, 2017

andreineculau commented Mar 13, 2017

lazedo commented Mar 13, 2017

lazedo commented Mar 13, 2017

lazedo commented Mar 27, 2017

andreineculau commented Mar 29, 2017 via email

lazedo commented May 10, 2017

andreineculau May 18, 2017

lazedo May 18, 2017

andreineculau commented May 20, 2017 •

edited

Loading

2600hz changes #42

2600hz changes #42

Conversation

lazedo commented Feb 24, 2017

andreineculau Mar 12, 2017

Choose a reason for hiding this comment

andreineculau Mar 12, 2017

Choose a reason for hiding this comment

andreineculau Mar 12, 2017

Choose a reason for hiding this comment

andreineculau commented Mar 12, 2017 • edited Loading

lazedo commented Mar 13, 2017

andreineculau commented Mar 13, 2017

lazedo commented Mar 13, 2017

lazedo commented Mar 13, 2017

lazedo commented Mar 27, 2017

andreineculau commented Mar 29, 2017 via email

lazedo commented May 10, 2017

andreineculau May 18, 2017

Choose a reason for hiding this comment

lazedo May 18, 2017

Choose a reason for hiding this comment

andreineculau commented May 20, 2017 • edited Loading

andreineculau commented Mar 12, 2017 •

edited

Loading

andreineculau commented May 20, 2017 •

edited

Loading