-
Notifications
You must be signed in to change notification settings - Fork 69
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
First implementation of templates #193
Conversation
* @return the generated string where each parameter is replaced by its value provided in the map | ||
* @throws TemplateException in case of issues during the string generation, e.g. missing parameter | ||
*/ | ||
String process(Map<String, String> parameters) throws TemplateException; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With a Map<String, String>
we can only handle 1 fields level in templates. What happen if a template engine accepts complex objects (${user.address.street}
for example)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
* @return the generated string where each parameter is replaced by its value provided in the map | ||
* @throws TemplateException in case of issues during the string generation, e.g. missing parameter | ||
*/ | ||
String process(Map<String, String> parameters) throws TemplateException; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With a Map<String, String>
as parameter we can only manage 1 level of attributes while most of the template engines support tree values as data-model (cf. https://freemarker.apache.org/docs/dgui_quickstart_datamodel.html).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Freemarker can process an object for instance (https://freemarker.apache.org/docs/api/freemarker/template/Template.html#process-java.lang.Object-java.io.Writer-), that would be nice to study different template engines to see what they offer but I think this is where we should go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FreeMarker accepts any kind of objects, like Map<String, Object> or java beans: https://freemarker.apache.org/docs/pgui_quickstart_createdatamodel.html
Velocity accepts a Context object which is a Map like object, see : http://velocity.apache.org/engine/devel/developer-guide.html#the-context
Thymleaf accepts an IContext object: https://www.thymeleaf.org/doc/tutorials/3.0/usingthymeleaf.html#contexts
I would suggest to accept a Object in the interface and each implementation will have to check the actual type.
public interface Template { | ||
|
||
/** | ||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall description missing
* Interface to represent a template. | ||
* A template takes parameters in input and outputs a string. | ||
*/ | ||
public interface Template { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@API
Guardian annotation are missing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- create a demo in
sumphony-bdk-examples
module
/** | ||
* Internal class used when instantiating a {@link FreeMarkerTemplate} with {@link FreeMarkerEngine#newTemplateFromUrl(String)} | ||
*/ | ||
public class UrlTemplateLoader extends URLTemplateLoader { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this class should be package-protected. I don't think developers will touch this class.
.warn("More than 1 TemplateEngine implementation found in classpath, will use : {}", | ||
templateEngines.stream().findFirst().get()); | ||
} | ||
LoggerFactory.getLogger(TemplateEngine.class).info("TemplateEngine implementation found"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@symphony-thibault I cannot use the Slf4j annotation as it is only valid on classes and enums. This leads to the following error : Failed to load class "org.slf4j.impl.StaticLoggerBinder".
What should I do ? Remove the logs ?
} | ||
|
||
private static String getStringFromBuiltInTemplate() throws TemplateException { | ||
Template template = new FreeMarkerEngine().newBuiltInTemplate("simpleMML"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we need a enum listing all built-in templates we have? IMHO, it might be more convenient if we write something like: new FreeMarkerEngine().newBuiltInTemplate(BuiltInTemplate.SIMPLE_MML);
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
* @return a new {@link Template} instantiated from the provided file | ||
* @throws TemplateException when template cannot be loaded, e.g. file not accessible | ||
*/ | ||
Template newTemplateFromFile(String templatePath) throws TemplateException; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps we could support URI? and then find out it is a file or the classpath based on it?
file:///root/blah.text classpath://com/symphony/bdk/blah.text
* @return a new {@link Template} instantiated from the provided url, should be a valid {@link java.net.URL} string. | ||
* @throws TemplateException when template cannot be loaded, e.g. url not accessible | ||
*/ | ||
Template newTemplateFromUrl(String url) throws TemplateException; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah we have URL here, would not URI cover everything?
No description provided.