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 <merrim...@gmail.com>
Date:   2017-08-04T20:24:11Z

    initial commit

commit 9a4281cbd0ea4c382c411bc1f107d523125cb4b7
Author: merrimanr <merrim...@gmail.com>
Date:   2017-08-10T20:28:03Z

    added documentation

commit 9a2bf6199a8160a82924598213f79df4e26d21aa
Author: merrimanr <merrim...@gmail.com>
Date:   2017-08-10T20:40:12Z

    updated eclipselink plugin and excluded hibernate dependencies

commit 179aeaf33859bf20bf6f9ce4e17b7350a4e23830
Author: merrimanr <merrim...@gmail.com>
Date:   2017-08-10T20:40:30Z

    added dependency licenses

commit 0ddceaaeab33ce21d96eb9c1f53cba58d1305f2b
Author: merrimanr <merrim...@gmail.com>
Date:   2017-08-10T20:42:14Z

    added unit test

commit 0a75e79bfa86b7430a54e798cf5ef0c1eab4ab0b
Author: merrimanr <merrim...@gmail.com>
Date:   2017-08-10T22:09:32Z

    reverted log level

commit 54d37396c9ededf72eed98895f1abb6c611a087a
Author: merrimanr <merrim...@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 <merrim...@gmail.com>
Date:   2017-08-11T20:18:15Z

    Merge remote-tracking branch 'mirror/master' into METRON-1085

commit b13e0da86b82c8a7c0d1dc560b140e7e1d7378d3
Author: merrimanr <merrim...@gmail.com>
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to