Skip to content
This repository was archived by the owner on Jul 15, 2025. It is now read-only.

check response input is json#63

Merged
backwardspy merged 1 commit intomasterfrom
feature/MER-1958-api-reflector-check-the-input-is-a-valid-json-when-content-type-is-application-json
Jan 6, 2023
Merged

check response input is json#63
backwardspy merged 1 commit intomasterfrom
feature/MER-1958-api-reflector-check-the-input-is-a-valid-json-when-content-type-is-application-json

Conversation

@mmjjoozz
Copy link
Copy Markdown
Contributor

@mmjjoozz mmjjoozz commented Jan 4, 2023

Copy link
Copy Markdown
Owner

@backwardspy backwardspy left a comment

Choose a reason for hiding this comment

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

this needs to check content type is application/json before trying to validate the content, otherwise we will lose support for non json responses.

additionally, i can see the appeal of doing this sort of validation for other content types in future. perhaps consider moving the validation into its own function and mapping content types to validators.

perhaps something along these lines:

def is_valid_json(data: str) -> None:
	json.loads(data)

validators = {
	"application/json": is_valid_json,
}

def on_model_change(...):
	if validator := validators.get(form.content_type.data):
		validator(form.content.data)
	super.on_model_change(...)

Copy link
Copy Markdown
Contributor

@Mick-Cadac Mick-Cadac left a comment

Choose a reason for hiding this comment

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

As well as Chris's comment you need to make sure AC2 is covered too. The content field can be left empty, no validation is required for this case.

Copy link
Copy Markdown
Owner

@backwardspy backwardspy left a comment

Choose a reason for hiding this comment

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

lgtm

currently supports the application/json content type with space for more in future.
@backwardspy backwardspy force-pushed the feature/MER-1958-api-reflector-check-the-input-is-a-valid-json-when-content-type-is-application-json branch from d982cc4 to 367c657 Compare January 6, 2023 15:51
@backwardspy backwardspy merged commit 4f6f2a1 into master Jan 6, 2023
@backwardspy backwardspy deleted the feature/MER-1958-api-reflector-check-the-input-is-a-valid-json-when-content-type-is-application-json branch January 6, 2023 15:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants