Phoenix Update Behavior from Gen.JSON

Hi,
I am quite new to Phoenix and Elixir, but I love it already.
I just want to make sure the behavior I have observed is correct.
I use Phoenix 1.4.

I want to create a REST server.

  1. I created an app and installed all dependencies:
mix phx.new my-app --app my_app --no-html --no-webpack
  1. Created the DB:
cd my-app
mix ecto.create
  1. Generated a Product Schema
mix phx.gen.context Operations Product products product_name:string in_in_stock:boolean quantity:integer
  1. Then, migration:
mix ecto.migrate
  1. I generate a JSON CRUD with --no-context and --no-schema, since we already have them available:
mix phx.gen.json Operations Product products product_name:string in_in_stock:boolean quantity:integer --no-context --no-schema
  1. I add recommended resource endpoints to the router in the /api scope:
resources "/products", ProductController
  1. Then, I start the server in the dev mode with the local DB:
mix phx.server
  1. Finally, I can create the creation endpoint (by default all parameters are required):
    a) all parameters supplied - SUCCESS!
curl -H "Content-Type: application/json" -X POST -d '{"product":{"product_name":"Product1","in_in_stock":true, "quantity": 1}}' http://localhost:4000/api/products

b) missing one required parameter - FAILURE! (Correct behavior!)

curl -H "Content-Type: application/json" -X POST -d '{"product":{"product_name":"Product1","in_in_stock":true, "quantity1": 1}}' http://localhost:4000/api/products
  1. Here is when I am a little puzzled: PUT. I send a PUT request where none of the parameters in the body are correct and I get status 200OK, but the product does not change at all.
curl -H "Content-Type: application/json" -X PUT -d '{"product":{"product_name1":"Product1","in_in_stock1":true, "quantity1": 1}}' http://localhost:4000/api/products/1

Shouldn’t that return an error?
I tried changing Repo.update() to Repo.update!() in my_app/operations/operations.ex but it fails even with the correct data now.
How can I make sure that atleast one parameter (if not all that are supplied) are correct? How could I enforce all required parameters being passed in the PUT request?
What is the best practise?

Regards
Wojciech

The PUT method is an update… and as such, it should require a product id.

EDIT : Oh, It is in the path, sorry…

Because the keys You use are not in the changeset, it will cast nothing, and change nothing. Cast will simply discard wrong keys.

Try to change

"in_in_stock1":true

to

"in_in_stock":"WRONG DATA"

then it should fail, I think… or provide values that does not pass validation.

or simply provide new values to right keys, and see if it changes the record :slight_smile:

Providing wrong keys should result in a successful update, because there is nothing to change, this might be why You have a 200 status.

1 Like

It gets the product id at the end of endpoint (product with ID 1):

curl -H "Content-Type: application/json" -X PUT -d '{"product":{"product_name1":"Product1","in_in_stock1":true, "quantity1": 1}}' http://localhost:4000/api/products/1

Yes, you are right. When I use incorrect data types it works (I mean it throws errors as expected).
I will try not using casting for the update when I am back home.

You should use cast, it’s just expected that wrong keys are silently discarded.

1 Like

Yeah, sorry, you are correct.
I guess I can enforce the product_name key being present with pattern matching in my-app/operations/operations.ex:

%{"product_name" => _product_name} = attrs 

It is not the same as throwing an error when some wrong parameters are parsed, but that should be ok.
However, I am not sure how I could make to re-route a specific error page, like 400 if the match fails.

To force product_name presence, You should add a validation to your changeset.

And there should be no redirection with json api. Instead, you should render a json error.

2 Likes

Yeah, that makes sense.
I think I am getting there, just one piece missing.
I attempted to validate the attrs as follows:

def update_product(%Product{} = product, attrs) do
    new_update_validation = %Product{} |> Product.changeset(attrs)

    product
    |> Product.changeset(attrs)
    |> Repo.update()
  end

Variable new_update_validation correctly contains errors if I do not specify a required parameter. How should I proceed? I was thinking about using add error to the product. Is there any smarter way to do it? Thanks

Sorry, I thought I understood what You were doing, but I am not.

What I can tell is changeset takes 2 parameters, a product, and attributes.

When creating a product, You pass an empty product, with attributes, like You do here

%Product{} |> Product.changeset(attrs)

And when You update, You pass an existing product… like here

product |> Product.changeset(attrs)

I don’t see why You want to mix new validation with update validation.

When in update mode, You don’t need to pass a full set of attributes, just those who change. the rest is populated by previous data.

And if You try to pass wrong keys, they will not pass the casting test, and they will be discarded…

If You pass nothing, or wrong keys, update will still be valid, because the previous record is obviously valid.

Sorry for the confusion, I was probably explaining it incorrectly.
What I am trying to achieve can be explain better with few scenarios:

  1. You want to update a product and you specify none of the required parameters => Error
  2. You want to update a product and you specify only one of the required parameters => Error
  3. You want to update a product and you specify all required parameters => OK
  4. You want to update a product and you specify all required parameters and some other unrelated stuff => OK

You could describe operation I want to make as “update-all-required-parameters-or-fail”.

You will need to make your custom validation in that case, because it’s not the default behaviour…

Those 2 points are valid in the Phoenix way, because, when updating, You rely on existing data. And it is also the case in Rails, and probably more frameworks…

1 Like

Ok, then I guess it is the best practise. I just wanted to make sure that is the case. Thanks for the help @kokolegorille.