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

NIFI-6752 Create ASN.1 RecordReader #4577

Closed
wants to merge 9 commits into from
Closed

Conversation

tpalfy
Copy link
Contributor

@tpalfy tpalfy commented Oct 6, 2020

https://issues.apache.org/jira/browse/NIFI-6752

This is a collaborative work with @ijokarumawak and the continuation of
#3796

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 NIFI-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 main)?

  • Is your initial contribution a single, squashed commit? Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not squash or use --force when pushing to allow for clean monitoring of changes.

For code changes:

  • Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
  • Have you written or updated unit tests to verify your changes?
  • Have you verified that the full build is successful on JDK 8?
  • Have you verified that the full build is successful on JDK 11?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
  • If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
  • If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?

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 GitHub Actions CI for build issues and submit an update to your PR as soon as possible.

@pvillard31
Copy link
Contributor

A few checkstyle violations to fix in this PR.

@mattyb149
Copy link
Contributor

Reviewing

@mattyb149
Copy link
Contributor

I'm having trouble testing this using the ASN1.xml template in the test resources. It looks like there's an unsupported property pointing to a JAR file that I can't find (/tmp/jasn1-example.jar) and (I assume as a result) the Root Model Class Name (set to org.apache.nifi.jasn1.example.Composite) cannot be found. Should we include that JAR in the test resources or is it something I need to build myself? If the latter, can you explain how to do it? Thanks!

@mattyb149
Copy link
Contributor

Also the versions are out of date, I can change them to 1.14.0-SNAPSHOT on merge unless you're making other changes (related to my comment above)

@tpalfy
Copy link
Contributor Author

tpalfy commented Feb 19, 2021

@mattyb149 I'll update the template.
The ASN files no longer needed to be compiled by the user. They can be provided in a property and NiFi will do the rest.

This is a fairly old PR so I'll rebase it onto main. That should take care of the version issue as well.

ijokarumawak and others added 7 commits February 19, 2021 16:03
…or more types. Added more tests.

Removed 'parent' from 'Recursive'. (Caused issues. The recursive nature is still there as it has a child with the same type).
Updated jasn1 1.11.2 to asn1bean 1.12.0. If an asn field name is a Java reserved keyword, the field gets a trailing "_" but the getter remains normal. In JASN1Utils adjusted logic when looking for the getter.
Added support for inherited types. OctetStrings are converted to Strings instead of byte arrays.
Service takes care of the compilation of the ASN files. Test sources are generated and removed from source control.
<dependency>
<groupId>org.apache.nifi</groupId>
<artifactId>nifi-asn1-nar</artifactId>
<version>1.13.0-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be 1.14.0-SNAPSHOT

}

private static void generateComposite(File dir) throws IOException {
final File file = new File(dir, "composite.dat");
Copy link
Contributor

Choose a reason for hiding this comment

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

When I run the new template, I get null for all values in the record(s), is something mis-configured? The checksum of the file in examples is the same as those generated by this test, and the unit tests run fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The template uses ForkRecord for the composite data. It seems that ForkRecord doesn't work. Not sure what exactly the issue is but I think it might have something to do with the fact that no matter the settings it uses the original schema when writing the extracted records. Not sure if the record writer logic has changed at some point or this processor never worked properly, at least with certain setup & input.

In any case, this doesn't seem to be related to the ASN.1 functionality. I'll replace that processor with a different one in the template.

…emplate. Updated (fixed) nifi-asn1-nar version in pom.xml.
@@ -0,0 +1,1360 @@
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs an Apache header or to be added to the Rat exclusion list (I prefer the former)

@mattyb149
Copy link
Contributor

+1 LGTM, tried the example template and verified comments were incorporated. Thanks for the new feature! Merging to main

@mattyb149 mattyb149 closed this in 68d38dd Feb 25, 2021
driesva pushed a commit to driesva/nifi that referenced this pull request Mar 19, 2021
NIFI-6752 Refactored type and value conversion logic. Added support for more types. Added more tests.
Removed 'parent' from 'Recursive'. (Caused issues. The recursive nature is still there as it has a child with the same type).
Updated jasn1 1.11.2 to asn1bean 1.12.0. If an asn field name is a Java reserved keyword, the field gets a trailing "_" but the getter remains normal. In JASN1Utils adjusted logic when looking for the getter.
Added support for inherited types. OctetStrings are converted to Strings instead of byte arrays.
Service takes care of the compilation of the ASN files. Test sources are generated and removed from source control.

NIFI-6752 Removed obsolete TODOs.

NIFI-6752 Updated nifi-asn1-nar version to 1.13.0-SNAPSHOT. Fixed checkstyle violations (unused imports).

NIFI-6752 ASN.1 reader - ASN.1 bundle requires 'include-asn1' profile to be active to be part of assembly.

NIFI-6752 ASN.1 reader - Updated ASN1.xml template.

NIFI-6752 ASN.1 reader - Updated versions.

NIFI-6752 ASN.1 reader - Update example generator. Updated ASN1.xml template. Updated (fixed) nifi-asn1-nar version in pom.xml.

NIFI-6752 ASN.1 reader - Added missing license for ASN1.xml.

Signed-off-by: Matthew Burgess <mattyb149@apache.org>

This closes apache#4577
krisztina-zsihovszki pushed a commit to krisztina-zsihovszki/nifi that referenced this pull request Jun 28, 2022
NIFI-6752 Refactored type and value conversion logic. Added support for more types. Added more tests.
Removed 'parent' from 'Recursive'. (Caused issues. The recursive nature is still there as it has a child with the same type).
Updated jasn1 1.11.2 to asn1bean 1.12.0. If an asn field name is a Java reserved keyword, the field gets a trailing "_" but the getter remains normal. In JASN1Utils adjusted logic when looking for the getter.
Added support for inherited types. OctetStrings are converted to Strings instead of byte arrays.
Service takes care of the compilation of the ASN files. Test sources are generated and removed from source control.

NIFI-6752 Removed obsolete TODOs.

NIFI-6752 Updated nifi-asn1-nar version to 1.13.0-SNAPSHOT. Fixed checkstyle violations (unused imports).

NIFI-6752 ASN.1 reader - ASN.1 bundle requires 'include-asn1' profile to be active to be part of assembly.

NIFI-6752 ASN.1 reader - Updated ASN1.xml template.

NIFI-6752 ASN.1 reader - Updated versions.

NIFI-6752 ASN.1 reader - Update example generator. Updated ASN1.xml template. Updated (fixed) nifi-asn1-nar version in pom.xml.

NIFI-6752 ASN.1 reader - Added missing license for ASN1.xml.

Signed-off-by: Matthew Burgess <mattyb149@apache.org>

This closes apache#4577
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants