Skip to content
Open
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
33 changes: 33 additions & 0 deletions lib/Oryzone/MediaStorage/MediaStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,39 @@ public function render(MediaInterface $media, $variant = NULL, $options = array(
$variant = $this->defaultVariant;
}

if (!$media->hasVariant($variant)) {
Copy link
Member

Choose a reason for hiding this comment

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

Here, IMHO, we should also check someway if the variant is defined in the variant tree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that check should be added as the user may mis-type a variant.

Copy link

Choose a reason for hiding this comment

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

I think that it should be in getUrl() method rather than in render()

// If this media does not have a variant based on its parent variant, if it has a parent
$filesystem = $this->getFilesystem($context->getFilesystemName());
$namingStrategy = $this->getNamingStrategy($context->getNamingStrategyName());
$context = $this->getContext($media->getContext());
$provider = $this->getProvider($context->getProviderName(), $context->getProviderOptions());
$variantsTree = $context->buildVariantTree();

/** @var $node VariantNode */
$node = $variantsTree->getNode($variant);
$childVariant = $node->getContent();
while ($node->getParent()) { // advance to parent node
$node = $node->getParent();
}

// Get parents variant information to base this new variant off
$parentVariant = $media->getVariantInstance($node->getContent()->getName());
// @todo
if (!$filesystem->has($parentVariant->getFilename())) {
throw new \InvalidArgumentException();
}

$filesystem->get($parentVariant->getFilename());
Copy link
Member

Choose a reason for hiding this comment

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

Missing assignment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Old line of code that needs removed. I will get that updated.

$result = $provider->processFromParent($media, $childVariant, $parentVariant, $filesystem);
$name = $namingStrategy->generateName($media, $childVariant, $filesystem);
$this->saveFileToFilesystem($result, $name, $filesystem, $childVariant);

//updates the variant in the media (to store the new values)
$media->addVariant($childVariant);

$this->persistenceAdapter->update($media);
}

$provider = $this->getProvider($context->getProviderName(), $context->getProviderOptions());
$variantInstance = $media->getVariantInstance($variant);

Expand Down
27 changes: 27 additions & 0 deletions lib/Oryzone/MediaStorage/Provider/ImageProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@

namespace Oryzone\MediaStorage\Provider;

use Gaufrette\Filesystem,
Gaufrette\Stream\Local,
Gaufrette\StreamMode;

use Oryzone\MediaStorage\Model\MediaInterface,
Oryzone\MediaStorage\Variant\VariantInterface,
Oryzone\MediaStorage\Exception\ProviderProcessException,
Expand Down Expand Up @@ -169,6 +173,29 @@ public function validateContent($content)
return false;
}

public function processFromParent($media, VariantInterface $variant, VariantInterface $parentVariant, Filesystem $filesystem)
Copy link
Member

Choose a reason for hiding this comment

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

I think this method would not need particular customizations for each provider type.
Probably we should move it to the main MediaStorage class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking that as well however we have to create a local temp directory and we don't have access to the providers temp path.

{
$tempName = $this->tempDir.'/'.$parentVariant->getFilename();

$src = $filesystem->createStream($parentVariant->getFilename());
$dst = new Local($tempName);

$src->open(new StreamMode('rb+'));
$dst->open(new StreamMode('wb+'));

while (!$src->eof()) {
$data = $src->read(100000);
$written = $dst->write($data);
}
$dst->close();
$src->close();

$this->addTempFile($tempName);

// We now have a local copy of file
return $this->process($media, $variant, new \SplFileInfo($tempName));
}

/**
* {@inheritDoc}
*/
Expand Down
2 changes: 2 additions & 0 deletions lib/Oryzone/MediaStorage/Provider/ProviderInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ public function prepare(MediaInterface $media, ContextInterface $context);
*/
public function process(MediaInterface $media, VariantInterface $variant, \SplFileInfo $source = NULL);

public function processFromParent($media, VariantInterface $variant, VariantInterface $parentVariant, Filesystem $filesystem);
Copy link
Member

Choose a reason for hiding this comment

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

As said above, probably we don't need to bloat the ProviderInterface with a new method if it's not adding "provider specific functionality"


/**
* Renders a variant to HTML code. Useful for twig (or other template engines) integrations
*
Expand Down