Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Draft 6 Specification #108

Merged
merged 36 commits into from
Feb 8, 2022
Merged

Conversation

kikofernandez
Copy link

This PR contains the implementation of Draft 6 of the Specification.

The implementation was a joint effort between @miterst and @kikofernandez and it was done during two Klarna hackathons.

The specification draft adds a few new keywords and some backwards breaking changes, which I believe we have dealt with. All is self-contained in jesse_validator_draft6.erl, and we have included unit tests to check the correctness of our implementation. The draft documentation has also been copied from the original source to the source code of jesse_validator_draft6.erl (as in previous drafts).

If you know how to perform separate compilation for different Erlang releases, then we can add uri-reference for OTP-21+, since the module I use was only available from OTP 21 onwards.

Any feedback is appreciated!
Thanks.

@paulo-ferraz-oliveira
Copy link
Contributor

This is great stuff.

If you know how to perform separate compilation for different Erlang releases

You mean "conditional compilation"? There's macro ?OTP_RELEASE for that. Prior to 21 it didn't exist, so...

@kikofernandez
Copy link
Author

Yes, I meant conditional compilation, sorry.

@paulo-ferraz-oliveira
Copy link
Contributor

-ifdef(OTP_RELEASE).
% OTP 21+ context
...
-else.
% OTP up to 21 (excluding it) context
...
-endif.

end.
-else.
uri_reference(_Value, State) ->
State.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and for pre-OTP-21 you can try to use http_uri module https://erldocs.com/20.0/inets/http_uri.html?i=0&search=http#undefined. But I think it's not that important.

@seriyps
Copy link
Contributor

seriyps commented May 24, 2021

I think the diff between validator v4 and validator v6 might be helpfull to highlight the difference:

--- src/jesse_validator_draft4.erl	2021-05-24 13:04:52.298767116 +0200
+++ src/jesse_validator_draft6.erl	2021-05-24 13:04:52.298767116 +0200
@@ -21,7 +21,7 @@
 %% @end
 %%%=============================================================================
 
--module(jesse_validator_draft4).
+-module(jesse_validator_draft6).
 
 %% API
 -export([ check_value/3
@@ -44,7 +44,7 @@
                       | ?wrong_type_dependency
                       | ?wrong_type_items
                       | ?wrong_type_specification
-                      | ?wrong_draft4_id_tag.
+                      | ?wrong_draft6_id_tag.
 
 -type schema_error_type() :: schema_error()
                            | {schema_error(), jesse:json_term()}.
@@ -81,8 +81,8 @@
                  , JsonSchema :: jesse:schema()
                  , State :: jesse_state:state()
                  ) -> jesse_state:state() | no_return().
-check_value(_Value, [{?IDv6, _ID} | _Attrs], State) ->
-  handle_schema_invalid(?wrong_draft4_id_tag, State);
+check_value(_, [{?ID, _} | _], State) ->
+  handle_schema_invalid(?wrong_draft6_id_tag, State);
 check_value(Value, [{?REF, RefSchemaURI} | Attrs], State) ->
   case Attrs of
     [] ->
@@ -93,6 +93,10 @@
 check_value(Value, [{?TYPE, Type} | Attrs], State) ->
   NewState = check_type(Value, Type, State),
   check_value(Value, Attrs, NewState);
+check_value(Value, [{?PROPERTIES, true} | Attrs], State) ->
+  check_value(Value, Attrs, State);
+check_value(Value, [{?PROPERTIES, false} | _Attrs], State) ->
+  handle_data_invalid(?validation_always_fails, Value, State);
 check_value(Value, [{?PROPERTIES, Properties} | Attrs], State) ->
   NewState = case jesse_lib:is_json_object(Value) of
                true  -> check_properties( Value
@@ -115,6 +119,37 @@
              end,
   check_value(Value, Attrs, NewState);
 check_value( Value
+           , [{?PROPERTYNAMES, true} | Attrs]
+           , State
+           ) ->
+  case jesse_lib:is_json_object(Value) of
+    true -> check_value( Value, Attrs, State);
+    false -> State
+  end;
+check_value( Value
+           , [{?PROPERTYNAMES, false} | _Attrs]
+           , State
+           ) ->
+  case jesse_lib:is_json_object(Value) of
+    true -> handle_data_invalid(?validation_always_fails, Value, State);
+    false -> State
+  end;
+check_value( Value
+           , [{?PROPERTYNAMES, PatternProperties} | Attrs]
+           , State
+           ) ->
+  %% The description of PropertyNames seems to do the same as PatternProperties.
+  %% I am not sure whether the PatternProperties follows the standard draft 4,
+  %% since it needs to consider that additionalProperties must be false.
+  NewState = case jesse_lib:is_json_object(Value) of
+               true  -> check_property_names( Value
+                                            , PatternProperties
+                                            , State
+                                            );
+               false -> State
+             end,
+  check_value(Value, Attrs, NewState);
+check_value( Value
            , [{?ADDITIONALPROPERTIES, AdditionalProperties} | Attrs]
            , State
            ) ->
@@ -126,6 +161,12 @@
                false -> State
        end,
   check_value(Value, Attrs, NewState);
+check_value(Value, [{?ITEMS, true} | Attrs], State) ->
+  check_value(Value, Attrs, State);
+check_value(Value, [{?ITEMS, false} | Attrs], State) ->
+% Default semantics: `not: {}`
+  Not = {?NOT, {[  ]}},
+  check_value(Value, [{?ITEMS, Not} | Attrs], State);
 check_value(Value, [{?ITEMS, Items} | Attrs], State) ->
   NewState = case jesse_lib:is_array(Value) of
                true  -> check_items(Value, Items, State);
@@ -139,6 +180,25 @@
            , State
            ) ->
   check_value(Value, Attrs, State);
+check_value(Value, [{?CONTAINS, true} | Attrs], State) ->
+  check_value(Value, Attrs, State);
+check_value(Value, [{?CONTAINS, false} | _Attrs], State) ->
+  handle_data_invalid(?validation_always_fails, Value, State);
+check_value(Value, [{?CONTAINS, Schema} | Attrs], State) ->
+  NewState = case jesse_lib:is_array(Value) of
+               true  -> check_contains(Value, Schema, State);
+               false -> State
+             end,
+  check_value(Value, Attrs, NewState);
+check_value(Value, [{?EXAMPLES, _Examples} | Attrs], State) ->
+  NewState = case jesse_lib:is_array(Value) of
+               true  ->
+                 %% No need to check.
+                 %% The schema is valid, by definition, at this point.
+                 State;
+               false -> handle_data_invalid(?not_array, Value, State)
+             end,
+  check_value(Value, Attrs, NewState);
 check_value(Value, [{?REQUIRED, Required} | Attrs], State) ->
   NewState = case jesse_lib:is_json_object(Value) of
                true  -> check_required(Value, Required, State);
@@ -154,10 +214,15 @@
 check_value(Value, [{?MINIMUM, Minimum} | Attrs], State) ->
   NewState = case is_number(Value) of
                true  ->
-                 ExclusiveMinimum = get_value( ?EXCLUSIVEMINIMUM
-                                             , get_current_schema(State)
-                                             ),
-                 check_minimum(Value, Minimum, ExclusiveMinimum, State);
+                 check_minimum(Value, Minimum, State);
+               false ->
+                 State
+             end,
+  check_value(Value, Attrs, NewState);
+check_value(Value, [{?EXCLUSIVEMINIMUM, ExclusiveMinimum} | Attrs], State) ->
+  NewState = case is_number(Value) of
+               true  ->
+                 check_exclusive_minimum(Value, ExclusiveMinimum, State);
                false ->
                  State
              end,
@@ -165,28 +230,19 @@
 check_value(Value, [{?MAXIMUM, Maximum} | Attrs], State) ->
   NewState = case is_number(Value) of
                true  ->
-                 ExclusiveMaximum = get_value( ?EXCLUSIVEMAXIMUM
-                                             , get_current_schema(State)
-                                             ),
-                 check_maximum(Value, Maximum, ExclusiveMaximum, State);
+                 check_maximum(Value, Maximum, State);
+               false ->
+                 State
+             end,
+  check_value(Value, Attrs, NewState);
+check_value(Value, [{?EXCLUSIVEMAXIMUM, ExclusiveMaximum} | Attrs], State) ->
+  NewState = case is_number(Value) of
+               true  ->
+                 check_exclusive_maximum(Value, ExclusiveMaximum, State);
                false ->
                  State
              end,
   check_value(Value, Attrs, NewState);
-%% doesn't really do anything, since this attribute will be handled
-%% by the previous function clause if it's presented in the schema
-check_value( Value
-           , [{?EXCLUSIVEMINIMUM, _ExclusiveMinimum} | Attrs]
-           , State
-           ) ->
-  check_value(Value, Attrs, State);
-%% doesn't really do anything, since this attribute will be handled
-%% by the previous function clause if it's presented in the schema
-check_value( Value
-           , [{?EXCLUSIVEMAXIMUM, _ExclusiveMaximum} | Attrs]
-           , State
-           ) ->
-  check_value(Value, Attrs, State);
 check_value(Value, [{?MINITEMS, MinItems} | Attrs], State) ->
   NewState = case jesse_lib:is_array(Value) of
                true  -> check_min_items(Value, MinItems, State);
@@ -226,6 +282,9 @@
 check_value(Value, [{?ENUM, Enum} | Attrs], State) ->
   NewState = check_enum(Value, Enum, State),
   check_value(Value, Attrs, NewState);
+check_value(Value, [{?CONST, Enum} | Attrs], State) ->
+  NewState = check_enum(Value, Enum, State),
+  check_value(Value, Attrs, NewState);
 check_value(Value, [{?FORMAT, Format} | Attrs], State) ->
   NewState = check_format(Value, Format, State),
   check_value(Value, Attrs, NewState);
@@ -310,7 +369,17 @@
 %% @private
 is_type_valid(Value, ?STRING)  -> is_binary(Value);
 is_type_valid(Value, ?NUMBER)  -> is_number(Value);
-is_type_valid(Value, ?INTEGER) -> is_integer(Value);
+%%
+%% Draft 6
+%%
+%% In draft-04"integer" is listed as a primitive type and defined as
+%% “a JSON number without a fraction or exponent part”; in draft-06, "integer"
+%% is not considered a primitive type and is only defined in the section for
+%% keyword "type" as “any number with a zero fractional part”; 1.0 is thus not a
+%% valid "integer" type in draft-04 and earlier, but is a valid "integer" type
+%% in draft-06 and later; note that both drafts say that integers SHOULD be
+%% encoded in JSON without fractional parts
+is_type_valid(Value, ?INTEGER) -> is_number(Value);
 is_type_valid(Value, ?BOOLEAN) -> is_boolean(Value);
 is_type_valid(Value, ?OBJECT)  -> jesse_lib:is_json_object(Value);
 is_type_valid(Value, ?ARRAY)   -> jesse_lib:is_array(Value);
@@ -326,61 +395,22 @@
 wrong_type(Value, State) ->
   handle_data_invalid(?wrong_type, Value, State).
 
-%% @doc 5.4.4. additionalProperties, properties and patternProperties
-%%
-%% 5.4.4.1. Valid values
-%%
-%%   The value of "additionalProperties" MUST be a boolean or an object. If it
-%%   is an object, it MUST also be a valid JSON Schema.
-%%
-%%   The value of "properties" MUST be an object. Each value of this object MUST
-%%   be an object, and each object MUST be a valid JSON Schema.
-%%
-%%   The value of "patternProperties" MUST be an object. Each property name of
-%%   this object SHOULD be a valid regular expression, according to the ECMA 262
-%%   regular expression dialect. Each property value of this object MUST be an
-%%   object, and each object MUST be a valid JSON Schema.
-%%
-%% 5.4.4.2. Conditions for successful validation
-%%
-%%   Successful validation of an object instance against these three keywords
-%%   depends on the value of "additionalProperties":
-%%
-%%     if its value is boolean true or a schema, validation succeeds;
-%%
-%%     if its value is boolean false, the algorithm to determine validation
-%%     success is described below.
-%%
-%% 5.4.4.3. Default values
-%%
-%%   If either "properties" or "patternProperties" are absent, they can be
-%%   considered present with an empty object as a value.
-%%
-%%   If "additionalProperties" is absent, it may be considered present with an
-%%   empty schema as a value.
-%%
-%% 5.4.4.4. If "additionalProperties" has boolean value false
-%%
-%%   In this case, validation of the instance depends on the property set of
-%%   "properties" and "patternProperties". In this section, the property names
-%%   of "patternProperties" will be called regexes for convenience.
-%%
-%%   The first step is to collect the following sets:
-%%
-%%     s  - The property set of the instance to validate.
-%%     p  - The property set from "properties".
-%%     pp - The property set from "patternProperties".
+
+%% @doc 6.20.  additionalProperties
 %%
-%%   Having collected these three sets, the process is as follows:
+%% The value of "additionalProperties" MUST be a valid JSON Schema.
 %%
-%%     remove from "s" all elements of "p", if any;
+%% This keyword determines how child instances validate for objects, and
+%% does not directly validate the immediate instance itself.
 %%
-%%     for each regex in "pp", remove all elements of "s" which this regex
-%%     matches.
+%% Validation with "additionalProperties" applies only to the child
+%% values of instance names that do not match any names in "properties",
+%% and do not match any regular expression in "patternProperties".
 %%
-%%   Validation of the instance succeeds if, after these two steps, set "s" is
-%%   empty.
+%% For all such properties, validation succeeds if the child instance
+%% validates against the "additionalProperties" schema.
 %%
+%% Omitting this keyword has the same behavior as an empty schema.
 %% @private
 check_properties(Value, Properties, State) ->
   TmpState
@@ -390,8 +420,7 @@
                            CurrentState;
                          Property ->
                            NewState = set_current_schema( CurrentState
-                                                        , PropertySchema
-                                                        ),
+                                                        , PropertySchema),
                            check_value( PropertyName
                                       , Property
                                       , PropertySchema
@@ -417,6 +446,17 @@
                         ),
   set_current_schema(TmpState, get_current_schema(State)).
 
+check_property_names(Value, PatternProperties, State) ->
+  P1P2 = [{P1, P2} || P1 <- unwrap(Value), P2  <- unwrap(PatternProperties)],
+  TmpState = lists:foldl(
+               fun({Property, Pattern}, CurrentState) ->
+                   check_match_property(Property, Pattern, CurrentState)
+               end
+              , State
+              , P1P2
+              ),
+  set_current_schema(TmpState, get_current_schema(State)).
+
 %% @private
 check_match({PropertyName, PropertyValue}, {Pattern, Schema}, State) ->
   case re:run(PropertyName, Pattern, [{capture, none}, unicode]) of
@@ -430,6 +470,16 @@
       State
   end.
 
+%% @private
+check_match_property({PropertyName, _}, {_, Regex}, State) ->
+  case re:run(PropertyName, Regex, [{capture, none}, unicode]) of
+    match   ->
+      State;
+    nomatch ->
+      handle_data_invalid(?no_match, PropertyName, State)
+  end.
+
+
 %% @doc additionalProperties
 %% See check_properties/3.
 %% @private
@@ -507,38 +557,22 @@
            end,
   lists:filter(Filter, ExtraNames).
 
-%% @doc 5.3.1. additionalItems and items
-%%
-%% 5.3.1.1. Valid values
-%%
-%%   The value of "additionalItems" MUST be either a boolean or an object. If it
-%%   is an object, this object MUST be a valid JSON Schema.
+%% @doc 6.10. additionalItems and items
 %%
-%%   The value of "items" MUST be either an object or an array. If it is an
-%%   object, this object MUST be a valid JSON Schema. If it is an array, items
-%%   of this array MUST be objects, and each of these objects MUST be a valid
-%%   JSON Schema.
+%% The value of "additionalItems" MUST be a valid JSON Schema.
 %%
-%% 5.3.1.2. Conditions for successful validation
+%% This keyword determines how child instances validate for arrays, and
+%% does not directly validate the immediate instance itself.
 %%
-%%   Successful validation of an array instance with regards to these two
-%%   keywords is determined as follows:
+%% If "items" is an array of schemas, validation succeeds if every
+%% instance element at a position greater than the size of "items"
+%% validates against "additionalItems".
 %%
-%%     if "items" is not present, or its value is an object, validation of the
-%%     instance always succeeds, regardless of the value of "additionalItems";
-%%
-%%     if the value of "additionalItems" is boolean value true or an object,
-%%     validation of the instance always succeeds;
-%%
-%%     if the value of "additionalItems" is boolean value false and the value of
-%%     "items" is an array, the instance is valid if its size is less than, or
-%%     equal to, the size of "items".
-%%
-%% 5.3.1.4. Default values
-%%
-%%   If either keyword is absent, it may be considered present with an empty
-%%   schema.
+%% Otherwise, "additionalItems" MUST be ignored, as the "items" schema
+%% (possibly the default value of an empty schema) is applied to all
+%% elements.
 %%
+%% Omitting this keyword has the same behavior as an empty schema.
 %% @private
 check_items(Value, Items, State) ->
   case jesse_lib:is_json_object(Items) of
@@ -562,6 +596,26 @@
       handle_schema_invalid({?wrong_type_items, Items}, State)
   end.
 
+check_contains([], _Schema, State) ->
+  State;
+check_contains(Values, Schema, State) ->
+  DefaultAssumption = {false, State},
+  Result = lists:foldl(fun (Value, Acc) ->
+                         case Acc of
+                           {false, _} ->
+                             validate_schema(Value, Schema, State);
+                           {true, _} ->
+                             Acc
+                         end
+                       end, DefaultAssumption, Values),
+  case Result of
+    {true, _} ->
+      State;
+    {false, _} ->
+      handle_data_invalid(?data_invalid, Values, State)
+  end.
+
+
 %% @private
 check_items_array(Value, Items, State) ->
   JsonSchema = get_current_schema(State),
@@ -603,36 +657,25 @@
                              ),
   set_current_schema(TmpState, get_current_schema(State)).
 
-%% @doc 5.4.5.  dependencies
-%%
-%% 5.4.5.1. Valid values
+%% @doc 6.21.  dependencies
 %%
-%%   This keyword's value MUST be an object. Each value of this object MUST be
-%%   either an object or an array.
+%%   This keyword specifies rules that are evaluated if the instance is an
+%%   object and contains a certain property.
 %%
-%%   If the value is an object, it MUST be a valid JSON Schema. This is called a
-%%   schema dependency.
-%%
-%%   If the value is an array, it MUST have at least one element. Each element
-%%   MUST be a string, and elements in the array MUST be unique. This is called
-%%   a property dependency.
-%%
-%% 5.4.5.2. Conditions for successful validation
-%%
-%%   5.4.5.2.1. Schema dependencies
-%%
-%%     For all (name, schema) pair of schema dependencies, if the instance has a
-%%     property by this name, then it must also validate successfully against
-%%     the schema.
-%%
-%%     Note that this is the instance itself which must validate successfully,
-%%     not the value associated with the property name.
+%%   This keyword's value MUST be an object.  Each property specifies a
+%%   dependency.  Each dependency value MUST be an array or a valid JSON
+%%   Schema.
 %%
-%%   5.4.5.2.2. Property dependencies
+%%   If the dependency value is a subschema, and the dependency key is a
+%%   property in the instance, the entire instance must validate against
+%%   the dependency value.
+%%
+%%   If the dependency value is an array, each element in the array, if
+%%   any, MUST be a string, and MUST be unique.  If the dependency key is
+%%   a property in the instance, each of the items in the dependency value
+%%   must be a property that exists in the instance.
 %%
-%%     For each (name, propertyset) pair of property dependencies, if the
-%%     instance has a property by this name, then it must also have properties
-%%     with the same names as propertyset.
+%%   Omitting this keyword has the same behavior as an empty object.
 %%
 %% @private
 check_dependencies(Value, Dependencies, State) ->
@@ -693,99 +736,78 @@
              , State
              , Dependency
              ).
-
-%% @doc 5.1.3. minimum and exclusiveMinimum
-%%
-%% 5.1.3.1. Valid values
-%%
-%%   The value of "minimum" MUST be a JSON number. The value of
-%%   "exclusiveMinimum" MUST be a boolean.
-%%
-%%   If "exclusiveMinimum" is present, "minimum" MUST also be present.
 %%
-%% 5.1.3.2. Conditions for successful validation
+%% 6.4.  minimum
 %%
-%%   Successful validation depends on the presence and value of
-%%   "exclusiveMinimum":
+%%    The value of "minimum" MUST be a number, representing an inclusive
+%%    upper limit for a numeric instance.
 %%
-%%     if "exclusiveMinimum" is not present, or has boolean value false, then
-%%     the instance is valid if it is greater than, or equal to, the value of
-%%     "minimum";
+%%    If the instance is a number, then this keyword validates only if the
+%%    instance is greater than or exactly equal to "minimum".
 %%
-%%     if "exclusiveMinimum" is present and has boolean value true, the instance
-%%     is valid if it is strictly greater than the value of "minimum".
+%% 6.5.  exclusiveMinimum
 %%
-%% 5.1.3.3. Default value
+%%    The value of "exclusiveMinimum" MUST be number, representing an
+%%    exclusive upper limit for a numeric instance.
 %%
-%%   "exclusiveMinimum", if absent, may be considered as being present with
-%%   boolean value false.
+%%    If the instance is a number, then the instance is valid only if it
+%%    has a value strictly greater than (not equal to) "exclusiveMinimum".
 %%
 %% @private
-check_minimum(Value, Minimum, ExclusiveMinimum, State) ->
-  Result = case ExclusiveMinimum of
-             true -> Value > Minimum;
-             _    -> Value >= Minimum
-           end,
-  case Result of
+check_minimum(Value, Minimum, State) ->
+  case (Value >= Minimum) of
     true  -> State;
     false ->
       handle_data_invalid(?not_in_range, Value, State)
   end.
 
-%% @doc 5.1.2. maximum and exclusiveMaximum
-%%
-%% 5.1.2.1. Valid values
-%%
-%%   The value of "maximum" MUST be a JSON number. The value of
-%%   "exclusiveMaximum" MUST be a boolean.
-%%
-%%   If "exclusiveMaximum" is present, "maximum" MUST also be present.
-%%
-%% 5.1.2.2. Conditions for successful validation
+check_exclusive_minimum(Value, ExclusiveMinimum, State) ->
+  case (Value > ExclusiveMinimum) of
+    true  -> State;
+    false ->
+      handle_data_invalid(?not_in_range, Value, State)
+  end.
+
+
+%% @doc 6.2. maximum and exclusiveMaximum
 %%
-%%   Successful validation depends on the presence and value of
-%%   "exclusiveMaximum":
+%% The value of "maximum" MUST be a number, representing an inclusive
+%% upper limit for a numeric instance.
 %%
-%%     if "exclusiveMaximum" is not present, or has boolean value false, then
-%%     the instance is valid if it is lower than, or equal to, the value of
-%%     "maximum";
+%% If the instance is a number, then this keyword validates only if the
+%% instance is less than or exactly equal to "maximum".
 %%
-%%     if "exclusiveMaximum" has boolean value true, the instance is valid if it
-%%     is strictly lower than the value of "maximum".
+%% 6.3.  exclusiveMaximum
 %%
-%% 5.1.2.3. Default value
+%% The value of "exclusiveMaximum" MUST be number, representing an
+%% exclusive upper limit for a numeric instance.
 %%
-%%   "exclusiveMaximum", if absent, may be considered as being present with
-%%   boolean value false.
+%% If the instance is a number, then the instance is valid only if it
+%% has a value strictly less than (not equal to) "exclusiveMaximum".
 %%
 %% @private
-check_maximum(Value, Maximum, ExclusiveMaximum, State) ->
-  Result = case ExclusiveMaximum of
-             true -> Value < Maximum;
-             _    -> Value =< Maximum
-           end,
-  case Result of
+check_maximum(Value, Maximum, State) ->
+  case (Value =< Maximum) of
     true  -> State;
     false ->
       handle_data_invalid(?not_in_range, Value, State)
   end.
 
-%% @doc 5.3.3.  minItems
-%%
-%% 5.3.3.1. Valid values
-%%
-%%   The value of this keyword MUST be an integer. This integer MUST be greater
-%%   than, or equal to, 0.
-%%
-%% 5.3.3.2.  Conditions for successful validation
+check_exclusive_maximum(Value, ExclusiveMaximum, State) ->
+  case (Value < ExclusiveMaximum) of
+    true  -> State;
+    false ->
+      handle_data_invalid(?not_in_range, Value, State)
+  end.
+
+%% @doc 6.12.  minItems
 %%
