Adding YAML file verification.

lunrong · Dec 14, 2012 · 427577c · 427577c
1 parent 3510d67
commit 427577c
Show file tree

Hide file tree

Showing 4 changed files with 166 additions and 83 deletions.
diff --git a/base.py b/base.py
@@ -54,19 +54,33 @@ def JsonError(self, error_message, code=404):
     self.response.out.write(json.dumps({'error': str(error_message)}))
 
   def handle_exception(self, exception, debug_mode):
+    """Overwrites the default exception handler."""
     logging.error('Exception raised: %s' % exception)
+
     if isinstance(exception, self.NotLoggedInException):
       self.redirect(users.create_login_url(self.request.uri))
       return
+
     if isinstance(exception, self.UnauthorizedUserException):
       self.error(401)
-      self.response.out.write('401 Unauthorized: %s' % exception)
+      self.response.out.write(json.dumps(
+          {'code': '401 Unauthorized', 'error': str(exception)}))
+      return
+
+    if isinstance(exception, self.InvalidInputException):
+      self.error(400)
+      self.response.out.write(json.dumps(
+          {'code': '400 Bad Request', 'error': str(exception)}))
       return
+
     webapp2.RequestHandler.handle_exception(self, exception, debug_mode)
     logging.error('Exception was not handled: %s' % exception)
 
   class UnauthorizedUserException(Exception):
     """Error class for unauthorized access."""
 
   class NotLoggedInException(Exception):
-    """Error class for users that are not logged in."""
+    """Error class for users that are not logged in (error code 401)."""
+
+  class InvalidInputException(Exception):
+    """Error class for invalid input on the user's side (error code 400)."""
diff --git a/converter.py b/converter.py
@@ -22,29 +22,84 @@
 
 EDITOR_MODE = 'editor'
 
-def Import(yaml_file):
-    """Converts a YAML file to an exploration and saves it in the datastore."""
-    # TODO(sll): Enforce the following constraints.
-    # - There must be at least one state.
-    # - Each state_name must exist and be unique. No state may be named 'END'.
-    # - Content is optional, treated as [] if non-existent.
-    # - input_type.widget is optional, treated as default for given input type
-    #   if non-existent
-    # - input_type.name is optional, the default is exact match. If present, it
-    #   must be one of some enum, otherwise it goes to the default.
-    # - answers.default.dest must exist, and be valid.
-    # - All dest states must be valid.
-    #
-    # NB: the first state in the list is the initial state.
-
-    exploration = yaml.safe_load(yaml_file)
-
-    logging.info(exploration)
-    return exploration
+
+def ValidateState(description):
+  """Validates a state representation.
+
+  This enforces the following constraints:
+  - The only accepted fields are ['answers', 'content', 'input_type'].
+  - Permitted subfields of 'content' are ['text', 'image', 'video', 'widget'].
+  - Permitted subfields of 'input_type' are ['name', 'widget'].
+  - Each item in 'answers' must have exactly one key.
+  - 'content' is optional and defaults to [].
+  - input_type.name is optional and defaults to 'none'. If it exists, it must be
+    one of ['none', 'multiple_choice', 'int', 'set', 'text'].
+    - If input_type.name == 'none' and there is more than one answer category, an
+      error is thrown. The name of the answer category can be anything; it is
+      ignored.
+  - input_type.widget is optional and defaults to the default for the given
+        input type.
+  - If input_type != 'multiple_choice' then answers.default.dest must exist, and
+    be the last one in the list.
+  - If input_type == 'multiple_choice' then 'answers' must not be non-empty.
+  """
+  logging.info(description)
+  if 'answers' not in description or len(description['answers']) == 0:
+    return False, 'No answer choices supplied'
+
+  if 'content' not in description:
+    description['content'] = []
+  if 'input_type' not in description:
+    description['input_type'] = {'name': 'none'}
+
+  for key in description:
+    if key not in ['answers', 'content', 'input_type']:
+      return False, 'Invalid key: %s' % key
+
+  for item in description['content']:
+    if len(item) != 1:
+      return False, 'Invalid content item: %s' % item
+    for key in item:
+      if key not in ['text', 'image', 'video', 'widget']:
+        return False, 'Invalid key in content array: %s' % key
+
+  for key in description['input_type']:
+    if key not in ['name', 'widget']:
+      return False, 'Invalid key in input_type: %s' % key
+
+  if (description['input_type']['name'] not in
+      ['none', 'multiple_choice', 'int', 'set', 'text']):
+    return False, 'Invalid key in input_type.name: %s' % description['input_type']['name']
+
+  for item in description['answers']:
+    if len(item) != 1:
+      return False, 'Invalid answer item: %s' % item
+
+  if description['input_type']['name'] == 'none' and len(description['answers']) > 1:
+    return False, 'Expected only a single answer for a state with no input'
+
+  if description['input_type']['name'] != 'multiple_choice':
+    if description['answers'][-1].keys() != ['default']:
+      return False, 'The last category of the answers array should be \'default\''
+
+  return True, ''
 
 
 class ImportPage(editor.BaseHandler):
-  """Imports a YAML file and creates an exploration."""
+  """Imports a YAML file and creates a state from it."""
+
+  def Import(self, yaml_file):
+      """Converts a YAML file into a state description."""
+      try:
+        description = yaml.safe_load(yaml_file)
+      except yaml.YAMLError as e:
+        raise self.InvalidInputException(e)
+
+      is_valid, error_log = ValidateState(description)
+      if is_valid:
+        return description
+      else:
+        raise self.InvalidInputException(error_log)
 
   def get(self, exploration_id):  # pylint: disable-msg=C6409
     """Handles GET requests."""
@@ -64,63 +119,69 @@ def put(self, exploration_id):  # pylint: disable-msg=C6409
       exploration_id: string representing the exploration id.
     """
     user, exploration = self.GetUserAndExploration(exploration_id)
+    state_id = self.request.get('state_id')
+    if not state_id:
+      raise self.InvalidInputException('No state id received.')
+    state = utils.GetEntity(models.State, state_id)
 
+    state_name = self.request.get('state_name')
     yaml_file = self.request.get('yaml_file')
-    if not yaml_file:
-      self.JsonError('No exploration data received.')
-      return
-
-    description = Import(yaml_file)
-
-    # Delete all states belonging to this exploration first.
-    for state in exploration.states:
-      state.delete()
-    exploration.states = []
-
-    init_state_found = False
-
-    for state_name in description:
-      state_hash_id = utils.GetNewId(models.State, state_name)
-      input_view_name = 'none'
-      if ('input_type' in description[state_name] and
-          'name' in description[state_name]['input_type']):
-        input_view_name = description[state_name]['input_type']['name']
-      input_view = models.InputView.gql(
-          'WHERE name = :name', name=input_view_name).get()
-
-      content = []
-      for dic in description[state_name]['content']:
-        content_item = {}
-        for key, val in dic.iteritems():
-          content_item['type'] = key
-          content_item['value'] = val
-          content.append(content_item)
-
-      action_set_list = []
-      for index in range(len(description[state_name]['answers'])):
-        for key, val in description[state_name]['answers'][index].iteritems():
-          # TODO(sll): add destination information here (remember that it could
-          # be 'END').
-          action_set = models.ActionSet(category_index=index, text=val['text'])
-          action_set.put()
-          action_set_list.append(action_set.key)
-
-      state = models.State(
-          name=state_name, hash_id=state_hash_id, text=content,
-          input_view=input_view.key, action_sets=action_set_list,
-          parent=exploration.key)
+    if not state_name and not yaml_file:
+      raise self.InvalidInputException('No data received.')
+    if state_name and yaml_file:
+      raise self.InvalidInputException(
+          'Only one of the state name and the state description can be edited'
+          'at a time')
+
+    if state_name:
+      # Replace the state name with this one, after checking validity.
+      if state_name == 'END':
+        raise self.InvalidInputException('Invalid state name: END')
+      # Check that no other state has this name.
+      if (state_name != state.name and utils.CheckExistenceOfName(
+              models.State, state_name, exploration)):
+          raise self.InvalidInputException(
+              'Duplicate state name: %s', state_name)
+      state.name = state_name
       state.put()
+      return
 
-      action_set.dest = state.key
-      action_set.put()
-      exploration.states.append(state.key)
-
-      # TODO(sll): ensure that this actually finds the correct initial state.
-      if not init_state_found:
-        init_state_found = True
-        exploration.init_state = state.key
-
-    exploration.put()
+    # Otherwise, a YAML file has been passed in.
+    description = self.Import(yaml_file)
+
+    input_view_name = 'none'
+    if ('input_type' in description and 'name' in description['input_type']):
+      input_view_name = description['input_type']['name']
+    input_view = models.InputView.gql(
+        'WHERE name = :name', name=input_view_name).get()
+
+    content = []
+    for dic in description['content']:
+      content_item = {}
+      for key, val in dic.iteritems():
+        content_item['type'] = key
+        content_item['value'] = val
+        content.append(content_item)
+    action_set_list = []
+    for index in range(len(description['answers'])):
+      for key, val in description['answers'][index].iteritems():
+        # TODO(sll): add destination information here (remember that it could
+        # be 'END').
+        # TODO(sll): If a dest state does not exist, it needs to be created. States
+        # are referred to by their name in 'description'.
+        action_set = models.ActionSet(category_index=index, text=val['text'])
+        action_set.put()
+        action_set_list.append(action_set.key)
+
+    state.input_view = input_view.key
+    if state_name:
+      state.name = state_name
+    state.text = content
+    state.action_sets = action_set_list
+    state.put()
+
+    action_set.dest = state.key
+    action_set.put()
 
     self.response.out.write(json.dumps({
         'explorationId': exploration_id,

diff --git a/static/templates/yaml_editor.html b/static/templates/yaml_editor.html
@@ -2,6 +2,13 @@
     <div class="container-fluid">
       <div class="oppia-content row-fluid">
 
+        <div ng-repeat="warning in warnings">
+          <p class="alert alert-block">
+            <button type="button" class="close" data-dismiss="alert">&times;</button>
+            <[warning]>
+          </p>
+        </div>
+
         <h3>
           <span> <[stateName]> </span>
         </h3>

diff --git a/templates/dev/head/js/editorExploration.js b/templates/dev/head/js/editorExploration.js
@@ -1283,26 +1283,27 @@ function YamlEditor($scope, $http) {
 
   // TODO(sll): Initialize this default with the current status of the
   // exploration in the datastore.
-  $scope.yaml = 'Activity 1:\n  answers:\n  - default:\n      dest: END\n' +
-      '      text: First category\n  content:\n  - text: Text1\n  - text: Text2\n  ' +
-      '- image: 41pMJXnVrf\n  - video: jd5ffc48RJU\n  - widget: Yp9mxyOKSa\n' +
-      '  input_type:\n    name: set';
+  $scope.yaml = 'answers:\n- default:\n    dest: END\n' +
+      '    text: First category\ncontent:\n- text: Text1\n- text: Text2\n' +
+      '- image: 41pMJXnVrf\n- video: jd5ffc48RJU\n- widget: Yp9mxyOKSa\n' +
+      'input_type:\n  name: set';
 
+  // TODO(sll): Add a handler for state_name.
   /**
    * Saves the YAML representation of a state.
    */
-  // TODO(sll): Change this to save the representation of a state, not an
-  // exploration.
   $scope.saveState = function() {
     $http.put(
         '/create/convert/' + $scope.explorationId,
-        'yaml_file=' + encodeURIComponent($scope.yaml),
+        'state_id=' + $scope.stateId +
+            '&yaml_file=' + encodeURIComponent($scope.yaml),
         {headers: {'Content-Type': 'application/x-www-form-urlencoded'}}).
             success(function(data) {
               console.log(data);
+              // Update the $scope.states vars here.
             }).error(function(data) {
               $scope.addWarning(data.error ||
-                  'Error: Could not add new exploration.');
+                  'Error: Could not add new state.');
             });
   };
 }