[BUG] HandlebarsEngineAdapter.java failed to use custom FieldValueResolver causing IllegalAccessException for maps (#19634)

* Update HandlebarsEngineAdapter.java

The old MY_FIELD_VALUE_RESOLVER.INSTANCE is equivalent to FieldValueResolver.INSTANCE, which is assigned `new FieldValueResolver();` so our custom class was never used.

* Add test verifying partial template works; Extract accessAwareFieldValueResolver; Add test verifying priority fo values extracted from object
This commit is contained in:
WouterBaeyens 2024-10-03 09:56:53 +02:00 committed by GitHub
parent c84af35e7b
commit 6686c4d02f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 108 additions and 28 deletions

View File

@ -33,6 +33,7 @@ import com.github.jknack.handlebars.io.TemplateSource;
import lombok.Setter;
import org.openapitools.codegen.api.AbstractTemplatingEngineAdapter;
import org.openapitools.codegen.api.TemplatingExecutor;
import org.openapitools.codegen.templating.handlebars.AccessAwareFieldValueResolver;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -45,7 +46,7 @@ import java.util.Set;
import java.util.stream.Collectors;
public class HandlebarsEngineAdapter extends AbstractTemplatingEngineAdapter {
final Logger LOGGER = LoggerFactory.getLogger(HandlebarsEngineAdapter.class);
final Logger LOGGER = LoggerFactory.getLogger(HandlebarsEngineAdapter.class);
private final String[] extensions = {"handlebars", "hbs"};
// We use this as a simple lookup for valid file name extensions. This adapter will inspect .mustache (built-in) and infer the relevant handlebars filename
@ -73,36 +74,13 @@ public class HandlebarsEngineAdapter extends AbstractTemplatingEngineAdapter {
}
};
// $ref: https://github.com/jknack/handlebars.java/issues/917
var MY_FIELD_VALUE_RESOLVER = new FieldValueResolver() {
@Override
protected Set<FieldWrapper> members(
Class<?> clazz) {
var members = super.members(clazz);
return members.stream()
.filter(fw -> isValidField(fw))
.collect(Collectors.toSet());
}
boolean isValidField(
FieldWrapper fw) {
if (fw instanceof AccessibleObject) {
if (isUseSetAccessible(fw)) {
return true;
}
return false;
}
return true;
}
};
Context context = Context
.newBuilder(bundle)
.resolver(
MapValueResolver.INSTANCE,
JavaBeanValueResolver.INSTANCE,
MY_FIELD_VALUE_RESOLVER.INSTANCE,
MethodValueResolver.INSTANCE)
MethodValueResolver.INSTANCE,
AccessAwareFieldValueResolver.INSTANCE)
.build();
Handlebars handlebars = new Handlebars(loader);

View File

@ -0,0 +1,28 @@
package org.openapitools.codegen.templating.handlebars;
import com.github.jknack.handlebars.context.FieldValueResolver;
import java.lang.reflect.AccessibleObject;
import java.util.Set;
import java.util.stream.Collectors;
// $ref: https://github.com/jknack/handlebars.java/issues/917
public class AccessAwareFieldValueResolver extends FieldValueResolver {
public static final AccessAwareFieldValueResolver INSTANCE = new AccessAwareFieldValueResolver();
@Override
protected Set<FieldValueResolver.FieldWrapper> members(Class<?> clazz) {
var members = super.members(clazz);
return members.stream()
.filter(this::isValidField)
.collect(Collectors.toSet());
}
boolean isValidField(FieldWrapper fw) {
if (fw instanceof AccessibleObject) {
return isUseSetAccessible(fw);
}
return true;
}
}

View File

@ -1,9 +1,14 @@
package org.openapitools.codegen.templating;
import org.mockito.Mockito;
import org.openapitools.codegen.api.TemplatingExecutor;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import static org.testng.Assert.*;
import java.io.IOException;
import java.util.Map;
import static org.testng.Assert.assertEquals;
public class HandlebarsEngineAdapterTest {
@Test(dataProvider = "handlesFileExpectations")
@ -33,4 +38,73 @@ public class HandlebarsEngineAdapterTest {
{"README.md", false, "Should not attempt to handle non-handlebars extensions (other than mustache)"}
};
}
@Test(description = "verify https://github.com/jknack/handlebars.java/issues/940#issue-1111612043 is fixed")
public void testHandlePartialTemplate() throws IOException {
// Given
HandlebarsEngineAdapter adapter = new HandlebarsEngineAdapter();
TemplatingExecutor executorMock = Mockito.mock(TemplatingExecutor.class);
Mockito.when(executorMock.getFullTemplateContents("outerTemplate.hbs")).thenReturn("Contents: {{>innerTemplate}}");
Mockito.when(executorMock.getFullTemplateContents("innerTemplate.hbs")).thenReturn("'Specific contents'");
// When
String generatedFile = adapter.compileTemplate(executorMock, Map.of(), "outerTemplate.hbs");
// Then
assertEquals(generatedFile, "Contents: 'Specific contents'");
}
@Test(description = "should prioritize public getters over breaking encapsulation")
public void testResolverPriority() throws IOException {
// Given
HandlebarsEngineAdapter adapter = new HandlebarsEngineAdapter();
TemplatingExecutor executorMock = Mockito.mock(TemplatingExecutor.class);
Mockito.when(executorMock.getFullTemplateContents("outerTemplate.hbs")).thenReturn(
"Contents: {{#propertyObj}}\n" +
" public getter: {{valueMethodAndBean}}\n" +
" public method: {{valueAndMethod}}\n" +
" private property: {{valueOnly}}{{/propertyObj}}");
Map<String, Object> bundle = Map.of("propertyObj", new PropertyObject());
// When
String generatedFile = adapter.compileTemplate(executorMock, bundle, "outerTemplate.hbs");
// Then
assertEquals(generatedFile, "Contents: \n" +
" public getter: get_raw_data1_formatted\n" +
" public method: raw_data2_formatted\n" +
" private property: raw_data3");
}
static class PropertyObject {
/**
* getter-exposed
*/
private final String valueMethodAndBean = "raw_data1";
public String valueMethodAndBean() {
return valueMethodAndBean + "_formatted";
}
public String getValueMethodAndBean() {
return "get_" + valueMethodAndBean();
}
/**
* method-exposed
*/
private final String valueAndMethod = "raw_data2";
public String valueAndMethod() {
return valueAndMethod + "_formatted";
}
/**
* private
* note: ideally we long-term move towards respecting encapsulation where possible
*/
@SuppressWarnings({"unused", "java:S1068"}) // this private value is still read by our HandleBars engine
private final String valueOnly = "raw_data3";
}
}

View File

@ -21,7 +21,7 @@ public class StringHelpersTest {
Context context = Context
.newBuilder(data)
.resolver(
FieldValueResolver.INSTANCE)
AccessAwareFieldValueResolver.INSTANCE)
.build();
Template tmpl = handlebars.compileInline(template);