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

SSP-1752: add new freemarker method to search first value in array #29

Merged
merged 2 commits into from
Feb 5, 2018

Conversation

nailyk
Copy link
Contributor

@nailyk nailyk commented Feb 2, 2018

The new freemarker method allow to do this kind of thing, based on the JSON : (fyi, it only works on a condition with a string filter)

{
   "cars": [
      {
         "details": {
            "brand": "lexus",
            "color": "black"
         }
      },
      {
         "details": {
            "brand": "citroen",
            "color": "red"
         }
      }
   ]
}

I want the color of the car which have the 'citroen' brand : findFirstInArray("cars", "details.color == black", "details.brand") will return "lexus"

@nailyk nailyk requested review from AlixBa and Elyrixia February 2, 2018 13:22
import freemarker.template.TemplateModelException

@tailrec
private def findChildNode(parent: JsonNode, fullPath: String): JsonNode = {
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible that parent==null and this trows a NPE?

Copy link
Contributor Author

@nailyk nailyk Feb 2, 2018

Choose a reason for hiding this comment

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

If the path of the parent does not exist, the check on the condition will be false and a TemplateModelException Value not found .. will be thrown. Like mentioned in Jackson documentation, here is the return type of the findPathmethod : Value of first matching node found; or if not found, a "missing node" (non-null instance that has no value)

}

private def extractPathAndValueFromCondition(filteredChildCondition: String): (String, String) = {
val splitFilteredChildCondition = filteredChildCondition.split("==").map(_.trim)
Copy link
Contributor

Choose a reason for hiding this comment

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

mmm. I guess that it could be more generic by using JsonPath here, with the correct lib.

Copy link
Contributor Author

@nailyk nailyk Feb 2, 2018

Choose a reason for hiding this comment

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

This is exactly the mechanism i wanted to do with a Json path filter : json-path/JsonPath#272 i.e use an index on a filtered array with jsonPath. But as you can see, it is not possible as of now with JsonPath syntax. That is why i use a custom method to handle this problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK as a workaround

Copy link
Contributor

@Elyrixia Elyrixia left a comment

Choose a reason for hiding this comment

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

Some details to fix but the feature is correct, you can merge when you need to


override def exec(arguments: util.List[_]): SimpleScalar = {
if (arguments.size() != 3) {
throw new TemplateModelException("Wrong arguments : 3 expected : array node, filtered child condition, wanted node )")
Copy link
Contributor

Choose a reason for hiding this comment

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

bonus parenthesis at the end ?

if (arguments.size() != 3) {
throw new TemplateModelException("Wrong arguments : 3 expected : array node, filtered child condition, wanted node )")
}
arguments.asScala.toList match {
Copy link
Contributor

Choose a reason for hiding this comment

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

.map(_.getAsString)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As i can only apply getAsString on a SimpleScalar, it would be like this i think : arguments.asScala.toList.collect { case s: SimpleScalar => s.getAsString } match {

if (filteredChild.textValue() == filteredChildValue) {
Some(findChildNode(parentNode, wantedChildPath).textValue())
} else None
}.headOption match {
Copy link
Contributor

Choose a reason for hiding this comment

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

This part would be clearer in two steps

 arrayNode.elements().asScala
  .find(findChildNode(_, filteredChildPath).textValue() == filteredChildValue)
  .map(findChildNode(_, wantedChildPath).textValue())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point :)

val childNodes = fullPath.split('.')
if (childNodes.length == 1)
parent.findPath(fullPath)
else findChildNode(parent.findPath(childNodes.head), childNodes.tail.mkString("."))
Copy link
Contributor

Choose a reason for hiding this comment

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

either keep both as one liners, or use a newline for both

private def extractPathAndValueFromCondition(filteredChildCondition: String): (String, String) = {
val splitFilteredChildCondition = filteredChildCondition.split("==").map(_.trim)
if (splitFilteredChildCondition.length != 2) throw new TemplateModelException("Filtered child condition should be like this : car.color == red")
(splitFilteredChildCondition(0), splitFilteredChildCondition(1))
Copy link
Contributor

Choose a reason for hiding this comment

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

filteredChildCondition.split("==").map(_.trim).toList match {
  case path :: value :: Nil => (path, value)
  case _ => throw new TemplateModelException("Filtered child condition should be like this : car.color == red")
}

}

private def extractPathAndValueFromCondition(filteredChildCondition: String): (String, String) = {
val splitFilteredChildCondition = filteredChildCondition.split("==").map(_.trim)
Copy link
Contributor

Choose a reason for hiding this comment

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

OK as a workaround

val (array, filteredChildCondition, wantedChildPath) = (a1.getAsString, a2.getAsString, a3.getAsString)
val arrayNode = requestBody.findPath(array)
if (!arrayNode.isArray) throw new TemplateModelException("First arg should be an array node")
val (filteredChildPath, filteredChildValue) = extractPathAndValueFromCondition(filteredChildCondition)
Copy link
Contributor

Choose a reason for hiding this comment

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

This code deserves more space to make it more readable. Use some newline maybe ?

@nailyk nailyk merged commit 629a7d2 into master Feb 5, 2018
@teads-codebase-bot teads-codebase-bot deleted the SSP-1752 branch February 5, 2018 11:32
@nailyk nailyk restored the SSP-1752 branch February 5, 2018 17:45
@teads-codebase-bot teads-codebase-bot deleted the SSP-1752 branch February 5, 2018 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants