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

"When I attach the file" and Drupal temporary files #355

Open
jonathanjfshaw opened this Issue Feb 7, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@jonathanjfshaw
Copy link
Contributor

jonathanjfshaw commented Feb 7, 2017

The following steps work the first occasion they are run, but not on subsequent occasions:

    Given a "discussion" with the title "Test"
    And I am on "/discuss/test"
    When I attach the file "testfile.txt" to "files[field_files_0][]"
    And I press "Upload"
    And I press "Save"
    Then I should see the link "testfile.txt"
    When I visit "_flysystem/dropboxwebarchive/discussions/test/testfile.txt"
    Then I should see "test file contents"

The step When I visit "_flysystem/dropboxwebarchive/discussions/test/testfile.txt" fails on the subsequent occasions because the file from the first occasion is still present as a Drupal temporary file, and therefore the filename of the file from the second occasion is changed to "testfile_0.txt" to avoid overwriting.

What is needed is to keep track of uploaded files and to clean them up after scenarios.

I suggest we add:

  1. A step in the DrupalContext: "When I attach the file :filename to the file field :filefieldname"
  2. A fileupload function in rawDrupalContext, that includes logging the created file entity, as we do now for users, nodes, etc.
  3. An @afterScenario file entity cleanup mechanism in rawDrupalContext

The file cleanup could use the entityDelete function from the entityContext, so we should postpone this on #300, which is in turn postponed on on #337 (Robust entity handling).

In fact, #337 will probably include making a generic entity cleanup mechanism, rather than endlessly proliferating near-duplicate entity-type-specific mechanisms.

@jonathanjfshaw

This comment has been minimized.

Copy link
Contributor

jonathanjfshaw commented Feb 7, 2017

Sorry, I overlooked the obvious: this is a UI step not an API step, so we don't have a definitive reference to the file entity associated with the uploaded step, so we can't clean it up.

It's an instance of the more general problem of cleaning up changes to entities made by steps using the UI.

I can't see a good global solution. But we might be able to help people create their own solutions if the driver had an entityDelete function that could take a very flexible condition argument. With this developers could then create their own hook functions to delete entities meeting the conditions.

In the case of files, there could be an @cleanCreatedFiles hook that deleted any file entity created after the start time of the scenario.

@jonathanjfshaw

This comment has been minimized.

Copy link
Contributor

jonathanjfshaw commented Feb 7, 2017

OK, I'm slow today.

This is already handled for nodes and comments by Drupal itself using hook_ENTITY_TYPE_predelete to delete content created by a user. So when a user created by something like "Given I am logged in as" is cleaned up, their nodes and comments created through the UI is cleaned up.

The file module doesn't use hook_user_predelete to clean up file entities when a user is deleted, hence my steps fail.

Perhaps what we need is for the Drupal extension to have user delete/predelete hooks as well as the existing user create hooks. Then we could specify additional clean up for Behat created users, including files they 'own'.

@pfrenssen

This comment has been minimized.

Copy link
Collaborator

pfrenssen commented Apr 7, 2017

This is already handled for nodes and comments by Drupal itself using hook_ENTITY_TYPE_predelete to delete content created by a user. So when a user created by something like "Given I am logged in as" is cleaned up, their nodes and comments created through the UI is cleaned up.

Only if it is configured to do so, you can also configure Drupal to unpublish the content when deleting the user, or to mark it as being owned by the anonymous user. In these cases the content will remain after the test and needs to be cleaned up manually.

The rule in general is that anything created through the UI should be cleaned up manually at the end of the test.

I can't see a good global solution. But we might be able to help people create their own solutions if the driver had an entityDelete function that could take a very flexible condition argument. With this developers could then create their own hook functions to delete entities meeting the conditions.

Yes something like @Then I delete the :type entity with the label :label would be great for manual cleanups. But this is something that is near impossible to do in Drupal 6 & 7 since we don't have a $entity->getEntityKey('label'). We would have to add support in DrupalDriver for every core entity type + most of the popular contrib ones which is not something that seems feasible.

In project code this is really easy to do. In D8 I use an EntityTrait which has a method to retrieve the entity by label:

<?php

namespace Drupal\my_project\Traits;

/**
 * Helper methods to deal with entities.
 */
trait EntityTrait {

  /**
   * Returns the entity with the given type, bundle and label.
   *
   * If multiple entities have the same label then the first one is returned.
   *
   * @param string $entity_type
   *   The entity type to check.
   * @param string $label
   *   The label to check.
   * @param string $bundle
   *   Optional bundle to check. If omitted, the entity can be of any bundle.
   *
   * @return \Drupal\Core\Entity\EntityInterface
   *   The requested entity.
   *
   * @throws \Exception
   *   Thrown when an entity with the given type, label and bundle does not
   *   exist.
   */
  protected function getEntityByLabel($entity_type, $label, $bundle = NULL) {
    $entity_manager = \Drupal::entityTypeManager();
    $storage = $entity_manager->getStorage($entity_type);
    $entity = $entity_manager->getDefinition($entity_type);

    $query = $storage->getQuery()
      ->condition($entity->getKey('label'), $label)
      ->range(0, 1);

    // Optionally filter by bundle.
    if ($bundle) {
      $query->condition($entity->getKey('bundle'), $bundle);
    }

    $result = $query->execute();

    if ($result) {
      $result = reset($result);
      return $storage->load($result);
    }

    throw new \Exception("The entity with label '$label' was not found.");
  }

}

Then in the subcontexts of my modules I provide a human readable step, but this can also be using the generic step definition from above. Here is the one for nodes:

  /**
   * Checks if a node of a certain type with a given title exists.
   *
   * @param string $type
   *   The node type.
   * @param string $title
   *   The title of the node.
   *
   * @Then I should have a(n) :type (content )page titled :title
   */
  public function assertContentPageByTitle($type, $title) {
    $type = $this->getEntityByLabel('node_type', $type);
    // If the node doesn't exist, the exception will be thrown here.
    $this->getEntityByLabel('node', $title, $type->id());
  }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment