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

Conversation

@nickwallen
Copy link
Contributor

@nickwallen nickwallen commented Jul 3, 2019

This change updates the REST service to work with HBase 2.0.2

Changes

  • Modified the UserSettingsClient to use the HBaseClient abstraction.
  • This drove some of the additional operations that were added to the HBaseClient in METRON-2175 Introduce HBase Connection Abstractions for HBase 2.0.2 #1456.
  • Made the IndexDao extend Closeable so that when the DAOs are instantiated via Spring, there is a way to clean-up resources, like HBase connections.
  • Modified the HBaseDao and related tests to work with HBase 2.0.2.
  • Created the FakeHBaseClient which makes testing anything that interacts with HBase, like HBaseSettingsClient possible.
    • While this class is more complex than I'd like, I think its pros outweigh the cons. It will be used in quite a few places to enable testing of classes that would normally need to interact with HBase. It is also an improvement over the previous "Mock HBase" code because its implementation does not rely on any HBase APIs.

Acceptance Testing

This should not impact the deployment and functioning of the system. As a sanity check, spin-up Full Dev and ensure alerts are visible.

In addition, you can follow these steps to save and retrieve a user setting.

  1. Go to the Swagger UI.

  2. Retrieve the current user's settings using GET /api/v1/alerts/ui/settings. This should return 404 initially indicating there are no settings for the current user.

  3. Save a user setting. Pass the following as the alertsUIUserSettings body to POST /api/v1/alerts/ui/settings.

    {
      "facetFields": [
        "ip_src_addr"
      ]
      "user": "admin"
    }
    
  4. Retrieve the setting again using GET /api/v1/alerts/ui/settings. This should return 200 with the following response body

    {
      "user": "admin",
      "tableColumns": null,
      "savedSearches": null,
      "facetFields": [
        "ip_src_addr"
      ]
    }
    

Pull Request Checklist

  • Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
  • 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)?
  • Have you included steps to reproduce the behavior or problem that is being changed or addressed?
  • Have you included steps or a guide to how the change may be verified and tested manually?
  • Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
  • Have you written or updated unit tests and or integration tests to verify your changes?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?

boolean success = false;
try {
userSettingsClient.delete(user, ALERT_USER_SETTING_TYPE);
if(userSettingsClient.findOne(user, ALERT_USER_SETTING_TYPE).isPresent()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was needed to maintain the behavior of returning false if you attempt to delete settings that don't exist. When issuing the delete you don't get any indication of whether something was deleted or not.

expected.setTableColumns(Collections.singletonList("user1_field"));

// save a profile for current user
userSettingsClient.save(user1, ALERT_USER_SETTING_TYPE, toJSON(expected));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Being able to use a real HBaseSettingsClient instead of a mock made these tests much easier to grok and maintain in my opinion. Previously we had to use a lot of mockito-magic to test this.

@merrimanr
Copy link
Contributor

I ran through the test instructions and everything works as expected. I also tested the admin user settings endpoints and those also work as expected.

After seeing the FakeHBaseClient class in context, I agree there is no easy way to do this. This approach is nice because we can use the same underlying classes for multiple tests. Otherwise we would need to mock each test individually. I'm am satisfied with the tradeoff you have chosen. +1

@nickwallen
Copy link
Contributor Author

Thanks for the review. This has been merged into feature/METRON-2088-support-hdp-3.1.

@nickwallen nickwallen closed this Jul 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants