GitHub user merrimanr opened a pull request:
https://github.com/apache/metron/pull/694
METRON-1085: Add REST endpoint to save a user profile for the Alerts UI
## Contributor Comments
This PR adds an ORM framework to the REST application. An ORM (object
relational mapping) framework provides tools for mapping java objects to
relational database tables. Hibernate is probably the most well-known but is
not used here because of license issues. Eclipselink is used instead and is a
suitable replacement because it is Apache license friendly.
This PR also includes a REST endpoint that leverages this feature. The
intention is to give reviewers some context around how and why an ORM framework
is needed. The new endpoint allows a user to persist their searches in a
database and also allows an admin user to manage users' searches. These saved
searches are part of an "AlertsProfile" that is used in the alerts UI and will
replace the current approach of persisting user data client-side. Saved
searches are highlighted here but other information can (and will) be added as
needed. For example, a user's preferred display columns are also stored in
this profile.
This can be tested in full dev as follows:
1. Navigate to the
[alerts-profile-controller](http://node1:8082/swagger-ui.html#!/alerts-profile-controller)
in Swagger and login as "user1/password".
2. Create a new profile with the
[save](http://node1:8082/swagger-ui.html#!/alerts-profile-controller/saveUsingPOST)
function. The actual values in the profile are not important as long as you
can differentiate one profile from another. This is the profile I used for
user 1:
```
{
"savedSearches": [
{
"name": "user 1 search",
"searchRequest": {
"facetFields": [
"string"
],
"from": 0,
"indices": [
"string"
],
"query": "string",
"size": 0
}
}
],
"tableColumns": [
"string"
]
}
```
3. Login as "user2/password" (a new incognito window in chrome will work)
and create a profile.
4. The
[get](http://node1:8082/swagger-ui.html#!/alerts-profile-controller/getUsingGET)
function should return the profile of the logged in user.
5. Try to execute the
[getall](http://node1:8082/swagger-ui.html#!/alerts-profile-controller/findAllUsingGET)
or
[delete](http://node1:8082/swagger-ui.html#!/alerts-profile-controller/deleteUsingDELETE)
functions with either user1 or user2. You should get a 403 permission error.
6. Login as "admin/password" in a new window. You should now be able to
execute the getall and delete functions.
There are a couple design choices I would like to highlight and get the
community's feedback on:
1. I chose Eclipselink because it is license friendly and used in other
Apache projects. Is this the correct replacement for Hibernate?
2. The "savedSearches" and "tableColumns" fields are persisted as JSON
blobs in a single table instead of a normalized data model. In my experience
with ORM frameworks, it can sometimes be advantageous to store a serialized
version in a single column as opposed to representing an entity with multiple
tables. Consider the "tableColumns" field. A normalized approach would be a
separate "TableColumn" table with a foreign key column that references the
"AlertsProfile" table. This is not attractive in this specific case for a
couple reasons: the "TableColumn" table will end up having mostly duplicate
data and accessing the tables columns will only happen in one direction, you
will never start with a table column and lookup an alerts profile. The
"TableColumn" table will grow over time, lookups will eventually become slower
and database tuning will be needed.
3. I took the simpler approach to securing a user's profile. The "id" on
the "AlertsProfile" table is a user's id and the REST app uses the current
user's id for lookup. The only way to get another user's profile is to be
logged in as an admin user and list all profiles. Alternatively, this could
have been achieved by managing a list of "AlertsProfile" ACLs but I felt that
was overkill.
4. I added a short description and reference to Eclipselink in the REST
README. Is this sufficient documentation?
## Pull Request Checklist
Thank you for submitting a contribution to Apache Metron.
Please refer to our [Development
Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235)
for the complete guide to follow for contributions.
Please refer also to our [Build Verification
Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview)
for complete smoke testing guides.
In order to streamline the review of the contribution we ask you follow
these guidelines and ask you to double check the following:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? If not one needs to
be created at [Metron
Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
- [ ] Does your PR title start with METRON-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)?
### For code changes:
- [x] Have you included steps to reproduce the behavior or problem that is
being changed or addressed?
- [x] Have you included steps or a guide to how the change may be verified
and tested manually?
- [x] Have you ensured that the full suite of tests and checks have been
executed in the root metron folder via:
```
mvn -q clean integration-test install && build_utils/verify_licenses.sh
```
- [x] Have you written or updated unit tests and or integration tests to
verify your changes?
- [x] 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)?
- [x] Have you verified the basic functionality of the build by building
and running locally with Vagrant full-dev environment or the equivalent?
### For documentation related changes:
- [x] Have you ensured that format looks appropriate for the output in
which it is rendered by building and verifying the site-book? If not then run
the following commands and the verify changes via
`site-book/target/site/index.html`:
```
cd site-book
mvn site
```
#### 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.
It is also recommended that [travis-ci](https://travis-ci.org) is set up
for your personal repository such that your branches are built there before
submitting a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/merrimanr/incubator-metron METRON-1085
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/694.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 #694
----
commit 010b5c23a5d1ae704454c36ba30bc2364b8eff5b
Author: merrimanr <[email protected]>
Date: 2017-08-04T20:24:11Z
initial commit
commit 9a4281cbd0ea4c382c411bc1f107d523125cb4b7
Author: merrimanr <[email protected]>
Date: 2017-08-10T20:28:03Z
added documentation
commit 9a2bf6199a8160a82924598213f79df4e26d21aa
Author: merrimanr <[email protected]>
Date: 2017-08-10T20:40:12Z
updated eclipselink plugin and excluded hibernate dependencies
commit 179aeaf33859bf20bf6f9ce4e17b7350a4e23830
Author: merrimanr <[email protected]>
Date: 2017-08-10T20:40:30Z
added dependency licenses
commit 0ddceaaeab33ce21d96eb9c1f53cba58d1305f2b
Author: merrimanr <[email protected]>
Date: 2017-08-10T20:42:14Z
added unit test
commit 0a75e79bfa86b7430a54e798cf5ef0c1eab4ab0b
Author: merrimanr <[email protected]>
Date: 2017-08-10T22:09:32Z
reverted log level
commit 54d37396c9ededf72eed98895f1abb6c611a087a
Author: merrimanr <[email protected]>
Date: 2017-08-10T22:13:25Z
Merge remote-tracking branch 'mirror/master' into METRON-1085
# Conflicts:
#
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/MetronRestConstants.java
# metron-interface/metron-rest/src/main/resources/application.yml
commit 00f787e1224fbc9ec9136d197bda69ebca54b21d
Author: merrimanr <[email protected]>
Date: 2017-08-11T20:18:15Z
Merge remote-tracking branch 'mirror/master' into METRON-1085
commit b13e0da86b82c8a7c0d1dc560b140e7e1d7378d3
Author: merrimanr <[email protected]>
Date: 2017-08-11T22:23:00Z
Fixed merge conflicts
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
