You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by achristianson <gi...@git.apache.org> on 2017/12/12 21:15:02 UTC

[GitHub] nifi-minifi-cpp pull request #219: MINIFICPP-330 Implemented Expression Lang...

GitHub user achristianson opened a pull request:

    https://github.com/apache/nifi-minifi-cpp/pull/219

    MINIFICPP-330 Implemented Expression Language substring operations

    Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
    
    In order to streamline the review of the contribution we ask you
    to ensure the following steps have been taken:
    
    ### For all changes:
    - [ ] Is there a JIRA ticket associated with this PR? Is it referenced
         in the commit message?
    
    - [ ] Does your PR title start with MINIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
    
    - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
    
    - [ ] Is your initial contribution a single, squashed commit?
    
    ### For code changes:
    - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
    - [ ] If applicable, have you updated the LICENSE file?
    - [ ] If applicable, have you updated the NOTICE file?
    
    ### For documentation related changes:
    - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
    
    ### Note:
    Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/achristianson/nifi-minifi-cpp MINIFICPP-330

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi-minifi-cpp/pull/219.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #219
    
----
commit b0711e09b7baa872e486835604bf9a0bca4e7aae
Author: Andy I. Christianson <an...@andyic.org>
Date:   2017-12-12T17:04:46Z

    MINIFICPP-330 Implemented Expression Language substring operations

----


---

[GitHub] nifi-minifi-cpp pull request #219: MINIFICPP-330 Implemented Expression Lang...

Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on a diff in the pull request:

    https://github.com/apache/nifi-minifi-cpp/pull/219#discussion_r156815480
  
    --- Diff: extensions/expression-language/Expression.cpp ---
    @@ -64,12 +64,44 @@ std::string expr_toUpper(const std::vector<std::string> &args) {
       return result;
     }
     
    +std::string expr_substring(const std::vector<std::string> &args) {
    +  if (args.size() < 3) {
    +    return args[0].substr(std::stoul(args[1]));
    +  } else {
    +    return args[0].substr(std::stoul(args[1]), std::stoul(args[2]));
    +  }
    +}
    +
    +std::string expr_substringBefore(const std::vector<std::string> &args) {
    +  return args[0].substr(0, args[0].find(args[1]));
    +}
    +
    +std::string expr_substringBeforeLast(const std::vector<std::string> &args) {
    +  size_t last_pos = 0;
    +  while (args[0].find(args[1], last_pos + 1) != std::string::npos) {
    +    last_pos = args[0].find(args[1], last_pos + 1);
    +  }
    +  return args[0].substr(0, last_pos);
    +}
    +
    +std::string expr_substringAfter(const std::vector<std::string> &args) {
    +  return args[0].substr(args[0].find(args[1]) + args[1].length());
    +}
    +
    +std::string expr_substringAfterLast(const std::vector<std::string> &args) {
    +  size_t last_pos = 0;
    +  while (args[0].find(args[1], last_pos + 1) != std::string::npos) {
    +    last_pos = args[0].find(args[1], last_pos + 1);
    +  }
    +  return args[0].substr(last_pos + args[1].length());
    +}
    +
     template<std::string T(const std::vector<std::string> &)>
     Expression make_dynamic_function_incomplete(const std::string &function_name,
                                                 const std::vector<Expression> &args,
                                                 std::size_t num_args) {
    -  if (args.size() == num_args) {
    --- End diff --
    
    Based on my comment below, it seems like we'll simply throw "Attempted to call incomplete function" Is this a correct interpretation? 


---

[GitHub] nifi-minifi-cpp pull request #219: MINIFICPP-330 Implemented Expression Lang...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/nifi-minifi-cpp/pull/219


---

[GitHub] nifi-minifi-cpp issue #219: MINIFICPP-330 Implemented Expression Language su...

Posted by achristianson <gi...@git.apache.org>.
Github user achristianson commented on the issue:

    https://github.com/apache/nifi-minifi-cpp/pull/219
  
    @phrocker tests added.


---

[GitHub] nifi-minifi-cpp pull request #219: MINIFICPP-330 Implemented Expression Lang...

Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on a diff in the pull request:

    https://github.com/apache/nifi-minifi-cpp/pull/219#discussion_r156815280
  
    --- Diff: libminifi/test/expression-language-tests/ExpressionLanguageTests.cpp ---
    @@ -235,3 +242,51 @@ TEST_CASE("GetFile PutFile dynamic attribute", "[expressionLanguageTestGetFilePu
         REQUIRE("extracted_attr" == output_str.str());
       }
     }
    +
    +TEST_CASE("Substring 2 arg", "[expressionLanguageSubstring2]") {  // NOLINT
    +  auto expr = expression::compile("text_before${attr:substring(6, 8)}text_after");
    +
    +  auto flow_file_a = std::make_shared<MockFlowFile>();
    +  flow_file_a->addAttribute("attr", "__flow_a_attr_value_a__");
    +  REQUIRE("text_before_a_attr_text_after" == expr({flow_file_a}));
    +}
    +
    +TEST_CASE("Substring 1 arg", "[expressionLanguageSubstring1]") {  // NOLINT
    +  auto expr = expression::compile("text_before${attr:substring(6)}text_after");
    +
    +  auto flow_file_a = std::make_shared<MockFlowFile>();
    +  flow_file_a->addAttribute("attr", "__flow_a_attr_value_a__");
    +  REQUIRE("text_before_a_attr_value_a__text_after" == expr({flow_file_a}));
    +}
    +
    +TEST_CASE("Substring Before", "[expressionLanguageSubstringBefore]") {  // NOLINT
    +  auto expr = expression::compile("${attr:substringBefore('attr_value_a__')}");
    --- End diff --
    
    I saw the un-guarded substring calls in expr_substringBefore. I assumed that calling args[0] is likely not possible since there were no conditionals; however, what happens if the user enters attr:substrinBefore ? Will they receive an error that is gracefully handled? 


---

[GitHub] nifi-minifi-cpp issue #219: MINIFICPP-330 Implemented Expression Language su...

Posted by achristianson <gi...@git.apache.org>.
Github user achristianson commented on the issue:

    https://github.com/apache/nifi-minifi-cpp/pull/219
  
    @phrocker yes, if they call substringBefore on an attr with no other arguments, then they should get an "attempted to call incomplete function." I'll add a test case for that.


---