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

[WIP] Added resolve events to cache manager #388

Merged
merged 12 commits into from
Apr 10, 2014

Conversation

serdyuka
Copy link
Contributor

@serdyuka serdyuka commented Apr 8, 2014

WIP


$this->dispatcher->dispatch(ImagineEvents::PRE_RESOLVE, new PreResolveEvent($resolver, $path, $filter));

$url = $resolver->resolve($path, $filter);
Copy link
Collaborator

Choose a reason for hiding this comment

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

resolver, path, amd filter has to be taken from the event

@makasim makasim added this to the v1.0.0 milestone Apr 8, 2014
@makasim makasim self-assigned this Apr 8, 2014
@serdyuka
Copy link
Contributor Author

serdyuka commented Apr 9, 2014

@makasim review please

*/
protected $url;

public function __construct(ResolverInterface $resolver, $path, $filter, $url = null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

doc blocks, not only here

@makasim makasim changed the title Added resolve events to cache manager [WIP] Added resolve events to cache manager Apr 9, 2014
@makasim
Copy link
Collaborator

makasim commented Apr 9, 2014

tests of cource

@serdyuka
Copy link
Contributor Author

serdyuka commented Apr 9, 2014

@makasim, review please

@@ -176,7 +188,21 @@ public function resolve($path, $filter)
throw new NotFoundHttpException(sprintf("Source image was searched with '%s' outside of the defined root path", $path));
}

return $this->getResolver($filter)->resolve($path, $filter);
/** @var CacheResolveEvent $event */
$event = $this->dispatcher->dispatch(ImagineEvents::PRE_RESOLVE, new CacheResolveEvent(
Copy link
Collaborator

Choose a reason for hiding this comment

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

$preEvent

Copy link
Collaborator

Choose a reason for hiding this comment

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

$preEvent = new CacheResolveEvent($path, $filter));
$this->dispatcher->dispatch(ImagineEvents::PRE_RESOLVE, $preEvent):

@serdyuka
Copy link
Contributor Author

@makasim, review please

class CacheResolveEvent extends Event
{
/**
* Resource path
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is useless, please remove such comment

@serdyuka
Copy link
Contributor Author

@makasim, fixed. Review please

$this->event = null;
}

public function testGetPath()
Copy link
Collaborator

Choose a reason for hiding this comment

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

/**
 * @test
 */
public function shouldAllowSetPath();

Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldAllowGetPathPreviouslySet

Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldAllowGetPathSetInConstructor

$cacheManager->addResolver($expectedFilterOne, $resolver);
$cacheManager->addResolver($expectedFilterTwo, $resolver);

$cacheManager->remove(null, array($expectedFilterOne, $expectedFilterTwo));
}

public function testShouldBeDispatchedCachePreResolveEvent()
Copy link
Collaborator

Choose a reason for hiding this comment

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

testShouldDispatchCachePreResolveEvent

@serdyuka
Copy link
Contributor Author

@makasim, fixed, review please

$preEvent = new CacheResolveEvent($path, $filter);
$this->dispatcher->dispatch(ImagineEvents::PRE_RESOLVE, $preEvent);

$url = $this->getResolver($filter)->resolve($preEvent->getPath(), $preEvent->getFilter());
Copy link
Collaborator

Choose a reason for hiding this comment

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

$this->getResolver($filter) -> $this->getResolver($preEvent->getFilter())

@serdyuka
Copy link
Contributor Author

@makasim, fixed, review please

@makasim
Copy link
Collaborator

makasim commented Apr 10, 2014

@serdyuka looks good, I am waiting for green from CI and merge this

makasim added a commit that referenced this pull request Apr 10, 2014
[WIP] Added resolve events to cache manager
@makasim makasim merged commit d5e5f1c into liip:master Apr 10, 2014
@makasim makasim deleted the resolve-events branch April 10, 2014 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Attn: Blocker This item blocks other issue(s) or PR(s) and therefore must be resolved prior. Level: New Feature 🆕 This item involves the introduction of new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants