Rest API does not validate documents created

wale · January 18, 2022, 9:16am

Hi ,

I discovered that documents created via the ERPNext rest api are not getting validated. For example, a non-existent document can be referenced in a link field and the doc still get’s created!

Is there a flag that can be used to ensure docs created via rest api are validated ?

Thanks

ankush · January 18, 2022, 9:45am

can you confirm if the field is actually a link field?

API just passes the data to get_doc which inserts it as if you inserted it from Desk. There shouldn’t be any difference!

github.com

frappe/frappe/blob/9c9c62d71c529f7b524c317ae334a670a3c9a4b9/frappe/api.py#L138-L139


      
          					# insert document from request data
          					doc = frappe.get_doc(data).insert()

wale · January 18, 2022, 2:48pm

Hi @ankush

Thanks for your reply. Yes I’m sure it’s a link field. I was referring to POST requests, not GET. For instance, the doc below was created via API but the value for Terminal ID doesn’t exist!

Clicking on the link leads to this…

Any ideas ?

Thanks

wale · January 19, 2022, 5:48pm

I think the issue is coming from the insert part… is there a missing flag or is this a bug ?

Kind regards,

brian_pond · January 19, 2022, 11:52pm

@wale
Difficult to know whether these are bugs, “working as intended”, or simply considered non-issues by the maintainers.

Like you, I’ve also encountered many strange behaviors with the APIs and document model. Here’s an easy one:

Create a POST to the Supplier doctype.
Pass the following JSON payload:

{
	"name": "this will be ignored",
	"supplier_name": "Acme Supply Co.",
	"supplier_group": "Distributor",
	"supplier_type": "Individual",
	"foo": "so will this",
	"bar": "and this too"
}

Send this POST, and you will receive back an HTTP 200; no warnings.

The ‘name’ key is silently ignored because the DocType is set to choose a name automatically based on the Naming Series. The HTTP response will have a completely different value for ‘name’.
The keys ‘foo’ and ‘bar’ are silently ignored too, because there are no such DocFields in the Supplier document.

This particular lack of validation drove me crazy during a recent project. People were calling my APIs, expecting that the ERP saved all the values transmitted. When in fact, they were silently discarded. Ultimately, I had to just start adding code to document.py and api.py. And create new validation checks to reject invalid API calls, like the one above.

Unrelated to APIs specifically, there are other hidden “gotchas” in the document model. For example, the optional ‘validate()’ controller method is called before the Document checks that mandatory fields are populated. So, when writing code in validate(), you could be working with None or 0 values. Values that won’t get recognized and rejected until later in the process of saving a record.

wale · January 21, 2022, 10:04am

Hi @ankush

Should I go ahead and raise an issue or do you have any suggestions ?

Thanks

bahaou · January 21, 2022, 10:41am

@wale why don’t you just do the validation before insert it ?

wale · January 21, 2022, 2:17pm

Hi @bahaou

Could you pls explain how to achieve this on an API call ?

Thanks

bahaou · January 21, 2022, 2:23pm

before inserting the data , use a get call to check if id exists . where exactly are you calling the api ?

ankush · January 21, 2022, 2:42pm

I can’t reproduce the issue you’re describing.

linkval.jpg

You can open an issue but without the ability to reproduce what you’re facing on a different site, it’s unlikely that someone can fix it.

If you want to debug it yourself put breakpoint() in insert method and see why _validate_links isn’t getting called on your site.

wale · January 21, 2022, 3:32pm

This may not work in our case as the API is being called by external devices and there is no GET option

wale · January 21, 2022, 3:37pm

Hi @ankush

I suspect that’s because you’re using a Standard Doctype (ie Item). The API call also fails at our end when we try creating an Item with non-existent Item Group and that’s how it should be. The issue however still persists with Custom doctypes. Please test this on a Custom Doctype

Thanks

bahaou · January 21, 2022, 3:40pm

there is an option called ignore_links that can be called during an insert or save event . I dont know how exactly it works with api . but it can be the problem

ankush · January 21, 2022, 3:56pm

Well, tried with custom doctype too:

Screenshot 2022-01-21 at 9.17.08 PM.jpg

brian_pond · January 21, 2022, 10:27pm

I did the following test on a vanilla Frappe v13.8.1

Created a custom DocType.
Added a field to the custom DocType, of type Link, which itself linked to another custom DocType.
Performed an HTTP POST, but with an invalid value.

