metron-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From merrimanr <...@git.apache.org>
Subject [GitHub] metron pull request #694: METRON-1085: Add REST endpoint to save a user prof...
Date Mon, 14 Aug 2017 14:43:58 GMT
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 <merrimanr@gmail.com>
Date:   2017-08-04T20:24:11Z

    initial commit

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

    added documentation

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

    updated eclipselink plugin and excluded hibernate dependencies

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

    added dependency licenses

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

    added unit test

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

    reverted log level

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

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

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

Mime
View raw message