-%%   An array instance is valid against "minItems" if its size is greater than,
-%%   or equal to, the value of this keyword.
+%% The value of this keyword MUST be a non-negative integer.
 %%
-%% 5.3.3.3. Default value
+%% An array instance is valid against "minItems" if its size is greater
+%% than, or equal to, the value of this keyword.
 %%
-%%   If this keyword is not present, it may be considered present with a value
-%%   of 0.
+%% Omitting this keyword has the same behavior as a value of 0.
 %%
 %% @private
 check_min_items(Value, MinItems, State) when length(Value) >= MinItems ->
@@ -793,17 +815,12 @@
 check_min_items(Value, _MinItems, State) ->
   handle_data_invalid(?wrong_size, Value, State).
 
-%% @doc 5.3.2. maxItems
-%%
-%% 5.3.2.1. Valid values
-%%
-%%   The value of this keyword MUST be an integer. This integer MUST be greater
-%%   than, or equal to, 0.
+%% @doc 6.11. maxItems
 %%
-%% 5.3.2.2. Conditions for successful validation
+%% The value of this keyword MUST be a non-negative integer.
 %%
-%%   An array instance is valid against "maxItems" if its size is less than, or
-%%   equal to, the value of this keyword.
+%% An array instance is valid against "maxItems" if its size is less
+%% than, or equal to, the value of this keyword.
 %%
 %% @private
 check_max_items(Value, MaxItems, State) when length(Value) =< MaxItems ->
@@ -811,22 +828,15 @@
 check_max_items(Value, _MaxItems, State) ->
   handle_data_invalid(?wrong_size, Value, State).
 
-%% @doc 5.3.4. uniqueItems
-%%
-%% 5.3.4.1. Valid values
-%%
-%%   The value of this keyword MUST be a boolean.
-%%
-%% 5.3.4.2. Conditions for successful validation
+%% @doc 6.13. uniqueItems
 %%
-%%   If this keyword has boolean value false, the instance validates
-%%   successfully. If it has boolean value true, the instance validates
-%%   successfully if all of its elements are unique.
+%% The value of this keyword MUST be a boolean.
 %%
-%% 5.3.4.3. Default value
+%% If this keyword has boolean value false, the instance validates
+%% successfully.  If it has boolean value true, the instance validates
+%% successfully if all of its elements are unique.
 %%
-%%   If not present, this keyword may be considered present with boolean value
-%%   false.
+%% Omitting this keyword has the same behavior as a value of false.
 %%
 %% @private
 check_unique_items(_, false, State) ->
@@ -857,18 +867,16 @@
     throw:ErrorInfo -> handle_data_invalid(ErrorInfo, Value, State)
   end.
 
-%% @doc 5.2.3. pattern
+%% @doc 6.8. pattern
 %%
-%% 5.2.3.1. Valid values
+%% The value of this keyword MUST be a string.  This string SHOULD be a
+%% valid regular expression, according to the ECMA 262 regular
+%% expression dialect.
+%%
+%% A string instance is considered valid if the regular expression
+%% matches the instance successfully.  Recall: regular expressions are
+%% not implicitly anchored.
 %%
-%%   The value of this keyword MUST be a string. This string SHOULD be a valid
-%%   regular expression, according to the ECMA 262 regular expression dialect.
-%%
-%% 5.2.3.2. Conditions for successful validation
-%%
-%%   A string instance is considered valid if the regular expression matches
-%%   the instance successfully. Recall: regular expressions are not implicitly
-%%   anchored.
 %% @private
 check_pattern(Value, Pattern, State) ->
   case re:run(Value, Pattern, [{capture, none}, unicode]) of
@@ -877,25 +885,18 @@
       handle_data_invalid(?no_match, Value, State)
   end.
 
-%% @doc 5.2.2.  minLength
-%%
-%% 5.2.2.1. Valid values
+%% @doc 6.7.  minLength
 %%
-%%   The value of this keyword MUST be an integer. This integer MUST be greater
-%%   than, or equal to, 0.
+%% The value of this keyword MUST be a non-negative integer.
 %%
-%% 5.2.2.2. Conditions for successful validation
+%% A string instance is valid against this keyword if its length is
+%% greater than, or equal to, the value of this keyword.
 %%
-%%   A string instance is valid against this keyword if its length is greater
-%%   than, or equal to, the value of this keyword.
+%% The length of a string instance is defined as the number of its
+%% characters as defined by RFC 7159 [RFC7159].
 %%
-%%   The length of a string instance is defined as the number of its characters
-%%   as defined by RFC 4627 [RFC4627].
+%% Omitting this keyword has the same behavior as a value of 0.
 %%
-%% 5.2.2.3. Default value
-%%
-%%   "minLength", if absent, may be considered as being present with integer
-%%   value 0.
 %% @private
 check_min_length(Value, MinLength, State) ->
   case length(unicode:characters_to_list(Value)) >= MinLength of
@@ -904,20 +905,15 @@
       handle_data_invalid(?wrong_length, Value, State)
   end.
 
-%% @doc 5.2.1. maxLength
-%%
-%% 5.2.1.1. Valid values
+%% @doc 6.6. maxLengthmaxLength
 %%
-%%   The value of this keyword MUST be an integer. This integer MUST be greater
-%%   than, or equal to, 0.
+%% The value of this keyword MUST be a non-negative integer.
 %%
-%% 5.2.1.2. Conditions for successful validation
+%% A string instance is valid against this keyword if its length is less
+%% than, or equal to, the value of this keyword.
 %%
-%%   A string instance is valid against this keyword if its length is less than,
-%%   or equal to, the value of this keyword.
-%%
-%%   The length of a string instance is defined as the number of its characters
-%%   as defined by RFC 4627 [RFC4627].
+%% The length of a string instance is defined as the number of its
+%% characters as defined by RFC 7159 [RFC7159]
 %%
 %% @private
 check_max_length(Value, MaxLength, State) ->
@@ -927,19 +923,15 @@
       handle_data_invalid(?wrong_length, Value, State)
   end.
 
-%% @doc 5.5.1. enum
-%%
-%% 5.5.1.1. Valid values
-%%
-%%   The value of this keyword MUST be an array. This array MUST have at least
-%%   one element. Elements in the array MUST be unique.
+%% @doc 6.23. enum
 %%
-%%   Elements in the array MAY be of any type, including null.
+%% The value of this keyword MUST be an array.  This array SHOULD have
+%% at least one element.  Elements in the array SHOULD be unique.
 %%
-%% 5.5.1.2. Conditions for successful validation
+%% An instance validates successfully against this keyword if its value
+%% is equal to one of the elements in this keyword's array value.
 %%
-%%   An instance validates successfully against this keyword if its value is
-%%   equal to one of the elements in this keyword's array value.
+%% Elements in the array might be of any value, including null.
 %%
 %% @private
 check_enum(Value, Enum, State) ->
@@ -985,20 +977,30 @@
 check_format(Value, _Format = <<"uri">>, State) when is_binary(Value) ->
   %% not yet supported
   State;
+check_format(Value, <<"uri-reference">>, State) when is_binary(Value) ->
+  uri_reference(Value, State);
 check_format(_Value, _Format, State) ->
   State.
 
-%% @doc 5.1.1. multipleOf
-%%
-%% 5.1.1.1. Valid values
-%%
-%%   The value of "multipleOf" MUST be a JSON number. This number MUST be
-%%   strictly greater than 0.
+-ifdef(OTP_RELEASE).
+uri_reference(Value, State) when is_binary(Value) ->
+  case uri_string:parse(Value) of
+    {error, _ErrorType, _Term} ->
+      handle_data_invalid(?wrong_format, Value, State);
+    _ -> State
+  end.
+-else.
+uri_reference(_Value, State) ->
+  State.
+-endif.
+
+
+%% @doc 6.1. multipleOf
 %%
-%% 5.1.1.2. Conditions for successful validation
+%% The value of "multipleOf" MUST be a number, strictly greater than 0.
 %%
-%%   A numeric instance is valid against "multipleOf" if the result of the
-%%   division of the instance by this keyword's value is an integer.
+%% A numeric instance is valid only if division by this keyword's value
+%% results in an integer.
 %%
 %% @private
 check_multiple_of(Value, MultipleOf, State)
@@ -1013,19 +1015,19 @@
 check_multiple_of(_Value, _MultipleOf, State) ->
   handle_schema_invalid(?wrong_multiple_of, State).
 
-%% @doc 5.4.3. required
-%%
-%% 5.4.3.1. Valid values
+%% @doc 6.17. required
 %%
-%%   The value of this keyword MUST be an array. This array MUST have at least
-%%   one element. Elements of this array MUST be strings, and MUST be unique.
+%% The value of this keyword MUST be an array.  Elements of this array,
+%% if any, MUST be strings, and MUST be unique.
 %%
-%% 5.4.3.2. Conditions for successful validation
+%% An object instance is valid against this keyword if every item in the
+%% array is the name of a property in the instance.
 %%
-%%   An object instance is valid against this keyword if its property set
-%%   contains all elements in this keyword's array value.
+%% Omitting this keyword has the same behavior as an empty array.
 %%
 %% @private
+check_required(Value, [] = Required, State) ->
+  check_required_values(Value, Required, State);
 check_required(Value, [_ | _] = Required, State) ->
   check_required_values(Value, Required, State);
 check_required(_Value, _InvalidRequired, State) ->
@@ -1042,17 +1044,12 @@
       check_required_values(Value, Required, State)
   end.
 
-%% @doc 5.4.1. maxProperties
-%%
-%% 5.4.1.1. Valid values
-%%
-%%   The value of this keyword MUST be an integer. This integer MUST be greater
-%%   than, or equal to, 0.
+%% @doc 6.15. maxProperties
 %%
-%% 5.4.1.2.Conditions for successful validation
+%% The value of this keyword MUST be a non-negative integer.
 %%
-%%   An object instance is valid against "maxProperties" if its number of
-%%   properties is less than, or equal to, the value of this keyword.
+%% An object instance is valid against "maxProperties" if its number of
+%% properties is less than, or equal to, the value of this keyword.
 %%
 %% @private
 check_max_properties(Value, MaxProperties, State)
@@ -1064,22 +1061,14 @@
 check_max_properties(_Value, _MaxProperties, State) ->
   handle_schema_invalid(?wrong_max_properties, State).
 
-%% @doc 5.4.2. minProperties
-%%
-%% 5.4.2.1. Valid values
-%%
-%%   The value of this keyword MUST be an integer. This integer MUST be greater
-%%   than, or equal to, 0.
-%%
-%% 5.4.2.2. Conditions for successful validation
+%% @doc 6.16. minProperties
 %%
-%%   An object instance is valid against "minProperties" if its number of
-%%   properties is greater than, or equal to, the value of this keyword.
+%% The value of this keyword MUST be a non-negative integer.
 %%
-%% 5.4.2.3. Default value
+%% An object instance is valid against "minProperties" if its number of
+%% properties is greater than, or equal to, the value of this keyword.
 %%
-%%   If this keyword is not present, it may be considered present with a value
-%%   of 0.
+%% Omitting this keyword has the same behavior as a value of 0.
 %%
 %% @private
 check_min_properties(Value, MinProperties, State)
@@ -1091,20 +1080,14 @@
 check_min_properties(_Value, _MaxProperties, State) ->
   handle_schema_invalid(?wrong_min_properties, State).
 
-%% @doc 5.5.3. allOf
+%% @doc 6.26. allOf
 %%
-%% 5.5.3.1. Valid values
+%% This keyword's value MUST be a non-empty array.  Each item of the
+%% array MUST be a valid JSON Schema.
 %%
-%%   This keyword's value MUST be an array. This array MUST have at least one
-%%   element.
-%%
-%%   Elements of the array MUST be objects. Each object MUST be a valid JSON
-%%   Schema.
-%%
-%% 5.5.3.2. Conditions for successful validation
-%%
-%%   An instance validates successfully against this keyword if it validates
-%%   successfully against all schemas defined by this keyword's value.
+%% An instance validates successfully against this keyword if it
+%% validates successfully against all schemas defined by this keyword's
+%% value.
 %%
 %% @private
 check_all_of(Value, [_ | _] = Schemas, State) ->
@@ -1122,20 +1105,14 @@
       handle_data_invalid({?all_schemas_not_valid, Errors}, Value, State)
   end.
 
-%% @doc 5.5.4. anyOf
+%% @doc 6.27. anyOf
 %%
-%% 5.5.4.1. Valid values
+%% This keyword's value MUST be a non-empty array.  Each item of the
+%% array MUST be a valid JSON Schema.
 %%
-%%   This keyword's value MUST be an array. This array MUST have at least one
-%%   element.
-%%
-%%   Elements of the array MUST be objects. Each object MUST be a valid JSON
-%%   Schema.
-%%
-%% 5.5.4.2. Conditions for successful validation
-%%
-%%   An instance validates successfully against this keyword if it validates
-%%   successfully against at least one schema defined by this keyword's value.
+%% An instance validates successfully against this keyword if it
+%% validates successfully against at least one schema defined by this
+%% keyword's value.
 %%
 %% @private
 check_any_of(Value, [_ | _] = Schemas, State) ->
@@ -1163,20 +1140,14 @@
       check_any_of_(Value, Schemas, State, shortest(NewErrors, Errors))
   end.
 
-%% @doc 5.5.5. oneOf
-%%
-%% 5.5.5.1. Valid values
-%%
-%%   This keyword's value MUST be an array. This array MUST have at least one
-%%   element.
+%% @doc 6.28. oneOf
 %%
-%%   Elements of the array MUST be objects. Each object MUST be a valid JSON
-%%   Schema.
-%%
-%% 5.5.5.2. Conditions for successful validation
+%% This keyword's value MUST be a non-empty array.  Each item of the
+%% array MUST be a valid JSON Schema.
 %%
-%%   An instance validates successfully against this keyword if it validates
-%%   successfully against exactly one schema defined by this keyword's value.
+%% An instance validates successfully against this keyword if it
+%% validates successfully against exactly one schema defined by this
+%% keyword's value.
 %%
 %% @private
 check_one_of(Value, [_ | _] = Schemas, State) ->
@@ -1207,17 +1178,12 @@
       check_one_of_(Value, Schemas, State, Valid, Errors ++ NewErrors)
   end.
 
-%% @doc 5.5.6. not
-%%
-%% 5.5.6.1. Valid values
-%%
-%%   This keyword's value MUST be an object. This object MUST be a valid JSON
-%%   Schema.
+%% @doc 6.29. not
 %%
-%% 5.5.6.2. Conditions for successful validation
+%% This keyword's value MUST be a valid JSON Schema.
 %%
-%%   An instance is valid against this keyword if it fails to validate
-%%   successfully against the schema defined by this keyword.
+%% An instance is valid against this keyword if it fails to validate
+%% successfully against the schema defined by this keyword.
 %%
 %% @private
 check_not(Value, Schema, State) ->

Copy link
Contributor

@seriyps seriyps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! Thanks, that's amazing work!
Could you, please, also update the README.md, because it mentions in several places that we only support draft-03 and draft-04

%% @doc Json schema validation module.
%%
%% This module is the core of jesse, it implements the validation functionality
%% according to the standard(draft4).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
%% according to the standard(draft4).
%% according to the standard(draft6).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think I found all the places, which were the README and this suggestion :)

@seriyps
Copy link
Contributor

seriyps commented May 24, 2021

What I also noticed is that the test coverage of the new module is quite low compared to the other modules:

$ rebar3 eunit -c
$ rebar3 ct -c
$ rebar3 cover -v
<..>
  |--------------------------|------------|
  |                  module  |  coverage  |
  |--------------------------|------------|
  |                   jesse  |       26%  |
  |               jesse_cli  |        0%  |
  |          jesse_database  |        0%  |
  |             jesse_error  |       65%  |
  |         jesse_json_path  |       77%  |
  |               jesse_lib  |       91%  |
  |  jesse_schema_validator  |      100%  |
  |             jesse_state  |       73%  |
  |        jesse_tests_util  |       93%  |
  |  jesse_validator_draft3  |       92%  |
  |  jesse_validator_draft4  |       90%  |
  |  jesse_validator_draft6  |       28%  |
  |--------------------------|------------|
  |                   total  |       62%  |
  |--------------------------|------------|

Do you think there is any way we can improve that? Or it is because of #107 ?

@kikofernandez
Copy link
Author

Do you think there is any way we can improve that? Or it is because of #107 ?

I am pretty sure this is regarding #107. I have up our game and can report a 44% correctness. For the remaining, I think it is crucial that #107 is fixed. Otherwise, I can write manual tests that will be superseeded by #107.

The latest changes (8278c6f) test all drafts where possible.

Copy link
Contributor

@seriyps seriyps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, I can write manual tests that will be superseeded by #107.

Don't think this is needed, because, as far as I see, all the new clauses which were introduced in this PR which are new compared to draft-04 are covered. And the ones which were copied from draft-04 without changes are sort-of covered by draft-04 tests. So, it can wait for #107 I think.
Would be nice to update this section of README as well, but this is minor https://github.com/for-GET/jesse#json-schema-versions

So, from my point of view it looks all good. Would be nice to get an approval from @andreineculau

@kikofernandez
Copy link
Author

Would be nice to update this section of README as well, but this is minor https://github.com/for-GET/jesse#json-schema-versions

Done. Thank you so much for the feedback :)

Copy link
Member

@andreineculau andreineculau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First and foremost - thanks for the contribution! Many will be thrilled with the update. cc #61 @handrews

See inline for some requested changes. And add another one: please bump the external tests as you correctly mention in #107 to enable draft6 tests.

README.md Show resolved Hide resolved
Comment on lines 308 to 311
case jesse_json_path:value(?SCHEMA, Schema, undefined) of
?json_schema_draft6 -> get_schema_id_by_version(?IDv6, Schema);
_ -> get_schema_id_by_version(?ID, Schema)
end.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have at least one other place where you need to decide if you're going to use $id or id.

I was going to say that it would be better to extract that to it's own function e.g. get_schema_id_prop_by_version(Schema) but unless I'm mistaken the problem is slightly more complex as Schema doesn't need to be a JSON Schema draft, at least not directly.

The next best thing to traversing all the schemata in search of a draft number is a prioritization of $id over id. Not perfect but it would do for now.

Copy link

@handrews handrews May 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but unless I'm mistaken the problem is slightly more complex as Schema doesn't need to be a JSON Schema draft, at least not directly

Yeah, $schema meta-schema URIs are technically allowed to be anything, which makes them of questionable use determining syntax and semantics. An unrecognized meta-schema URI may actually be just like the standard except for a tiny syntax restriction (e.g. forbid the format keyword), but there's no programmatic way to know that in draft-06 or draft-07.

Starting in draft 2019-09 (which would have been draft-08, except for reasons), this is handled through the vocabularies concept. $vocabulary is a keyword that describes JSON Schema keyword semantics in the instance document. Which means it is only useful if the instance document is a schema, so in practice it's only used in meta-schemas. You can have however many meta-schema URIs describing all sorts of combinations of vocabularies and even syntax restrictions (because meta-schemas validate syntax just like before), but there's a small and generally well-known set of vocabulary URIs that tells you what code to actually run.

In the meantime, in practice it's fine to check for draft-04, draft-06, draft-07, etc., or just require the user to specify a draft through configuration or command line options.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, even more complex than initially thought :) Thanks for your input.
My complexity bounds were regarding the possibility that $schema can point to https://example.com/andreis-schema, which in turn extends say draft-6.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It ends up being fairly simple- https://example.com/andreis-schema would list the same standard vocabularies as the standard meta-schema, allOf the standard meta-schema, and also describe whatever extension you add. You can make that extension a vocabulary if you want other implementations to be able to implement it, or you can just assume that your target implementation understands it (as you would now).

It sounds more complicated because there's a new concept, but vocabularies are more modular and likely to remain a relatively small stable set than meta-schemas, which have never functioned well as a keyword extension system.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, I am not suggesting that you add support for anything like this now!

src/jesse_schema_validator.hrl Outdated Show resolved Hide resolved
@andreineculau
Copy link
Member

@kikofernandez I had a go to update the upstream tests (I used their master, since their last tag 2.0.0 is also 2018-old), and it revealed that the current jesse implementation fails a few tests on draft 3 and 4. Maybe you have already tried and know all this.

So while I still keep to having their draft6 tests running for this PR to go through (it will certify the quality of the draft6 implementation), I think the reasonable thing to do is to ignore any draft6 tests that are failing across draft3 or 4 as well i.e. implementation that is anyway broken today. Whatever is failing can later be logged and fixed accordingly. We will have failing CI runs because we added more GOOD tests, which is more valuable than having a fake green CI :)

One more thing: I should have clarified something from the beginning, which is not to shy away from telling us "guys, hackathon has ended so I have no time right now for this". Your contribution is as valuable!

@kikofernandez
Copy link
Author

Hi! Thanks for your words.
I would like to see if I can continue a bit on this PR, but time is limited. If you push somewhere your changes for the upstream tests, I can see which ones are uniquely failing for draft6.

Thanks for the feedback and the go to update the upstream tests :)
I will try to reply to your specific comments from above in a timely manner.

Thanks a lot.

@andreineculau andreineculau mentioned this pull request May 26, 2021
7 tasks
@seriyps
Copy link
Contributor

seriyps commented Dec 14, 2021

We merged the updated test suites, but now there are some merge conflicts.
ButI believe as soon as conflicts are resolved, we should be able to merge this PR.

UPD: well, ok, not just "resolve the conflicts", but now I think jesse_tests_draft6_SUITE.erl needs to be created to run the official test suite over v6 implementation

@seriyps
Copy link
Contributor

seriyps commented Jan 27, 2022

So, after I rebased it it seems to work. Now I'm going to add jesse_tests_draft6_SUITE.erl to run the official test cases over it, will see if it passes.

@seriyps
Copy link
Contributor

seriyps commented Jan 27, 2022

Well, quite a lot of tests are failing. I atleast see that https://datatracker.ietf.org/doc/html/draft-wright-json-schema-01#section-4.4

Boolean values are equivalent to the following behaviors:

true Always passes validation, as if the empty schema {}
false Always fails validation, as if the schema { "not":{} }

is not implemented in a generic way, but rather ad-hoc for items =(
I'll see what I can do

@seriyps
Copy link
Contributor

seriyps commented Jan 27, 2022

Wow, that was not easy. Official test suite passes now (with a few exceptions). It was a good idea to not merge it before the test suite is available @andreineculau !

  |--------------------------|------------|
  |                  module  |  coverage  |
  |--------------------------|------------|
  |                   jesse  |       26%  |
  |               jesse_cli  |        0%  |
  |          jesse_database  |        0%  |
  |             jesse_error  |       65%  |
  |         jesse_json_path  |       77%  |
  |               jesse_lib  |       95%  |
  |  jesse_schema_validator  |      100%  |
  |             jesse_state  |       76%  |
  |        jesse_tests_util  |       92%  |
  |  jesse_validator_draft3  |       92%  |
  |  jesse_validator_draft4  |       90%  |
  |  jesse_validator_draft6  |       84%  |
  |--------------------------|------------|
  |                   total  |       78%  |
  |--------------------------|------------|

@seriyps
Copy link
Contributor

seriyps commented Feb 1, 2022

Fixed some more review comments and enabled remaining test cases from official test suite (have tofix a few bugs). Some cases are still broken (listed in SkipList), I think there is some issue in how we resolve references, but I'm out of energy to debug it. We have the same kind of problem in draft-04 version as well. I may open a separate issue as a reminder to fix those.

Otherwise, I think it's ready for inclusion. WDYT @andreineculau ?
@kikofernandez please have some final looks if you want to add anything

@kikofernandez
Copy link
Author

@seriyps this looks amazing! Thank you so much for finishing it up. I hope it did not require too many changes, and from the history it is difficult to see. Thanks a lot!

My final thought is whether my name should be added to the AUTHOR list, but I understand that my effort may be seen as a contribution and not as AUTHOR. Either way, thanks a lot!

@kikofernandez
Copy link
Author

In the next Klarna Hackathon, I will start working on Draft 7 :)

@seriyps
Copy link
Contributor

seriyps commented Feb 2, 2022

from the history it is difficult to see

the changes I made are the last 4 commits. Rebase did not require much changes, so commits you made are pretty much the same

My final thought is whether my name should be added to the AUTHOR list, but I understand that my effort may be seen as a contribution and not as AUTHOR.

Well, please do! Just push one more commit with updated AUTHORS on top.

@seriyps seriyps merged commit eb35eff into for-GET:master Feb 8, 2022
@seriyps
Copy link
Contributor

seriyps commented Feb 8, 2022

Ok, thank you @kikofernandez ! Great work!

@jadeallenx
Copy link

Thanks for this! V exciting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants