You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@metron.apache.org by "ASF GitHub Bot (JIRA)" <ji...@apache.org> on 2017/08/14 14:44:00 UTC
[jira] [Commented] (METRON-1085) Add REST endpoint to save a user
profile for the Alerts UI
[ https://issues.apache.org/jira/browse/METRON-1085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16125769#comment-16125769 ]
ASF GitHub Bot commented on METRON-1085:
----------------------------------------
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 <me...@gmail.com>
Date: 2017-08-04T20:24:11Z
initial commit
commit 9a4281cbd0ea4c382c411bc1f107d523125cb4b7
Author: merrimanr <me...@gmail.com>
Date: 2017-08-10T20:28:03Z
added documentation
commit 9a2bf6199a8160a82924598213f79df4e26d21aa
Author: merrimanr <me...@gmail.com>
Date: 2017-08-10T20:40:12Z
updated eclipselink plugin and excluded hibernate dependencies
commit 179aeaf33859bf20bf6f9ce4e17b7350a4e23830
Author: merrimanr <me...@gmail.com>
Date: 2017-08-10T20:40:30Z
added dependency licenses
commit 0ddceaaeab33ce21d96eb9c1f53cba58d1305f2b
Author: merrimanr <me...@gmail.com>
Date: 2017-08-10T20:42:14Z
added unit test
commit 0a75e79bfa86b7430a54e798cf5ef0c1eab4ab0b
Author: merrimanr <me...@gmail.com>
Date: 2017-08-10T22:09:32Z
reverted log level
commit 54d37396c9ededf72eed98895f1abb6c611a087a
Author: merrimanr <me...@gmail.com>
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 <me...@gmail.com>
Date: 2017-08-11T20:18:15Z
Merge remote-tracking branch 'mirror/master' into METRON-1085
commit b13e0da86b82c8a7c0d1dc560b140e7e1d7378d3
Author: merrimanr <me...@gmail.com>
Date: 2017-08-11T22:23:00Z
Fixed merge conflicts
----
> Add REST endpoint to save a user profile for the Alerts UI
> ----------------------------------------------------------
>
> Key: METRON-1085
> URL: https://issues.apache.org/jira/browse/METRON-1085
> Project: Metron
> Issue Type: New Feature
> Reporter: Ryan Merriman
> Assignee: Ryan Merriman
>
> We need a way to persist user data for the Alerts UI. This could include saved searches, preferences, UI layout, etc. This will require an ORM framework that is Apache license friendly.
> This endpoint should enforce security and allow a user to see only his profile. An admin user should be able to see all profiles and delete a profile.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)