Request with missing nested map param throws validation error when "type: :map" specified for param

[theirishpenguin]

Thanks for you work on the project @ozziexsh !

I have found a issue when I am working with nested parameters. Here is my example...

The Problem

• Given I have the following validation rules defined - especially with type: :map specified for the "address" param

   def rules(conn) do                           
     dbg() # <---- Callout: conn.params is %{} here                                                                                                                                                                                                                                                                           
     %{                                                                                                                                                                                                                                                                                                                    
       "address" => [                                                                                                                                                                                                                                                                                                         
         required: false,                                                                                                                                                                                                                                                                                                  
         # Callout: The next line seems to cause the problem                                                                                                                                                                                              
         type: :map,                                                                                                                                                                                                                                                                                                     
         nullable: true,                                                                                                                                                                                                                                                                                                   
         map: %{                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              
           "house_number" => [required: false, nullable: true, cast: :integer, between: {1, 50}]                                                                                                                                                                                                                          
         }                                                                                                                                                                                                                                                                                                                 
       ]                                                                                                                                                                                                                                                                                                                   
     }                                                                                                                                                                                                                                                                                                                     
   end       

• When I make a simple GET request in which I don't supply any "address" param
• Then I get the following validation errors...

[       
  %Validate.Validator.Error{
    path: ["address"],
    message: "expected map received nil",
    rule: :type
  }
]

Workaround
If I comment up the type: :map specified for the "address" param and repeat the experiment I get no validation errors (as expected). ie.

   def rules(conn) do                           
     dbg() # <---- Callout: conn.params is %{} here                                                                                                                                                                                                                                                                           
     %{                                                                                                                                                                                                                                                                                                                    
       "address" => [                                                                                                                                                                                                                                                                                                         
         required: false,                                                                                                                                                                                                                                                                                                  
         # NB: Important: to *not* set type here or else map will blow up when no "page" params supplied at all                                                                                                                                                                                                            
         # type: :map,                                                                                                                                                                                                                                                                                                     
         nullable: true,                                                                                                                                                                                                                                                                                                   
         map: %{                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              
           "house_number" => [required: false, nullable: true, cast: :integer, between: {1, 50}]                                                                                                                                                                                                                          
         }                                                                                                                                                                                                                                                                                                                 
       ]                                                                                                                                                                                                                                                                                                                   
     }                                                                                                                                                                                                                                                                                                                     
   end       

Here is the example in a unit test...

   test "it should allow a nested map param to *not* be supplied when type: :map and nullable: true and specified" do                                                                                                                                                                                                                                                               
     rules =%{                                                                                                                                                                                                                                                                                                             
       "address" => [                                                                                                                                                                                                                                                                                                      
         required: false,                                                                                                                                                                                                                                                                                                  
         # Callout: The next line seems to cause the problem                                                                                                                                                                                                                                                               
         type: :map,                                                                                                                                                                                                                                                                                                       
         nullable: true,                                                                                                                                                                                                                                                                                                   
         map: %{                                                                                                                                                                                                                                                                                                           
           "house_number" => [required: false, nullable: true, cast: :integer, between: {1, 50}]                                                                                                                                                                                                                           
         }                                                                                                                                                                                                                                                                                                                 
       ]                                                                                                                                                                                                                                                                                                                   
     }                                                                                                                                                                                                                                                                                                                     
                                                                                                                                                                                                                                                                                                                           
     # No address key supplied                                                                                                                                                                                                                                                                         
     input = %{}                                                                                                                                                                                                                                                                                                           
                                                                                                                                                                                                                                                                                                                           
     assert {:ok, _} = Validate.validate(input, rules)                                                                                                                                                                                                                                                                     
   end      

This test fails. If you comment out the type: map line it passes.

My Expectation
This issue occurred when I was used the plug / form request approach. In that scenario, when I stop submitting an "address" param with the HTTP request, the request param obviously does not get sent at all to the server (ie. I won't get a %{"address" => nil}, rather I will get %{} and that is problematic. I am not sure if the "nullable" option is behaving as you would expect in the above scenario - maybe it is. If it is behaving as expected then maybe a allow_missing: true option could be added? This would allow a param to be absent as well as present but nil - though that I what I would expect the required: false to allow me to do.

P.S. I can workaround the issue by not supplying type: :map of course - but that seems like a hack - really what I should be able to specify here is a map that can be missing.

[ozziexsh]

hey thanks for raising this! sorry in advance for formatting as i am travelling for the next week and typing this via the app

the order of the rules do matter, which i need to document better

if you move the nullable rule before the type: :map rule it should work how you expect. this is because the nullable rule will prevent any other rules from firing if the value is nil. also it works with a missing key (in this case "address") because of how Map.get() returns nil when it doesnt find it

it would be nice to add a "presence" rule though like you mentioned, so ill look into that when i'm back!

[theirishpenguin]

Hi @ozziexsh, thanks for the reply while traveling. Impressive formatting while on the move 😄

I tried out your suggestion to move the nullable rule before the type: :map rule and it worked nicely, thanks!

One suggestion on that (and no worries if it is too much work to implement - it's just a thought), if your library could hoist the nullable rule before the type: :map rule whenever it encounters both rules when parsing the rules then that would make your DSL more deterministic and order independent in this situation (I'm assuming that there is never a good reason to put the nullable rule after the type: :map rule but that could be a mistaken assumption). Something like a per-processing step that makes adjustments like this in advance.

A "presence" rule sounds good 👍 I guess it would be an interesting area to document. I guess "required" means that a key and value are required (then setting "nullable" allows the value to be nil). Whereas "presence" would be that a key is required (this begs the question, how would presence" be meaningfully different from required: true with a nullable: true... and maybe there is a way in which "required" can be made more flexible/robust... or not). Sorry, if I've misunderstood this - I'm context switching a bit this morning as I write. My takeaway point is something like, "Do we need :nullable and :required and :presence or is there some redundancy that can be eliminated between these 3 concepts without breaking any existing compatibility?"

No rush replying. Enjoy the travels and I'm happy to take up this again when you get back or when you get a chance to look at this. Especially, now that you've explained the rule ordering, I'm unblocked.