Skip to content

Commit

Permalink
Add schema for handler classes editor, library modules (oppia#13422)
Browse files Browse the repository at this point in the history
  • Loading branch information
Nik-09 authored Aug 20, 2021
1 parent a171b39 commit e124a4f
Show file tree
Hide file tree
Showing 18 changed files with 1,060 additions and 309 deletions.
4 changes: 3 additions & 1 deletion core/controllers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -397,9 +397,11 @@ def validate_and_normalize_args(self):
'Missing schema for %s method in %s handler class.' % (
request_method, handler_class_name))

allow_string_to_bool_conversion = request_method in ['GET', 'DELETE']
normalized_arg_values, errors = (
payload_validator.validate(
handler_args, schema_for_request_method, extra_args_are_allowed)
handler_args, schema_for_request_method, extra_args_are_allowed,
allow_string_to_bool_conversion)
)

self.normalized_payload = {
Expand Down
14 changes: 12 additions & 2 deletions core/controllers/base_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1442,7 +1442,11 @@ def test_default_value_in_schema_conforms_with_schema(self):
default_value_schema = {arg: schema}

_, errors = payload_validator.validate(
default_value, default_value_schema, True)
default_value,
default_value_schema,
allowed_extra_args=True,
allow_string_to_bool_conversion=False
)
if len(errors) == 0:
continue
self.log_line(
Expand Down Expand Up @@ -1641,6 +1645,12 @@ class MockHandlerWithDefaultGetSchema(base.BaseHandler):
'type': 'basestring'
},
'default_value': 'random_exp_id'
},
'apply_draft': {
'schema': {
'type': 'bool'
},
'default_value': False
}
}
}
Expand Down Expand Up @@ -1731,7 +1741,7 @@ def test_can_access_exploration_with_default_value_in_schema(self):
self.login(self.OWNER_EMAIL)

with self.swap(self, 'testapp', self.mock_testapp3):
self.get_json('/mock_play_exploration')
self.get_json('/mock_play_exploration?apply_draft=true')

csrf_token = self.get_new_csrf_token()
with self.swap(self, 'testapp', self.mock_testapp4):
Expand Down
47 changes: 27 additions & 20 deletions core/controllers/domain_objects_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,24 @@
from core.domain import collection_domain
from core.domain import config_domain
from core.domain import exp_domain
from core.domain import state_domain
import python_utils

from typing import Any, Dict # isort:skip pylint: disable=wrong-import-order, wrong-import-position, unused-import, import-only-modules
from typing import Any, Dict, Optional, Union # isort:skip


def validate_exploration_change(obj):
# type: (Dict[String, Any]) -> None
def validate_exploration_change(obj: Dict[str, Any]) -> None:
"""Validates exploration change.
Args:
obj: dict. Data that needs to be validated.
"""
# No explicit call to validate_dict method is necessary, because
# ExplorationChange calls validate method while initialization.
exp_domain.ExplorationChange(obj)
exp_domain.ExplorationChange(obj) # type: ignore[no-untyped-call]


def validate_new_config_property_values(obj):
# type: (Dict[String, Any]) -> None
def validate_new_config_property_values(obj: Dict[str, Any]) -> None:
"""Validates new config property values.
Args:
Expand All @@ -56,52 +55,61 @@ def validate_new_config_property_values(obj):
raise Exception(
'config property name should be a string, received'
': %s' % name)
config_property = config_domain.Registry.get_config_property(name)
config_property = config_domain.Registry.get_config_property(name) # type: ignore[no-untyped-call]
if config_property is None:
raise Exception('%s do not have any schema.' % name)

config_property.normalize(value)


def validate_change_dict_for_blog_post(change_dict):
# type: (Dict[Any, Any]) -> None
def validate_change_dict_for_blog_post(change_dict: Dict[str, Any]) -> None:
"""Validates change_dict required for updating values of blog post.
Args:
change_dict: dict. Data that needs to be validated.
"""
if 'title' in change_dict:
blog_domain.BlogPost.require_valid_title(
blog_domain.BlogPost.require_valid_title( # type: ignore[no-untyped-call]
change_dict['title'], True)
if 'thumbnail_filename' in change_dict:
blog_domain.BlogPost.require_valid_thumbnail_filename(
blog_domain.BlogPost.require_valid_thumbnail_filename( # type: ignore[no-untyped-call]
change_dict['thumbnail_filename'])
if 'tags' in change_dict:
blog_domain.BlogPost.require_valid_tags(
blog_domain.BlogPost.require_valid_tags( # type: ignore[no-untyped-call]
change_dict['tags'], False)
# Validates that the tags in the change dict are from the list of
# default tags set by admin.
list_of_default_tags = config_domain.Registry.get_config_property(
list_of_default_tags = config_domain.Registry.get_config_property( # type: ignore[no-untyped-call]
'list_of_default_tags_for_blog_post').value
if not all(tag in list_of_default_tags for tag in change_dict['tags']):
raise Exception(
'Invalid tags provided. Tags not in default tags list.')


def validate_collection_change(obj):
# type: (Dict[String, Any]) -> None
def validate_collection_change(obj: Dict[str, Any]) -> None:
"""Validates collection change.
Args:
obj: dict. Data that needs to be validated.
"""
# No explicit call to validate_dict method is necessary, because
# CollectionChange calls validate method while initialization.
collection_domain.CollectionChange(obj)
collection_domain.CollectionChange(obj) # type: ignore[no-untyped-call]


def validate_email_dashboard_data(data):
# type: (Dict[String, Optional[Union[bool, int]]]) -> None
def validate_state_dict(state_dict: Dict[str, Any]) -> None:
"""Validates state dict.
Args:
state_dict: dict. Data that needs to be validated.
"""
validation_class = state_domain.State.from_dict(state_dict) # type: ignore[no-untyped-call]
validation_class.validate(None, True)


def validate_email_dashboard_data(
data: Dict[str, Optional[Union[bool, int]]]
) -> None:
"""Validates email dashboard data.
Args:
Expand All @@ -118,8 +126,7 @@ def validate_email_dashboard_data(data):
raise Exception('400 Invalid input for query.')


def validate_task_entries(task_entries):
# type: (Dict[String, Any]) -> None
def validate_task_entries(task_entries: Dict[str, Any]) -> None:
"""Validates the task entry dict.
Args:
Expand Down
114 changes: 94 additions & 20 deletions core/controllers/domain_objects_validator_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@
from core.tests import test_utils

import utils
from typing import Any, Dict # isort:skip


class ValidateExplorationChangeTests(test_utils.GenericTestBase):
"""Tests to validate domain objects coming from API."""

def test_incorrect_object_raises_exception(self):
# type: () -> None
def test_incorrect_object_raises_exception(self) -> None:
incorrect_change_dict = {
'old_value': '',
'property_name': 'title',
Expand All @@ -38,8 +38,7 @@ def test_incorrect_object_raises_exception(self):
domain_objects_validator.validate_exploration_change(
incorrect_change_dict)

def test_correct_object_do_not_raises_exception(self):
# type: () -> None
def test_correct_object_do_not_raises_exception(self) -> None:
correct_change_dict = {
'cmd': 'edit_exploration_property',
'new_value': 'arbitary_new_value',
Expand All @@ -53,8 +52,7 @@ def test_correct_object_do_not_raises_exception(self):
class ValidateCollectionChangeTests(test_utils.GenericTestBase):
"""Tests to validate domain objects coming from API."""

def test_incorrect_object_raises_exception(self):
# type: () -> None
def test_incorrect_object_raises_exception(self) -> None:
incorrect_change_dict = {
'old_value': '',
'property_name': 'title',
Expand All @@ -65,8 +63,7 @@ def test_incorrect_object_raises_exception(self):
domain_objects_validator.validate_collection_change(
incorrect_change_dict)

def test_correct_object_do_not_raises_exception(self):
# type: () -> None
def test_correct_object_do_not_raises_exception(self) -> None:
correct_change_dict = {
'cmd': 'edit_collection_property',
'new_value': 'arbitary_new_value',
Expand All @@ -80,21 +77,21 @@ def test_correct_object_do_not_raises_exception(self):
class ValidateNewConfigPropertyValuesTests(test_utils.GenericTestBase):
"""Tests to validate config properties dict coming from API."""

def test_invalid_object_raises_exception(self):
def test_invalid_object_raises_exception(self) -> None:
config_properties = {'some_config_property': 20, }
with self.assertRaisesRegexp(
with self.assertRaisesRegexp( # type: ignore[no-untyped-call]
Exception, 'some_config_property do not have any schema.'):
domain_objects_validator.validate_new_config_property_values(
config_properties)

config_properties = {1234: 20, }
with self.assertRaisesRegexp(
config_properties = {1234: 20, } # type: ignore[dict-item]
with self.assertRaisesRegexp( # type: ignore[no-untyped-call]
Exception, 'config property name should be a string, received'
': %s' % 1234):
domain_objects_validator.validate_new_config_property_values(
config_properties)

def test_valid_object_raises_no_exception(self):
def test_valid_object_raises_no_exception(self) -> None:
config_properties = {
'max_number_of_tags_assigned_to_blog_post': 20,
}
Expand All @@ -106,38 +103,38 @@ class ValidateChangeDictForBlogPost(test_utils.GenericTestBase):
"""Tests to validate change_dict containing updated values for blog
post object coming from API."""

def test_invalid_title_raises_exception(self):
def test_invalid_title_raises_exception(self) -> None:
blog_post_change = {
'title': 123,
'tags': ['News'],
}
with self.assertRaisesRegexp(
with self.assertRaisesRegexp( # type: ignore[no-untyped-call]
utils.ValidationError, 'Title should be a string'):
domain_objects_validator.validate_change_dict_for_blog_post(
blog_post_change)

def test_invalid_tags_raises_exception(self):
def test_invalid_tags_raises_exception(self) -> None:
blog_post_change = {
'title': 'Hello Bloggers',
'tags': ['News', 'Some Tag'],
}
with self.assertRaisesRegexp(
with self.assertRaisesRegexp( # type: ignore[no-untyped-call]
Exception, 'Invalid tags provided. Tags not in default'
' tags list.'):
domain_objects_validator.validate_change_dict_for_blog_post(
blog_post_change)

blog_post_change = {
'title': 'Hello',
'tags': ['News', 123],
'tags': ['News', 123], # type: ignore[list-item]
}
with self.assertRaisesRegexp(
with self.assertRaisesRegexp( # type: ignore[no-untyped-call]
Exception, 'Expected each tag in \'tags\' to be a string,'
' received: \'123\''):
domain_objects_validator.validate_change_dict_for_blog_post(
blog_post_change)

def test_valid_dict_raises_no_exception(self):
def test_valid_dict_raises_no_exception(self) -> None:
blog_post_change = {
'title': 'Hello Bloggers',
'tags': ['News', 'Learners'],
Expand All @@ -152,3 +149,80 @@ def test_valid_dict_raises_no_exception(self):
}
domain_objects_validator.validate_change_dict_for_blog_post(
blog_post_change)


class ValidateStateDictInStateYamlHandler(test_utils.GenericTestBase):
"""Tests to validate state_dict of StateYamlHandler."""

def test_valid_object_raises_no_exception(self) -> None:
state_dict = {
'content': {'content_id': 'content', 'html': ''},
'param_changes': [],
'interaction': {
'solution': None,
'answer_groups': [],
'default_outcome': {
'param_changes': [],
'feedback': {
'content_id': 'default_outcome',
'html': ''
},
'dest': 'State A',
'refresher_exploration_id': None,
'missing_prerequisite_skill_id': None,
'labelled_as_correct': False
},
'customization_args': {
'rows': {
'value': 1
},
'placeholder': {
'value': {
'unicode_str': '',
'content_id': 'ca_placeholder_0'
}
}
},
'confirmed_unclassified_answers': [],
'id': 'TextInput',
'hints': []
},
'linked_skill_id': None,
'recorded_voiceovers': {
'voiceovers_mapping': {
'content': {},
'default_outcome': {},
'ca_placeholder_0': {}
}
},
'classifier_model_id': None,
'written_translations': {
'translations_mapping': {
'content': {},
'default_outcome': {},
'ca_placeholder_0': {}
}
},
'next_content_id_index': 1,
'card_is_checkpoint': False,
'solicit_answer_details': False
}
domain_objects_validator.validate_state_dict(state_dict)

def test_invalid_object_raises_exception(self) -> None:
invalid_state_dict: Dict[str, Any] = {
'classifier_model_id': None,
'written_translations': {
'translations_mapping': {
'content': {},
'default_outcome': {},
'ca_placeholder_0': {}
}
},
'next_content_id_index': 1,
'card_is_checkpoint': False,
'solicit_answer_details': False
}
# The error is representing the keyerror.
with self.assertRaisesRegexp(Exception, 'content'): # type: ignore[no-untyped-call]
domain_objects_validator.validate_state_dict(invalid_state_dict)
Loading

0 comments on commit e124a4f

Please sign in to comment.