Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix setting values for entity reference and text fields on POST #74

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ before_script:
- curl -O https://www.drupal.org/files/issues/2086225-entity-access-check-node-create-3.patch
- patch -p1 /home/travis/build/restful/drupal/sites/all/modules/entity/modules/callbacks.inc < 2086225-entity-access-check-node-create-3.patch

- curl -O https://www.drupal.org/files/issues/2264079-entity-wrapper-access-single-entity-reference-1.patch
- patch -p1 /home/travis/build/restful/drupal/sites/all/modules/entity/includes/entity.wrapper.inc < 2264079-entity-wrapper-access-single-entity-reference-1.patch
- curl -O https://www.drupal.org/files/issues/2264079-entity-wrapper-access-single-entity-reference-2.patch
- patch -p1 /home/travis/build/restful/drupal/sites/all/modules/entity/includes/entity.wrapper.inc < 2264079-entity-wrapper-access-single-entity-reference-2.patch

# start a web server on port 8080, run in the background; wait for initialization
- drush runserver 127.0.0.1:8080 &
Expand Down
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ with the ``field_`` prefix
* Only JSON format is supported
* Audience is developers and not site builders

## Module dependencies
* [Entity API](https://drupal.org/project/entity), with the following patches:
* [$wrapper->access() might be wrong for single entity reference field](https://www.drupal.org/node/2264079#comment-8911637)
* [Prevent notice in entity_metadata_no_hook_node_access() when node is not saved](https://drupal.org/node/2086225#comment-8768373)


## API via Drupal

Expand Down Expand Up @@ -222,11 +227,6 @@ while other resources can have its cache in MemCached or the database. To
configure this developers just have to specify the following keys in their
_restful_ plugin definition.

## Module dependencies
* [Entity API](https://drupal.org/project/entity), with the following patches:
* [$wrapper->access() might be wrong for single entity reference field](https://drupal.org/node/2264079#comment-8768581)
* [Prevent notice in entity_metadata_no_hook_node_access() when node is not saved](https://drupal.org/node/2086225#comment-8768373)

## Credits

* [Gizra](http://gizra.com)
Expand Down
78 changes: 73 additions & 5 deletions plugins/restful/RestfulEntityBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,10 @@ public function process($path = '', $request = NULL, $method = 'get') {
// This will throw the appropriate exception if needed.
$this->getRateLimitManager()->checkRateLimit($request);
}

// Remove the application property from the request.
static::cleanRequest($original_request);

if (!$path) {
// If $path is empty we don't need to pass it along.
return $this->{$method_name}($request, $account);
Expand Down Expand Up @@ -844,7 +848,7 @@ public function createEntity($request, $account) {

$entity = entity_create($this->entityType, $values);

if (!entity_access('create', $this->entityType, $entity, $account)) {
if (entity_access('create', $this->entityType, $entity, $account) === FALSE) {
// User does not have access to create entity.
$params = array('@resource' => $this->plugin['label']);
throw new RestfulForbiddenException(format_string('You do not have access to create a new @resource resource.', $params));
Expand Down Expand Up @@ -875,9 +879,14 @@ public function createEntity($request, $account) {
protected function setPropertyValues(EntityMetadataWrapper $wrapper, $request, $account, $null_missing_fields = FALSE) {
$save = FALSE;
$original_request = $request;

foreach ($this->getPublicFields() as $public_property => $info) {
// @todo: Pass value to validators, even if it doesn't exist, so we can
// validate required properties.
if (empty($info['property'])) {
// We may have for example an entity with no label property, but with a
// label callback. In that case the $info['property'] won't exist, so
// we skip this field.
continue;
}

$property_name = $info['property'];
if (!isset($request[$public_property])) {
Expand All @@ -892,7 +901,10 @@ protected function setPropertyValues(EntityMetadataWrapper $wrapper, $request, $
if (!$this->checkPropertyAccess($wrapper->{$property_name})) {
throw new RestfulBadRequestException(format_string('Property @name cannot be set.', array('@name' => $public_property)));
}
$wrapper->{$property_name}->set($request[$public_property]);

$field_value = $this->propertyValuesPreprocess($property_name, $request[$public_property]);

$wrapper->{$property_name}->set($field_value);
unset($original_request[$public_property]);
$save = TRUE;
}
Expand All @@ -911,6 +923,63 @@ protected function setPropertyValues(EntityMetadataWrapper $wrapper, $request, $
$wrapper->save();
}

/**
* Massage the value to set according to the format expected by the wrapper.
*
* @param $property_name
* The property name to set.
* @param $value
* The value passed in the request.
*
* @return mix
* The value to set for the wrapped property.
*/
public function propertyValuesPreprocess($property_name, $value) {
// Get the field info.
$field_info = field_info_field($property_name);

if ($field_info['type'] == 'entityreference') {
if ($field_info['cardinality'] != 1 && !is_array($value)) {
// If the field is entity reference type and its cardinality larger than
// 1 set value to an array.
return explode(',', $value);
}
}
elseif (in_array($field_info['type'], array('text', 'text_long', 'text_with_summary'))) {
// Text field. Check if field has an input format.
$instance = field_info_instance($this->getEntityType(), $property_name, $this->getBundle());

if ($field_info['cardinality'] == 1) {
// Single value.
if (!$instance['settings']['text_processing']) {
return $value;
}

return array (
'value' => $value,
'format' => 'filtered_html',
Copy link
Member

Choose a reason for hiding this comment

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

@mateu-aguilo-bosch do you have a better idea on what to do here? We could move this info into the getPublicFields(), but the format may change from user to user.

);
}

// Multiple values.
foreach ($value as $delta => $single_value) {
if (!$instance['settings']['text_processing']) {
$return[$delta] = $single_value;
}
else {
$return[$delta] = array(
'value' => $single_value,
'format' => 'filtered_html',
);
}
}
return $return;
}

// Return the value as is.
return $value;
}

/**
* Helper method to check access on a property.
*
Expand Down Expand Up @@ -939,7 +1008,6 @@ protected function checkPropertyAccess(EntityMetadataWrapper $property, $op = 'e
return;
}


$access = $property->access($op);
return $access === FALSE ? FALSE : TRUE;
}
Expand Down
147 changes: 136 additions & 11 deletions tests/RestfulCreateEntityTestCase.test
Original file line number Diff line number Diff line change
Expand Up @@ -16,43 +16,168 @@ class RestfulCreateEntityTestCase extends DrupalWebTestCase {
}

function setUp() {
parent::setUp('restful_example');
parent::setUp('restful_test', 'entityreference');

// Text - single.
$field = array(
'field_name' => 'text_single',
'type' => 'text_long',
'entity_types' => array('entity_test'),
'cardinality' => 1,
);
field_create_field($field);

$instance = array(
'field_name' => 'text_single',
'bundle' => 'main',
'entity_type' => 'entity_test',
'label' => t('Text single'),
'settings' => array(
// No text processing
'text_processing' => 0,
),
);
field_create_instance($instance);


// Text - multiple.
$field = array(
'field_name' => 'text_multiple',
'type' => 'text_long',
'entity_types' => array('entity_test'),
'cardinality' => FIELD_CARDINALITY_UNLIMITED,
);
field_create_field($field);

$instance = array(
'field_name' => 'text_multiple',
'bundle' => 'main',
'entity_type' => 'entity_test',
'label' => t('Text multiple'),
'settings' => array(
'text_processing' => 1,
),
);
field_create_instance($instance);

// Entity reference - single.
$field = array(
'entity_types' => array('entity_test'),
'settings' => array(
'handler' => 'base',
'target_type' => 'entity_test',
'handler_settings' => array(
),
),
'field_name' => 'entity_reference_single',
'type' => 'entityreference',
'cardinality' => 1,
);
field_create_field($field);

$instance = array(
'entity_type' => 'entity_test',
'field_name' => 'entity_reference_single',
'bundle' => 'main',
'label' => t('Entity reference single'),
);

field_create_instance($instance);

// Entity reference - multiple.
$field = array(
'entity_types' => array('entity_test'),
'settings' => array(
'handler' => 'base',
'target_type' => 'entity_test',
'handler_settings' => array(
),
),
'field_name' => 'entity_reference_multiple',
'type' => 'entityreference',
'cardinality' => FIELD_CARDINALITY_UNLIMITED,
);
field_create_field($field);

$instance = array(
'entity_type' => 'entity_test',
'field_name' => 'entity_reference_multiple',
'bundle' => 'main',
'label' => t('Entity reference multiple'),
);

field_create_instance($instance);
}

/**
* Test creating an entity (POST method).
*/
function testCreateEntity() {
$handler = restful_get_restful_handler('articles');
$request = array('label' => $this->randomName());
$handler = restful_get_restful_handler('main', 1, 1);

$text1 = $this->randomName();
$text2 = $this->randomName();
$request = array(
'text_single' => $text1,
'text_multiple' => array($text1, $text2),
);
$result = $handler->post('', $request);

$node = node_load($result['id']);
$expected_result = array(
'id' => $node->nid,
'label' => $node->title,
'self' => url('node/' . $node->nid, array('absolute' => TRUE)),
$ids = array();

$entity = entity_load_single('entity_test', $result['id']);
$ids[] = $entity->pid;

$striped_result = array(
'text_single' => trim(strip_tags($result['text_single'])),
'text_multiple' => array(
trim(strip_tags($result['text_multiple'][0])),
trim(strip_tags($result['text_multiple'][1])),
),
);

$this->assertEqual($result, $expected_result, 'Entity was created.');
$this->assertEqual($striped_result, $request, 'Entity was created with correct text value.');

// Create an entity with empty request.
try {
$handler->post();
$this->fail('User can create an entity with empty request.');
}
catch (Exception $e) {
catch (\RestfulBadRequestException $e) {
$this->pass('User cannot create an entity with empty request.');
}
catch (\Exception $e) {
$this->fail('Wrong exception thrown when creating an entity with empty request.');
}

// Create an entity with invalid property name.
$request['invalid'] = 'wrong';
try {
$handler->post('', $request);
$this->fail('User can create an entity with invalid property name.');
}
catch (Exception $e) {
catch (\RestfulBadRequestException $e) {
$this->pass('User cannot create an entity with invalid property name.');
}
catch (\Exception $e) {
$this->fail('Wrong exception thrown when creating an entity with invalid property name.');
}

// Create another test entity to be referenced.
$entity = entity_create('entity_test', array('name' => 'main'));
$entity->save();
$ids[] = $entity->pid;

// Create entity with multiple reference.
$request = array('entity_reference_multiple' => $ids);
$result = $handler->post('', $request);

$this->assertEqual($result['entity_reference_multiple'], $ids, 'Create entity with multiple reference.');


// Create entity with comma separated multiple reference.
$request = array('entity_reference_multiple' => implode(',', $ids));
$result = $handler->post('', $request);
$this->assertEqual($result['entity_reference_multiple'], $ids, 'Created entity with comma separated multiple reference.');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,18 @@ class RestfulTestMainResource__1_1 extends RestfulTestMainResource {
public function getPublicFields() {
$public_fields = parent::getPublicFields();

// Determine if the wrapper should use a "sub property" on a text field
// by checking if the "text_processing" is enabled.
$instance = field_info_instance($this->getEntityType(), 'text_single', $this->getBundle());
$public_fields['text_single'] = array(
'property' => 'text_single',
'sub_property' => 'value',
'sub_property' => $instance['settings']['text_processing'] ? 'value' : FALSE,
);

$instance = field_info_instance($this->getEntityType(), 'text_multiple', $this->getBundle());
$public_fields['text_multiple'] = array(
'property' => 'text_multiple',
'sub_property' => 'value',
'sub_property' => $instance['settings']['text_processing'] ? 'value' : FALSE,
);

$public_fields['entity_reference_single'] = array(
Expand Down