Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-5871

Enforce uniqueness of pattern names in CEP.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.0
    • Fix Version/s: 1.3.0
    • Component/s: CEP
    • Labels:

      Description

      Currently, although required, the NFACompiler does not impose uniqueness of the chosen pattern names. This issue addresses that, and the proposed strategy is to throw an exception whenever a malformed pattern like this is encountered.

        Issue Links

          Activity

          Hide
          kkl0u Kostas Kloudas added a comment -

          Merged at commit a92d78746318e9bbe949d9d030fc21b5348b56e2

          Show
          kkl0u Kostas Kloudas added a comment - Merged at commit a92d78746318e9bbe949d9d030fc21b5348b56e2
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/3381

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3381
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user kl0u commented on the issue:

          https://github.com/apache/flink/pull/3381

          Thanks for the review @tillrohrmann ! I just had a question about one of the comments.

          Show
          githubbot ASF GitHub Bot added a comment - Github user kl0u commented on the issue: https://github.com/apache/flink/pull/3381 Thanks for the review @tillrohrmann ! I just had a question about one of the comments.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user kl0u commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3381#discussion_r102916874

          — Diff: flink-libraries/flink-cep/src/test/java/org/apache/flink/cep/nfa/compiler/NFACompilerTest.java —
          @@ -42,6 +45,43 @@

          public class NFACompilerTest extends TestLogger {

          + @Rule
          + public ExpectedException expectedException = ExpectedException.none();
          +
          + @Test
          + public void testNFACompilerUniquePatternName() {
          +
          + // adjust the rule
          + expectedException.expect(MalformedPatternException.class);
          + expectedException.expectMessage("Duplicate pattern name: start. Pattern names must be unique.");
          +
          + Pattern<Event, ?> invalidPattern = Pattern.<Event>begin("start").where(new FilterFunction<Event>() {
          + private static final long serialVersionUID = -3863103355752267133L;
          +
          + @Override
          + public boolean filter(Event value) throws Exception

          { + return value.getName().equals("start"); + }

          + }).followedBy("middle").subtype(SubEvent.class).where(new FilterFunction<SubEvent>() {
          + private static final long serialVersionUID = -1413973420880409702L;
          +
          + @Override
          + public boolean filter(SubEvent value) throws Exception

          { + return value.getVolume() > 10.0; + }

          + }).followedBy("start").subtype(SubEvent.class).where(new FilterFunction<SubEvent>() {
          + private static final long serialVersionUID = -1413973420880409702L;
          +
          + @Override
          + public boolean filter(SubEvent value) throws Exception

          { + return value.getVolume() > 11.0; + }

          + }).within(Time.milliseconds(10));
          — End diff –

          Thanks for the review @tillrohrmann . I just had a question about one of your comments.

          Show
          githubbot ASF GitHub Bot added a comment - Github user kl0u commented on a diff in the pull request: https://github.com/apache/flink/pull/3381#discussion_r102916874 — Diff: flink-libraries/flink-cep/src/test/java/org/apache/flink/cep/nfa/compiler/NFACompilerTest.java — @@ -42,6 +45,43 @@ public class NFACompilerTest extends TestLogger { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Test + public void testNFACompilerUniquePatternName() { + + // adjust the rule + expectedException.expect(MalformedPatternException.class); + expectedException.expectMessage("Duplicate pattern name: start. Pattern names must be unique."); + + Pattern<Event, ?> invalidPattern = Pattern.<Event>begin("start").where(new FilterFunction<Event>() { + private static final long serialVersionUID = -3863103355752267133L; + + @Override + public boolean filter(Event value) throws Exception { + return value.getName().equals("start"); + } + }).followedBy("middle").subtype(SubEvent.class).where(new FilterFunction<SubEvent>() { + private static final long serialVersionUID = -1413973420880409702L; + + @Override + public boolean filter(SubEvent value) throws Exception { + return value.getVolume() > 10.0; + } + }).followedBy("start").subtype(SubEvent.class).where(new FilterFunction<SubEvent>() { + private static final long serialVersionUID = -1413973420880409702L; + + @Override + public boolean filter(SubEvent value) throws Exception { + return value.getVolume() > 11.0; + } + }).within(Time.milliseconds(10)); — End diff – Thanks for the review @tillrohrmann . I just had a question about one of your comments.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user kl0u commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3381#discussion_r102916718

          — Diff: flink-libraries/flink-cep/src/test/java/org/apache/flink/cep/nfa/compiler/NFACompilerTest.java —
          @@ -42,6 +45,43 @@

          public class NFACompilerTest extends TestLogger {

          + @Rule
          + public ExpectedException expectedException = ExpectedException.none();
          +
          + @Test
          + public void testNFACompilerUniquePatternName() {
          +
          + // adjust the rule
          + expectedException.expect(MalformedPatternException.class);
          + expectedException.expectMessage("Duplicate pattern name: start. Pattern names must be unique.");
          — End diff –

          I match on both type and message. Do you suggest to remove the message matching part or you have a better way to do so?

          Show
          githubbot ASF GitHub Bot added a comment - Github user kl0u commented on a diff in the pull request: https://github.com/apache/flink/pull/3381#discussion_r102916718 — Diff: flink-libraries/flink-cep/src/test/java/org/apache/flink/cep/nfa/compiler/NFACompilerTest.java — @@ -42,6 +45,43 @@ public class NFACompilerTest extends TestLogger { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Test + public void testNFACompilerUniquePatternName() { + + // adjust the rule + expectedException.expect(MalformedPatternException.class); + expectedException.expectMessage("Duplicate pattern name: start. Pattern names must be unique."); — End diff – I match on both type and message. Do you suggest to remove the message matching part or you have a better way to do so?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tillrohrmann commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3381#discussion_r102471183

          — Diff: flink-libraries/flink-cep/src/test/java/org/apache/flink/cep/nfa/compiler/NFACompilerTest.java —
          @@ -42,6 +45,43 @@

          public class NFACompilerTest extends TestLogger {

          + @Rule
          + public ExpectedException expectedException = ExpectedException.none();
          +
          + @Test
          + public void testNFACompilerUniquePatternName() {
          +
          + // adjust the rule
          + expectedException.expect(MalformedPatternException.class);
          + expectedException.expectMessage("Duplicate pattern name: start. Pattern names must be unique.");
          +
          + Pattern<Event, ?> invalidPattern = Pattern.<Event>begin("start").where(new FilterFunction<Event>() {
          + private static final long serialVersionUID = -3863103355752267133L;
          +
          + @Override
          + public boolean filter(Event value) throws Exception

          { + return value.getName().equals("start"); + }

          + }).followedBy("middle").subtype(SubEvent.class).where(new FilterFunction<SubEvent>() {
          + private static final long serialVersionUID = -1413973420880409702L;
          +
          + @Override
          + public boolean filter(SubEvent value) throws Exception

          { + return value.getVolume() > 10.0; + }

          + }).followedBy("start").subtype(SubEvent.class).where(new FilterFunction<SubEvent>() {
          + private static final long serialVersionUID = -1413973420880409702L;
          +
          + @Override
          + public boolean filter(SubEvent value) throws Exception

          { + return value.getVolume() > 11.0; + }

          + }).within(Time.milliseconds(10));
          — End diff –

          Test pattern could be much more concise for the test case.

          Show
          githubbot ASF GitHub Bot added a comment - Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/3381#discussion_r102471183 — Diff: flink-libraries/flink-cep/src/test/java/org/apache/flink/cep/nfa/compiler/NFACompilerTest.java — @@ -42,6 +45,43 @@ public class NFACompilerTest extends TestLogger { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Test + public void testNFACompilerUniquePatternName() { + + // adjust the rule + expectedException.expect(MalformedPatternException.class); + expectedException.expectMessage("Duplicate pattern name: start. Pattern names must be unique."); + + Pattern<Event, ?> invalidPattern = Pattern.<Event>begin("start").where(new FilterFunction<Event>() { + private static final long serialVersionUID = -3863103355752267133L; + + @Override + public boolean filter(Event value) throws Exception { + return value.getName().equals("start"); + } + }).followedBy("middle").subtype(SubEvent.class).where(new FilterFunction<SubEvent>() { + private static final long serialVersionUID = -1413973420880409702L; + + @Override + public boolean filter(SubEvent value) throws Exception { + return value.getVolume() > 10.0; + } + }).followedBy("start").subtype(SubEvent.class).where(new FilterFunction<SubEvent>() { + private static final long serialVersionUID = -1413973420880409702L; + + @Override + public boolean filter(SubEvent value) throws Exception { + return value.getVolume() > 11.0; + } + }).within(Time.milliseconds(10)); — End diff – Test pattern could be much more concise for the test case.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tillrohrmann commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3381#discussion_r102471060

          — Diff: flink-libraries/flink-cep/src/test/java/org/apache/flink/cep/nfa/compiler/NFACompilerTest.java —
          @@ -42,6 +45,43 @@

          public class NFACompilerTest extends TestLogger {

          + @Rule
          + public ExpectedException expectedException = ExpectedException.none();
          +
          + @Test
          + public void testNFACompilerUniquePatternName() {
          +
          + // adjust the rule
          + expectedException.expect(MalformedPatternException.class);
          + expectedException.expectMessage("Duplicate pattern name: start. Pattern names must be unique.");
          — End diff –

          Matching on exception messages is fragile

          Show
          githubbot ASF GitHub Bot added a comment - Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/3381#discussion_r102471060 — Diff: flink-libraries/flink-cep/src/test/java/org/apache/flink/cep/nfa/compiler/NFACompilerTest.java — @@ -42,6 +45,43 @@ public class NFACompilerTest extends TestLogger { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Test + public void testNFACompilerUniquePatternName() { + + // adjust the rule + expectedException.expect(MalformedPatternException.class); + expectedException.expectMessage("Duplicate pattern name: start. Pattern names must be unique."); — End diff – Matching on exception messages is fragile
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user kl0u opened a pull request:

          https://github.com/apache/flink/pull/3381

          FLINK-5871 [cep] Enforce uniqueness of pattern names in CEP.

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

          $ git pull https://github.com/kl0u/flink cep-uniq-pattern-name

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

          https://github.com/apache/flink/pull/3381.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 #3381


          commit 326c2c341fba02b6e8e41038cb5e5d732c1ede76
          Author: kl0u <kkloudas@gmail.com>
          Date: 2017-02-21T12:51:11Z

          FLINK-5871 [cep] Enforce uniqueness of pattern names in CEP.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user kl0u opened a pull request: https://github.com/apache/flink/pull/3381 FLINK-5871 [cep] Enforce uniqueness of pattern names in CEP. You can merge this pull request into a Git repository by running: $ git pull https://github.com/kl0u/flink cep-uniq-pattern-name Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3381.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 #3381 commit 326c2c341fba02b6e8e41038cb5e5d732c1ede76 Author: kl0u <kkloudas@gmail.com> Date: 2017-02-21T12:51:11Z FLINK-5871 [cep] Enforce uniqueness of pattern names in CEP.

            People

            • Assignee:
              kkl0u Kostas Kloudas
              Reporter:
              kkl0u Kostas Kloudas
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development