Skip to content

Commit bca9a26

Browse files
dbudbu
authored
Merge pull request #356 from FriendsOfSymfony/user-context-listener-options-resolver
Refactor UserContextListener to take options array
2 parents 5849929 + 184169c commit bca9a26

File tree

5 files changed

+155
-62
lines changed

5 files changed

+155
-62
lines changed

‎CHANGELOG.md‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ Changelog
1111
* [User Context] Added an option always_vary_on_context_hash to make it
1212
possible to disable automatically setting the vary headers for the user
1313
hash.
14-
15-
* [Event Listeners] Renamed the event listener classes to XxxLlistener.
1614

1715
* Updated the version of FOSHttpCache to 2.*. See [FOSHttpCache changelog]
1816
(https://github.com/FriendsOfSymfony/FOSHttpCache/blob/master/CHANGELOG.md).
@@ -32,6 +30,12 @@ Changelog
3230

3331
* Deprecated methods have been removed.
3432

33+
### Event listeners
34+
35+
* **BC break:** the `UserContextListener` constructor signature was changed to
36+
take an array of options.
37+
* **BC break:** renamed the event listener classes to XyzListener.
38+
3539
### Rule matcher
3640

3741
* **BC break:** The `match_response` and `additional_cacheable_status`

‎src/DependencyInjection/FOSHttpCacheExtension.php‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,14 @@ private function loadUserContext(ContainerBuilder $container, XmlFileLoader $loa
193193
->replaceArgument(0, $config['match']['accept'])
194194
->replaceArgument(1, $config['match']['method']);
195195

196+
$container->setParameter($this->getAlias().'.event_listener.user_context.options', [
197+
'user_identifier_headers' => $config['user_identifier_headers'],
198+
'user_hash_header' => $config['user_hash_header'],
199+
'ttl' => $config['hash_cache_ttl'],
200+
'add_vary_on_hash' => $config['always_vary_on_context_hash'],
201+
]);
196202
$container->getDefinition($this->getAlias().'.event_listener.user_context')
197-
->replaceArgument(0, new Reference($config['match']['matcher_service']))
198-
->replaceArgument(2, $config['user_identifier_headers'])
199-
->replaceArgument(3, $config['user_hash_header'])
200-
->replaceArgument(4, $config['hash_cache_ttl'])
201-
->replaceArgument(5, $config['always_vary_on_context_hash']);
203+
->replaceArgument(0, new Reference($config['match']['matcher_service']));
202204

203205
$container->getDefinition($this->getAlias().'.user_context.anonymous_request_matcher')
204206
->replaceArgument(0, $config['user_identifier_headers']);

‎src/EventListener/UserContextListener.php‎

Lines changed: 46 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
2121
use Symfony\Component\HttpKernel\HttpKernelInterface;
2222
use Symfony\Component\HttpKernel\KernelEvents;
23+
use Symfony\Component\OptionsResolver\OptionsResolver;
2324

2425
/**
2526
* Check requests and responses with the matcher.
@@ -43,30 +44,15 @@ class UserContextListener implements EventSubscriberInterface
4344
private $hashGenerator;
4445

4546
/**
46-
* @var string[]
47+
* @var array
4748
*/
48-
private $userIdentifierHeaders;
49+
private $options;
4950

5051
/**
5152
* @var string
5253
*/
5354
private $hash;
5455

55-
/**
56-
* @var string
57-
*/
58-
private $hashHeader;
59-
60-
/**
61-
* @var int
62-
*/
63-
private $ttl;
64-
65-
/**
66-
* @var bool Whether to automatically add the Vary header for the hash / user identifier if there was no hash
67-
*/
68-
private $addVaryOnHash;
69-
7056
/**
7157
* Used to exclude anonymous requests (no authentication nor session) from user hash sanity check.
7258
* It prevents issues when the hash generator that is used returns a customized value for anonymous users,
@@ -76,25 +62,41 @@ class UserContextListener implements EventSubscriberInterface
7662
*/
7763
private $anonymousRequestMatcher;
7864

65+
/**
66+
* Constructor.
67+
*
68+
* @param RequestMatcherInterface $requestMatcher
69+
* @param HashGenerator $hashGenerator
70+
* @param RequestMatcherInterface|null $anonymousRequestMatcher
71+
* @param array $options
72+
*/
7973
public function __construct(
8074
RequestMatcherInterface $requestMatcher,
8175
HashGenerator $hashGenerator,
82-
array $userIdentifierHeaders = ['Cookie', 'Authorization'],
83-
$hashHeader = 'X-User-Context-Hash',
84-
$ttl = 0,
85-
$addVaryOnHash = true,
86-
RequestMatcherInterface $anonymousRequestMatcher = null
76+
RequestMatcherInterface $anonymousRequestMatcher = null,
77+
array $options = []
8778
) {
88-
if (!count($userIdentifierHeaders)) {
89-
throw new \InvalidArgumentException('The user context must vary on some request headers');
90-
}
9179
$this->requestMatcher = $requestMatcher;
9280
$this->hashGenerator = $hashGenerator;
93-
$this->userIdentifierHeaders = $userIdentifierHeaders;
94-
$this->hashHeader = $hashHeader;
95-
$this->ttl = $ttl;
96-
$this->addVaryOnHash = $addVaryOnHash;
9781
$this->anonymousRequestMatcher = $anonymousRequestMatcher;
82+
83+
$resolver = new OptionsResolver();
84+
$resolver->setDefaults([
85+
'user_identifier_headers' => ['Cookie', 'Authorization'],
86+
'user_hash_header' => 'X-User-Context-Hash',
87+
'ttl' => 0,
88+
'add_vary_on_hash' => true,
89+
]);
90+
$resolver->setRequired(['user_identifier_headers', 'user_hash_header']);
91+
$resolver->setAllowedTypes('user_identifier_headers', 'array');
92+
$resolver->setAllowedTypes('user_hash_header', 'string');
93+
$resolver->setAllowedTypes('ttl', 'int');
94+
$resolver->setAllowedTypes('add_vary_on_hash', 'bool');
95+
$resolver->setAllowedValues('user_hash_header', function ($value) {
96+
return strlen($value) > 0;
97+
});
98+
99+
$this->options = $resolver->resolve($options);
98100
}
99101

100102
/**
@@ -112,7 +114,9 @@ public function onKernelRequest(GetResponseEvent $event)
112114
}
113115

114116
if (!$this->requestMatcher->matches($event->getRequest())) {
115-
if ($event->getRequest()->headers->has($this->hashHeader) && !$this->isAnonymous($event->getRequest())) {
117+
if ($event->getRequest()->headers->has($this->options['user_hash_header'])
118+
&& !$this->isAnonymous($event->getRequest())
119+
) {
116120
$this->hash = $this->hashGenerator->generateHash();
117121
}
118122

@@ -123,13 +127,13 @@ public function onKernelRequest(GetResponseEvent $event)
123127

124128
// status needs to be 200 as otherwise varnish will not cache the response.
125129
$response = new Response('', 200, [
126-
$this->hashHeader => $hash,
130+
$this->options['user_hash_header'] => $hash,
127131
'Content-Type' => 'application/vnd.fos.user-context-hash',
128132
]);
129133

130-
if ($this->ttl > 0) {
131-
$response->setClientTtl($this->ttl);
132-
$response->setVary($this->userIdentifierHeaders);
134+
if ($this->options['ttl'] > 0) {
135+
$response->setClientTtl($this->options['ttl']);
136+
$response->setVary($this->options['user_identifier_headers']);
133137
$response->setPublic();
134138
} else {
135139
$response->setClientTtl(0);
@@ -170,26 +174,28 @@ public function onKernelResponse(FilterResponseEvent $event)
170174

171175
$vary = $response->getVary();
172176

173-
if ($request->headers->has($this->hashHeader)) {
177+
if ($request->headers->has($this->options['user_hash_header'])) {
174178
// hash has changed, session has most certainly changed, prevent setting incorrect cache
175-
if (!is_null($this->hash) && $this->hash !== $request->headers->get($this->hashHeader)) {
179+
if (!is_null($this->hash) && $this->hash !== $request->headers->get($this->options['user_hash_header'])) {
176180
$response->setClientTtl(0);
177181
$response->headers->addCacheControlDirective('no-cache');
178182

179183
return;
180184
}
181185

182-
if ($this->addVaryOnHash && !in_array($this->hashHeader, $vary)) {
183-
$vary[] = $this->hashHeader;
186+
if ($this->options['add_vary_on_hash']
187+
&& !in_array($this->options['user_hash_header'], $vary)
188+
) {
189+
$vary[] = $this->options['user_hash_header'];
184190
}
185-
} elseif ($this->addVaryOnHash) {
191+
} elseif ($this->options['add_vary_on_hash']) {
186192
/*
187193
* Additional precaution: If for some reason we get requests without a user hash, vary
188194
* on user identifier headers to avoid the caching proxy mixing up caches between
189195
* users. For the hash lookup request, those Vary headers are already added in
190196
* onKernelRequest above.
191197
*/
192-
foreach ($this->userIdentifierHeaders as $header) {
198+
foreach ($this->options['user_identifier_headers'] as $header) {
193199
if (!in_array($header, $vary)) {
194200
$vary[] = $header;
195201
}

‎src/Resources/config/user_context.xml‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,8 @@
1616
<service id="fos_http_cache.event_listener.user_context" class="FOS\HttpCacheBundle\EventListener\UserContextListener">
1717
<argument type="service" id="fos_http_cache.user_context.request_matcher" />
1818
<argument type="service" id="fos_http_cache.user_context.hash_generator" />
19-
<argument />
20-
<argument />
21-
<argument />
22-
<argument />
2319
<argument type="service" id="fos_http_cache.user_context.anonymous_request_matcher" />
20+
<argument>%fos_http_cache.event_listener.user_context.options%</argument>
2421
<tag name="kernel.event_subscriber" />
2522
</service>
2623

‎tests/Unit/EventListener/UserContextListenerTest.php‎

Lines changed: 95 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ public function testMisconfiguration()
3030
new UserContextListener(
3131
\Mockery::mock(RequestMatcherInterface::class),
3232
\Mockery::mock(HashGenerator::class),
33-
[]
33+
null,
34+
[
35+
'user_hash_header' => '',
36+
]
3437
);
3538
}
3639

@@ -43,7 +46,15 @@ public function testOnKernelRequest()
4346
$hashGenerator = \Mockery::mock(HashGenerator::class);
4447
$hashGenerator->shouldReceive('generateHash')->andReturn('hash');
4548

46-
$userContextListener = new UserContextListener($requestMatcher, $hashGenerator, ['X-SessionId'], 'X-Hash');
49+
$userContextListener = new UserContextListener(
50+
$requestMatcher,
51+
$hashGenerator,
52+
null,
53+
[
54+
'user_identifier_headers' => ['X-SessionId'],
55+
'user_hash_header' => 'X-Hash',
56+
]
57+
);
4758
$event = $this->getKernelRequestEvent($request);
4859

4960
$userContextListener->onKernelRequest($event);
@@ -66,7 +77,15 @@ public function testOnKernelRequestNonMaster()
6677
$hashGenerator = \Mockery::mock(HashGenerator::class);
6778
$hashGenerator->shouldReceive('generateHash')->never();
6879

69-
$userContextListener = new UserContextListener($requestMatcher, $hashGenerator, ['X-SessionId'], 'X-Hash');
80+
$userContextListener = new UserContextListener(
81+
$requestMatcher,
82+
$hashGenerator,
83+
null,
84+
[
85+
'user_identifier_headers' => ['X-SessionId'],
86+
'user_hash_header' => 'X-Hash',
87+
]
88+
);
7089
$event = $this->getKernelRequestEvent($request, HttpKernelInterface::SUB_REQUEST);
7190

7291
$userContextListener->onKernelRequest($event);
@@ -83,7 +102,16 @@ public function testOnKernelRequestCached()
83102
$hashGenerator = \Mockery::mock(HashGenerator::class);
84103
$hashGenerator->shouldReceive('generateHash')->andReturn('hash');
85104

86-
$userContextListener = new UserContextListener($requestMatcher, $hashGenerator, ['X-SessionId'], 'X-Hash', 30);
105+
$userContextListener = new UserContextListener(
106+
$requestMatcher,
107+
$hashGenerator,
108+
null,
109+
[
110+
'user_identifier_headers' => ['X-SessionId'],
111+
'user_hash_header' => 'X-Hash',
112+
'ttl' => 30,
113+
]
114+
);
87115
$event = $this->getKernelRequestEvent($request);
88116

89117
$userContextListener->onKernelRequest($event);
@@ -106,7 +134,15 @@ public function testOnKernelRequestNotMatched()
106134
$hashGenerator = \Mockery::mock(HashGenerator::class);
107135
$hashGenerator->shouldReceive('generateHash')->andReturn('hash');
108136

109-
$userContextListener = new UserContextListener($requestMatcher, $hashGenerator, ['X-SessionId'], 'X-Hash');
137+
$userContextListener = new UserContextListener(
138+
$requestMatcher,
139+
$hashGenerator,
140+
null,
141+
[
142+
'user_identifier_headers' => ['X-SessionId'],
143+
'user_hash_header' => 'X-Hash',
144+
]
145+
);
110146
$event = $this->getKernelRequestEvent($request);
111147

112148
$userContextListener->onKernelRequest($event);
@@ -125,7 +161,15 @@ public function testOnKernelResponse()
125161
$requestMatcher = $this->getRequestMatcher($request, false);
126162
$hashGenerator = \Mockery::mock(HashGenerator::class);
127163

128-
$userContextListener = new UserContextListener($requestMatcher, $hashGenerator, ['X-SessionId'], 'X-Hash');
164+
$userContextListener = new UserContextListener(
165+
$requestMatcher,
166+
$hashGenerator,
167+
null,
168+
[
169+
'user_identifier_headers' => ['X-SessionId'],
170+
'user_hash_header' => 'X-Hash',
171+
]
172+
);
129173
$event = $this->getKernelResponseEvent($request);
130174

131175
$userContextListener->onKernelResponse($event);
@@ -143,7 +187,15 @@ public function testOnKernelResponseNotMaster()
143187
$requestMatcher = $this->getRequestMatcher($request, false);
144188
$hashGenerator = \Mockery::mock(HashGenerator::class);
145189

146-
$userContextListener = new UserContextListener($requestMatcher, $hashGenerator, ['X-SessionId'], 'X-Hash');
190+
$userContextListener = new UserContextListener(
191+
$requestMatcher,
192+
$hashGenerator,
193+
null,
194+
[
195+
'user_identifier_headers' => ['X-SessionId'],
196+
'user_hash_header' => 'X-Hash',
197+
]
198+
);
147199
$event = $this->getKernelResponseEvent($request, null, HttpKernelInterface::SUB_REQUEST);
148200

149201
$userContextListener->onKernelResponse($event);
@@ -162,7 +214,15 @@ public function testOnKernelResponseNotCached()
162214
$requestMatcher = $this->getRequestMatcher($request, false);
163215
$hashGenerator = \Mockery::mock(HashGenerator::class);
164216

165-
$userContextListener = new UserContextListener($requestMatcher, $hashGenerator, ['X-SessionId'], 'X-Hash');
217+
$userContextListener = new UserContextListener(
218+
$requestMatcher,
219+
$hashGenerator,
220+
null,
221+
[
222+
'user_identifier_headers' => ['X-SessionId'],
223+
'user_hash_header' => 'X-Hash',
224+
]
225+
);
166226
$event = $this->getKernelResponseEvent($request);
167227

168228
$userContextListener->onKernelResponse($event);
@@ -184,7 +244,15 @@ public function testFullRequestHashOk()
184244
$hashGenerator->shouldReceive('generateHash')->andReturn('hash');
185245

186246
// onKernelRequest
187-
$userContextListener = new UserContextListener($requestMatcher, $hashGenerator, ['X-SessionId'], 'X-Hash');
247+
$userContextListener = new UserContextListener(
248+
$requestMatcher,
249+
$hashGenerator,
250+
null,
251+
[
252+
'user_identifier_headers' => ['X-SessionId'],
253+
'user_hash_header' => 'X-Hash',
254+
]
255+
);
188256
$event = $this->getKernelRequestEvent($request);
189257

190258
$userContextListener->onKernelRequest($event);
@@ -217,7 +285,15 @@ public function testFullAnonymousRequestHashNotGenerated()
217285
$anonymousRequestMatcher->shouldReceive('matches')->andReturn(true);
218286

219287
// onKernelRequest
220-
$userContextListener = new UserContextListener($requestMatcher, $hashGenerator, ['X-SessionId'], 'X-Hash', 0, true, $anonymousRequestMatcher);
288+
$userContextListener = new UserContextListener(
289+
$requestMatcher,
290+
$hashGenerator,
291+
$anonymousRequestMatcher,
292+
[
293+
'user_identifier_headers' => ['X-SessionId'],
294+
'user_hash_header' => 'X-Hash',
295+
]
296+
);
221297
$event = $this->getKernelRequestEvent($request);
222298

223299
$userContextListener->onKernelRequest($event);
@@ -247,7 +323,15 @@ public function testFullRequestHashChanged()
247323
$hashGenerator->shouldReceive('generateHash')->andReturn('hash-changed');
248324

249325
// onKernelRequest
250-
$userContextListener = new UserContextListener($requestMatcher, $hashGenerator, ['X-SessionId'], 'X-Hash');
326+
$userContextListener = new UserContextListener(
327+
$requestMatcher,
328+
$hashGenerator,
329+
null,
330+
[
331+
'user_identifier_headers' => ['X-SessionId'],
332+
'user_hash_header' => 'X-Hash',
333+
]
334+
);
251335
$event = $this->getKernelRequestEvent($request);
252336

253337
$userContextListener->onKernelRequest($event);

0 commit comments

Comments
 (0)