I got a 417 response, same as @ankush

 {
  "session_expired": 1,
  "exc_type": "LinkValidationError",
  "exc": "[\"Traceback (most recent call last):\\n  File \\\"/erpnext/mybench/apps/frappe/frappe/app.py\\\", line 68, in application\\n    response = frappe.api.handle()\\n  File \\\"/erpnext/mybench/apps/frappe/frappe/api.py\\\", line 177, in handle\\n    doc = frappe.get_doc(data).insert()\\n  File \\\"/erpnext/mybench/apps/frappe/frappe/model/document.py\\\", line 258, in insert\\n    self._validate_links()\\n  File \\\"/erpnext/mybench/apps/frappe/frappe/model/document.py\\\", line 874, in _validate_links\\n    frappe.throw(_(\\\"Invalid link. Could not find {0}\\\").format(msg),\\n  File \\\"/erpnext/mybench/apps/frappe/frappe/__init__.py\\\", line 462, in throw\\n    msgprint(msg, raise_exception=exc, title=title, indicator='red', is_minimizable=is_minimizable, wide=wide, as_list=as_list)\\n  File \\\"/erpnext/mybench/apps/frappe/frappe/__init__.py\\\", line 441, in msgprint\\n    _raise_exception()\\n  File \\\"/erpnext/mybench/apps/frappe/frappe/__init__.py\\\", line 376, in _raise_exception\\n    raise raise_exception(msg)\\nfrappe.exceptions.LinkValidationError: Invalid link. Could not find Baz: 12345\\n\"]",
  "_server_messages": "[\"{\\\"message\\\": \\\"Invalid link. 
Could not find Baz: 12345\\\", \\\"indicator\\\": \\\"red\\\", \\\"raise_exception\\\": 1}\"]"
}

(when I changed my JSON payload to use a real value, I verified that I received back a 200)

wale · January 24, 2022, 10:58am

Hi @ankush @brian_pond

Thanks for your responses and tests. I’ve spent a good part of the weekend trying to diagnose the issue but still haven’t gotten exactly what it is. It however must be somehow related to that particular field because when I rename it to something else (e.g. terminalid1) the validation occurs as expected but once I revert to the original fieldname (terminalid), there’s no validation!

Any ideas?

Its important for us to use the correct field name

brian_pond · January 24, 2022, 5:17pm

@wale

Here’s a sequence you could try.

First, execute a 'bench migrate' command, to ensure the SQL schema is synchronized.
Next, examine the SQL table itself. Delete any leftover columns that no longer exist

(remember, the framework -adds- columns during ‘bench migrate’, but never deletes and cleans-up any unused columns.)

Run a 'bench clear-cache' command.
Delete the Python bytecode files. These often cause me headaches. The bytecode files (those with extension *.pyc), sometimes don’t reflect the real, current Python code. I remove them by changing to my frappe-bench directory, and running this command.

find . -name *.pyc -delete

Restart the Frappe web server.

This should clear up any ‘cached’ behaviors, and ensure that everything is running as-per-code.

If this doesn’t do the trick? Then I’d suggest getting a 2nd person involved. Have a developer review your DocTypes and code, and perhaps debug the Python code from start to finish.

wale · February 1, 2022, 5:13pm

Hi @brian_pond @ankush

Trust y’all had a great weekend. So after much investigation and testing, I think I found the bug. If you add another field to your doctype and set that field to ‘Fetch From’ the link field, the link field is no longer validated!

So just for clarity, you can replicate using the following steps:

Create a doctype with 2 fields… Field A (Link Field) and Field B (Data Field)
Set the ‘Fetch From’ option in Field B so that it fetches it’s value from a valid field in the doctype that Field A links to (ie field_a.some_valid_field)
Make an API call to create the doctype using a non-existent value for Field A

You will notice the doc gets created successfully without validating the link!

Thanks

brian_pond · February 1, 2022, 7:43pm

Hi @wale: Not able to duplicate on my side.

Here’s the DocType I created:

image1164×315 21.2 KB
The 3rd field has a Fetch From:

When I do an HTTP POST, with an invalid Customer value, I get a 500 response:

curl --request POST \
  --url http://localhost:8000/api/resource/LinkTest \
  --header 'Authorization: token abc:123' \
  --header 'Content-Type: application/jsonapplication/json' \
  --cookie 'sid=Guest; system_user=no; full_name=Guest; user_id=Guest; user_image=' \
  --data '{
	"description": "Some description",
	"customer": "Foo"
}'

wale · February 2, 2022, 5:00pm

Wow… this is really puzzling. Guess I’m better off looking into writing custom APIs to meet the immediate need while I keep digging to uncover the source of this issue

Thanks plenty