Skip to content

Commit 2ad0004

Browse files
jasmith-hsjasmith-hs
committed
Change the EagerTagFactory to use the tag instances that exist already on the context
1 parent 9bbd611 commit 2ad0004

8 files changed

Lines changed: 110 additions & 91 deletions

File tree

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerCycleTag.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ public EagerCycleTag() {
1717
super(new CycleTag());
1818
}
1919

20+
public EagerCycleTag(CycleTag cycleTag) {
21+
super(cycleTag);
22+
}
23+
2024
@SuppressWarnings("unchecked")
2125
@Override
2226
public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter) {

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerFromTag.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ public EagerFromTag() {
2121
super(new FromTag());
2222
}
2323

24+
public EagerFromTag(FromTag fromTag) {
25+
super(fromTag);
26+
}
27+
2428
@Override
2529
public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter) {
2630
List<String> helper = FromTag.getHelpers(tagToken);

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTag.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ public EagerImportTag() {
2828
super(new ImportTag());
2929
}
3030

31+
public EagerImportTag(ImportTag importTag) {
32+
super(importTag);
33+
}
34+
3135
@Override
3236
public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter) {
3337
List<String> helper = ImportTag.getHelpers(tagToken);

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerTagFactory.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,22 +47,22 @@ public class EagerTagFactory {
4747

4848
@SuppressWarnings("unchecked")
4949
public static <T extends Tag> Optional<EagerTagDecorator<T>> getEagerTagDecorator(
50-
Class<T> clazz
50+
T tag
5151
) {
52+
Class<? extends Tag> clazz = tag.getClass();
5253
try {
5354
if (TAG_CLASSES_TO_SKIP.contains(clazz)) {
5455
return Optional.empty();
5556
}
5657
if (EAGER_TAG_OVERRIDES.containsKey(clazz)) {
5758
EagerTagDecorator<?> decorator = EAGER_TAG_OVERRIDES
5859
.get(clazz)
59-
.getDeclaredConstructor()
60-
.newInstance();
60+
.getDeclaredConstructor(clazz)
61+
.newInstance(tag);
6162
if (decorator.getTag().getClass() == clazz) {
6263
return Optional.of((EagerTagDecorator<T>) decorator);
6364
}
6465
}
65-
T tag = clazz.getDeclaredConstructor().newInstance();
6666
return Optional.of(new EagerGenericTag<>(tag));
6767
} catch (NoSuchMethodException e) {
6868
return Optional.empty();

‎src/main/java/com/hubspot/jinjava/mode/EagerExecutionMode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public void prepareContext(Context context) {
2626
.getAllTags()
2727
.stream()
2828
.filter(tag -> !(tag instanceof EagerTagDecorator))
29-
.map(tag -> EagerTagFactory.getEagerTagDecorator(tag.getClass()))
29+
.map(EagerTagFactory::getEagerTagDecorator)
3030
.filter(Optional::isPresent)
3131
.forEach(maybeEagerTag -> context.registerTag(maybeEagerTag.get()));
3232
context.setExpressionStrategy(new EagerExpressionStrategy());

‎src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerFromTagTest.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ public String getString(
3333
String fullName,
3434
Charset encoding,
3535
JinjavaInterpreter interpreter
36-
)
37-
throws IOException {
36+
) throws IOException {
3837
return Resources.toString(
3938
Resources.getResource(String.format("tags/macrotag/%s", fullName)),
4039
StandardCharsets.UTF_8
@@ -58,7 +57,7 @@ public Optional<LocationResolver> getLocationResolver() {
5857
.build()
5958
);
6059
Tag tag = EagerTagFactory
61-
.getEagerTagDecorator(FromTag.class)
60+
.getEagerTagDecorator(new FromTag())
6261
.orElseThrow(RuntimeException::new);
6362
context.registerTag(tag);
6463
context.put("deferred", DeferredValue.instance());

‎src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTagTest.java‎

Lines changed: 71 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import com.hubspot.jinjava.interpret.DeferredValue;
99
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
1010
import com.hubspot.jinjava.lib.tag.FromTag;
11+
import com.hubspot.jinjava.lib.tag.ImportTag;
1112
import com.hubspot.jinjava.lib.tag.ImportTagTest;
1213
import com.hubspot.jinjava.lib.tag.Tag;
1314
import com.hubspot.jinjava.loader.LocationResolver;
@@ -42,7 +43,7 @@ public void eagerSetup() {
4243
.build()
4344
);
4445
Tag tag = EagerTagFactory
45-
.getEagerTagDecorator(FromTag.class)
46+
.getEagerTagDecorator(new ImportTag())
4647
.orElseThrow(RuntimeException::new);
4748
context.registerTag(tag);
4849
context.put("deferred", DeferredValue.instance());
@@ -113,21 +114,21 @@ public void itHandlesMultiLayerAliased() {
113114
);
114115
assertThat(interpreter.getContext().get(CONTEXT_VAR)).isInstanceOf(Map.class);
115116
assertThat(
116-
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get(child2Alias)
117-
)
117+
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get(child2Alias)
118+
)
118119
.isInstanceOf(Map.class);
119120
assertThat(
120-
(
121-
(Map<String, Object>) (
122-
(Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)
123-
).get(child2Alias)
124-
).get("foo")
125-
)
121+
(
122+
(Map<String, Object>) (
123+
(Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)
124+
).get(child2Alias)
125+
).get("foo")
126+
)
126127
.isEqualTo("foo val");
127128

128129
assertThat(
129-
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get("bar")
130-
)
130+
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get("bar")
131+
)
131132
.isEqualTo("bar val");
132133
}
133134

@@ -156,25 +157,25 @@ public void itHandlesMultiLayerAliasedAndDeferred() {
156157
);
157158
assertThat(interpreter.getContext().get(CONTEXT_VAR)).isInstanceOf(PyMap.class);
158159
assertThat(
159-
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get(child2Alias)
160-
)
160+
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get(child2Alias)
161+
)
161162
.isInstanceOf(DeferredValue.class);
162163
assertThat(
164+
(
163165
(
164-
(
165-
(Map<String, Object>) (
166-
(DeferredValue) (
167-
(Map<String, Object>) (interpreter.getContext().get(CONTEXT_VAR))
168-
).get(child2Alias)
169-
).getOriginalValue()
170-
).get("foo")
171-
)
166+
(Map<String, Object>) (
167+
(DeferredValue) (
168+
(Map<String, Object>) (interpreter.getContext().get(CONTEXT_VAR))
169+
).get(child2Alias)
170+
).getOriginalValue()
171+
).get("foo")
172172
)
173+
)
173174
.isEqualTo("foo val");
174175

175176
assertThat(
176-
(((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get("bar"))
177-
)
177+
(((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get("bar"))
178+
)
178179
.isEqualTo("bar val");
179180
}
180181

@@ -203,25 +204,25 @@ public void itHandlesMultiLayerAliasedAndNullDeferred() {
203204
);
204205
assertThat(interpreter.getContext().get(CONTEXT_VAR)).isInstanceOf(PyMap.class);
205206
assertThat(
206-
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get(child2Alias)
207-
)
207+
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get(child2Alias)
208+
)
208209
.isInstanceOf(DeferredValue.class);
209210
assertThat(
211+
(
210212
(
211-
(
212-
(Map<String, Object>) (
213-
(DeferredValue) (
214-
(Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)
215-
).get(child2Alias)
216-
).getOriginalValue()
217-
).get("foo")
218-
)
213+
(Map<String, Object>) (
214+
(DeferredValue) (
215+
(Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)
216+
).get(child2Alias)
217+
).getOriginalValue()
218+
).get("foo")
219219
)
220+
)
220221
.isEqualTo("foo val");
221222

222223
assertThat(
223-
(((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get("bar"))
224-
)
224+
(((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get("bar"))
225+
)
225226
.isEqualTo("bar val");
226227
}
227228

@@ -247,14 +248,14 @@ public void itHandlesMultiLayerDeferred() {
247248
);
248249
assertThat(interpreter.getContext().get("foo")).isInstanceOf(DeferredValue.class);
249250
assertThat(
250-
(((DeferredValue) (interpreter.getContext().get("foo"))).getOriginalValue())
251-
)
251+
(((DeferredValue) (interpreter.getContext().get("foo"))).getOriginalValue())
252+
)
252253
.isEqualTo("foo val");
253254

254255
assertThat(interpreter.getContext().get("bar")).isInstanceOf(DeferredValue.class);
255256
assertThat(
256-
(((DeferredValue) (interpreter.getContext().get("bar"))).getOriginalValue())
257-
)
257+
(((DeferredValue) (interpreter.getContext().get("bar"))).getOriginalValue())
258+
)
258259
.isEqualTo("bar val");
259260
}
260261

@@ -291,25 +292,25 @@ public void itHandlesMultiLayerSomeAliased() {
291292
);
292293
assertThat(interpreter.getContext().get(CONTEXT_VAR)).isInstanceOf(Map.class);
293294
assertThat(
294-
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get(child3Alias)
295-
)
295+
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get(child3Alias)
296+
)
296297
.isInstanceOf(Map.class);
297298
assertThat(
298-
(
299-
(Map<String, Object>) (
300-
(Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)
301-
).get(child3Alias)
302-
).get("foobar")
303-
)
299+
(
300+
(Map<String, Object>) (
301+
(Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)
302+
).get(child3Alias)
303+
).get("foobar")
304+
)
304305
.isEqualTo("foobar val");
305306

306307
assertThat(
307-
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get("bar")
308-
)
308+
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get("bar")
309+
)
309310
.isEqualTo("bar val");
310311
assertThat(
311-
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get("foo")
312-
)
312+
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get("foo")
313+
)
313314
.isEqualTo("foo val");
314315
}
315316

@@ -347,35 +348,33 @@ public void itHandlesMultiLayerAliasedAndParallel() {
347348
);
348349
assertThat(interpreter.getContext().get(CONTEXT_VAR)).isInstanceOf(Map.class);
349350
assertThat(
350-
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get(child2Alias)
351-
)
351+
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get(child2Alias)
352+
)
352353
.isInstanceOf(Map.class);
353354
assertThat(
354-
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get(
355-
child2BAlias
356-
)
357-
)
355+
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get(child2BAlias)
356+
)
358357
.isInstanceOf(Map.class);
359358
assertThat(
360-
(
361-
(Map<String, Object>) (
362-
(Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)
363-
).get(child2Alias)
364-
).get("foo")
365-
)
359+
(
360+
(Map<String, Object>) (
361+
(Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)
362+
).get(child2Alias)
363+
).get("foo")
364+
)
366365
.isEqualTo("foo val");
367366
assertThat(
368-
(
369-
(Map<String, Object>) (
370-
(Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)
371-
).get(child2BAlias)
372-
).get("foo_b")
373-
)
367+
(
368+
(Map<String, Object>) (
369+
(Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)
370+
).get(child2BAlias)
371+
).get("foo_b")
372+
)
374373
.isEqualTo("foo_b val");
375374

376375
assertThat(
377-
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get("bar")
378-
)
376+
((Map<String, Object>) interpreter.getContext().get(CONTEXT_VAR)).get("bar")
377+
)
379378
.isEqualTo("bar val");
380379
}
381380

@@ -459,8 +458,7 @@ public String getString(
459458
String fullName,
460459
Charset encoding,
461460
JinjavaInterpreter interpreter
462-
)
463-
throws IOException {
461+
) throws IOException {
464462
return Resources.toString(
465463
Resources.getResource(String.format("tags/eager/importtag/%s", fullName)),
466464
StandardCharsets.UTF_8

‎src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerTagFactoryTest.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import com.hubspot.jinjava.lib.tag.IncludeTag;
66
import com.hubspot.jinjava.lib.tag.RawTag;
77
import com.hubspot.jinjava.lib.tag.Tag;
8+
import java.util.Objects;
89
import java.util.Optional;
910
import java.util.Set;
1011
import java.util.stream.Collectors;
@@ -14,32 +15,41 @@ public class EagerTagFactoryTest {
1415

1516
@Test
1617
public void itGetsEagerTagDecoratorForOverrides() {
17-
Set<EagerTagDecorator<?>> eagerTagDecoratorSet = EagerTagFactory
18-
.EAGER_TAG_OVERRIDES.keySet()
18+
Set<EagerTagDecorator<?>> eagerTagDecoratorSet = EagerTagFactory.EAGER_TAG_OVERRIDES
19+
.keySet()
1920
.stream()
21+
.map(
22+
clazz -> {
23+
try {
24+
return clazz.getDeclaredConstructor().newInstance();
25+
} catch (Exception ignored) {
26+
return null;
27+
}
28+
}
29+
)
30+
.filter(Objects::nonNull)
2031
.map(EagerTagFactory::getEagerTagDecorator)
2132
.filter(Optional::isPresent)
2233
.map(Optional::get)
2334
.collect(Collectors.toSet());
2435
assertThat(eagerTagDecoratorSet.size())
2536
.isEqualTo(EagerTagFactory.EAGER_TAG_OVERRIDES.keySet().size());
2637
assertThat(
27-
eagerTagDecoratorSet
28-
.stream()
29-
.map(e -> e.getTag().getClass())
30-
.collect(Collectors.toSet())
31-
)
38+
eagerTagDecoratorSet
39+
.stream()
40+
.map(e -> e.getTag().getClass())
41+
.collect(Collectors.toSet())
42+
)
3243
.isEqualTo(EagerTagFactory.EAGER_TAG_OVERRIDES.keySet());
3344
}
3445

3546
@Test
3647
public void itGetsEagerTagDecoratorForNonOverride() {
37-
Class<? extends Tag> clazz = IncludeTag.class;
3848
Optional<? extends EagerTagDecorator<? extends Tag>> maybeEagerGenericTag = EagerTagFactory.getEagerTagDecorator(
39-
clazz
49+
new IncludeTag()
4050
);
4151
assertThat(maybeEagerGenericTag).isPresent();
4252
assertThat(maybeEagerGenericTag.get()).isInstanceOf(EagerGenericTag.class);
43-
assertThat(maybeEagerGenericTag.get().getTag()).isInstanceOf(clazz);
53+
assertThat(maybeEagerGenericTag.get().getTag()).isInstanceOf(IncludeTag.class);
4454
}
4555
}

0 commit comments

Comments
 (0)