Skip to content
This repository was archived by the owner on Feb 16, 2025. It is now read-only.

CM H3 Java JNI whitelist plugin #3889

Merged
merged 22 commits into from
Aug 26, 2021
Merged

CM H3 Java JNI whitelist plugin #3889

merged 22 commits into from
Aug 26, 2021

Conversation

tucek
Copy link
Contributor

@tucek tucek commented Jun 9, 2021 •
edited
Loading

  • Added whitelist Java JNI checker plugin
  • Removed plugin loaded java plugin loading

Basics

These points need to be fulfilled for every PR:

  • Short descriptions of your changes are in the release notes
    (added as entry in doc/news/_preparation_next_release.md which
    contains _(my name)_)
    Please always add something to the release notes.
  • Details of what you changed are in commit messages
    (first line should have module: short statement syntax)
  • References to issues, e.g. close #X, are in the commit messages.
  • The buildservers are happy. If not, fix in this order:
    • add a line in doc/news/_preparation_next_release.md
    • reformat the code with scripts/dev/reformat-all
    • make all unit tests pass
    • fix all memleaks
  • The PR is rebased with current master.

If you have any troubles fulfilling these criteria, please write
about the trouble as comment in the PR. We will help you.
But we cannot accept PRs that do not fulfill the basics.

Checklist

Check relevant points but please do not remove entries.
For docu fixes, spell checking, and similar none of these points below
need to be checked.

  • I added unit tests for my code
  • I fully described what my PR does in the documentation
    (not in the PR description)
  • I fixed all affected documentation
  • I added code comments, logging, and assertions as appropriate (see Coding Guidelines)
  • I updated all meta data (e.g. README.md of plugins and METADATA.ini)
  • I mentioned every code not directly written by me in THIRD-PARTY-LICENSES

Review

Reviewers will usually check the following:

Labels

If you are already Elektra developer:

  • Add the "work in progress" label if you do not want the PR to be reviewed yet.
  • Add the "ready to merge" label if the basics are fulfilled and you also
    say that everything is ready to be merged.

@tucek tucek marked this pull request as ready for review June 9, 2021 22:06
@tucek tucek force-pushed the javaJniWhitelistPlugin branch 2 times, most recently from e3822ed to 550c42b Compare June 9, 2021 23:25
@tucek
Copy link
Contributor Author

tucek commented Jun 9, 2021

I don't get why the build checks are failing (e.g. coverage?)

@mpranj
Copy link
Member

mpranj commented Jun 11, 2021

I don't get why the build checks are failing (e.g. coverage?)

The formatting check is failing so the other tests are aborted and the coverage can not be determined.

@tucek
Copy link
Contributor Author

tucek commented Jul 4, 2021 •
edited
Loading

to fix: (feedback from @markus2330)

  • severe logic error: only parentKey and not keys in keySet get checked
  • check/whitelist not properly specified, differences to enum?
  • also allows garbage below check/whitelist

@tucek tucek force-pushed the javaJniWhitelistPlugin branch from 1677a02 to 2ff4aa3 Compare July 4, 2021 14:37
@tucek tucek force-pushed the javaJniWhitelistPlugin branch from 4497638 to c6b72ca Compare August 6, 2021 11:58
@tucek tucek self-assigned this Aug 6, 2021
@tucek tucek mentioned this pull request Aug 6, 2021
Merged
20 tasks
tucek pushed a commit to tucek/libelektra that referenced this pull request Aug 6, 2021
ElektraInitiative#3889
76eeb01 
- fixed whitelist java plugin
- updated tutorials/java-kdb.md
- changed `Key nextMeta()` to `Optional<Key> nextMeta ()`
@tucek
Copy link
Contributor Author

tucek commented Aug 6, 2021

@markus2330

  • check/whitelist not properly specified, differences to enum?

What exactly do you expect here? The whitelist plugin is a simple sample Java plugin. In contrary to the enum plugin it is not case insensitive and does not do any normalizing.

@tucek
Copy link
Contributor Author

tucek commented Aug 6, 2021

Should i also implement #3871 within this PR?

@tucek tucek requested review from kodebach and markus2330 August 6, 2021 16:23
@tucek
Copy link
Contributor Author

tucek commented Aug 6, 2021

I will rebase this PR on #3790, since this should be merged before anyways

tucek pushed a commit to tucek/libelektra that referenced this pull request Aug 6, 2021
ElektraInitiative#3889
e23c30a 
- fixed whitelist java plugin
- updated tutorials/java-kdb.md
- changed `Key nextMeta()` to `Optional<Key> nextMeta ()`
@tucek tucek force-pushed the javaJniWhitelistPlugin branch from b404f93 to 11d3985 Compare August 6, 2021 17:16
@restyled-io restyled-io bot mentioned this pull request Aug 6, 2021
Closed
@tucek tucek mentioned this pull request Aug 6, 2021
Merged
20 tasks
tucek pushed a commit to tucek/libelektra that referenced this pull request Aug 6, 2021
ElektraInitiative#3889
ea55b28 
- fixed whitelist java plugin
- updated tutorials/java-kdb.md
- changed `Key nextMeta()` to `Optional<Key> nextMeta ()`
@tucek tucek force-pushed the javaJniWhitelistPlugin branch from 79eb945 to 5e9e251 Compare August 6, 2021 23:27
tucek pushed a commit to tucek/libelektra that referenced this pull request Aug 8, 2021
ElektraInitiative#3889
bffe61a 
- fixed whitelist java plugin
- updated tutorials/java-kdb.md
- changed `Key nextMeta()` to `Optional<Key> nextMeta ()`
@tucek tucek force-pushed the javaJniWhitelistPlugin branch from 5e9e251 to 891adae Compare August 8, 2021 10:40
@tucek tucek linked an issue Aug 9, 2021 that may be closed by this pull request
Add binary value key support [JNA bindings] #3969
Closed
@tucek
Copy link
Contributor Author

tucek commented Aug 9, 2021

#3969 is implemented in this PR

@tucek tucek marked this pull request as draft August 9, 2021 13:16
@tucek tucek linked an issue Aug 9, 2021 that may be closed by this pull request
Example App throws exception [JNA bindings] #3757
Closed
@tucek
Copy link
Contributor Author

tucek commented Aug 9, 2021

#3757 is implemented in this PR

@restyled-io restyled-io bot mentioned this pull request Aug 12, 2021
Closed
Michael Tucek and others added 19 commits August 26, 2021 20:01
@tucek
- updated CMakeLists.txt with new files
9289fd2 
- disabled test for darwin (as was before, but not listing every one individually for exclusion)
@tucek
added missing WhitelistPluginTests
fcf0a63
@tucek
applied style
4bf1864
@tucek
ElektraInitiative#3889
b35c6a2 
- fixed whitelist java plugin
- updated tutorials/java-kdb.md
- changed `Key nextMeta()` to `Optional<Key> nextMeta ()`
@tucek
updated release notes
083ec97
@tucek
fixed WhitelistPluginTests after rebase
e8f5f0e
@tucek
applied markdoen restyle
c2eb037
@tucek
updated documentation
35d2a0e
@tucek
updated release notes
f2b62f8
@tucek
applied markdoen restyle
cbbcb7a
@tucek
fixed release notes
6259535
@tucek
KDBClosedException, KeyReleasedException and KeySetReleasedException …
62157ca 
…are now inheriting IllegalStateException
@tucek
ElektraInitiative#3969 Added support of binary valued keys
2eb24e0
@tucek
ElektraInitiative#3969 fixed JNA CMakeLists.txt
da42974
@tucek
ElektraInitiative#3757 fixed examples/external/java/read-keys-example…
1e8586c 
… project
@tucek
ElektraInitiative#3889 integrated review feedback
8f45067
@tucek
ElektraInitiative#3889 added more details to release notes
d366ccf
@tucek
ElektraInitiative#3889 removed gradle wrapper for external Java examp…
0af1c9a 
…le project
@mpranj @tucek
Apply suggestions from code review
3f18960
@tucek tucek force-pushed the javaJniWhitelistPlugin branch from 1ccec87 to 3f18960 Compare August 26, 2021 17:02
@tucek
Copy link
Contributor Author

tucek commented Aug 26, 2021

I've rebased this branch to Elektra master.

@mpranj mpranj merged commit 0bf8dfc into ElektraInitiative:master Aug 26, 2021
@mpranj
Copy link
Member

mpranj commented Aug 26, 2021

Thank you so much, great work! 🚀

@tucek
Copy link
Contributor Author

tucek commented Aug 26, 2021

@mpranj Thank you for merging! ❤️

@tucek tucek deleted the javaJniWhitelistPlugin branch August 26, 2021 22:45
@mpranj mpranj added this to the 0.9.8 milestone Aug 29, 2021
lawli3t pushed a commit to lawli3t/libelektra that referenced this pull request Sep 15, 2021
@lawli3t
ElektraInitiative#3889
4488d1c 
- fixed whitelist java plugin
- updated tutorials/java-kdb.md
- changed `Key nextMeta()` to `Optional<Key> nextMeta ()`
lawli3t pushed a commit to lawli3t/libelektra that referenced this pull request Sep 15, 2021
@lawli3t
ElektraInitiative#3889 integrated review feedback
d0ccd4b
lawli3t pushed a commit to lawli3t/libelektra that referenced this pull request Sep 15, 2021
@tucek @lawli3t
ElektraInitiative#3889 added more details to release notes
4abd2a0
lawli3t pushed a commit to lawli3t/libelektra that referenced this pull request Sep 15, 2021
@tucek @lawli3t
ElektraInitiative#3889 removed gradle wrapper for external Java examp…
938d992 
…le project
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mpranj mpranj mpranj left review comments

@markus2330 markus2330 markus2330 left review comments

@kodebach kodebach kodebach left review comments

Assignees

@tucek tucek

Labels
None yet
Projects
None yet
Milestone
0.9.8
Development

Successfully merging this pull request may close these issues.

Add binary value key support [JNA bindings] Example App throws exception [JNA bindings]
4 participants
@tucek @mpranj @markus2330 @kodebach