Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue #219: Refactor AppResources Controller #274

Open
wants to merge 8 commits into
base: feature/task_cwms_support
Choose a base branch
from
Open

Issue #219: Refactor AppResources Controller #274

wants to merge 8 commits into from

Conversation

zack-rma
Copy link
Collaborator

Problem Description

Fixes #219 .

Solution

Replaces DAO usage with OpenDCS DAI implementations.

how you tested the change

Unit tests included.

Where the following done:

  • Tests. Check all that apply:
    • Unit tests created or modified that run during ant test.
    • Integration tests created or modified that run during integration testing
      (Formerly called regression tests.)
    • Test procedure descriptions for manual testing
  • Was relevant documentation updated?
  • Were relevant config element (e.g. XML data) updated as appropriate

If you aren't sure leave unchecked and we will help guide you to want needs changing where.

@zack-rma zack-rma changed the base branch from main to feature/task_cwms_support December 17, 2024 19:31
@zack-rma zack-rma requested a review from adamkorynta December 17, 2024 19:33
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
13.0% Coverage on New Code (required ≥ 30%)

See analysis details on SonarQube Cloud

adamkorynta
adamkorynta requested changes Dec 18, 2024
View reviewed changes
Copy link
Contributor

@adamkorynta adamkorynta left a comment

Choose a reason for hiding this comment

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

rebasing will give you better code coverage metrics

opendcs-rest-api/src/main/java/org/opendcs/odcsapi/res/AppResources.java Show resolved Hide resolved
opendcs-rest-api/src/main/java/org/opendcs/odcsapi/res/AppResources.java Outdated Show resolved Hide resolved
opendcs-rest-api/src/main/java/org/opendcs/odcsapi/res/AppResources.java
CompAppInfo ret = new CompAppInfo();
if (app.getAppId() != null)
{
ret.setAppId(DbKey.createDbKey(app.getAppId()));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think DbKey.NullKey should be used if the app id is null

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added NullKey as result if app id is null

opendcs-rest-api/src/main/java/org/opendcs/odcsapi/res/AppResources.java Outdated Show resolved Hide resolved
opendcs-rest-api/src/main/java/org/opendcs/odcsapi/res/AppResources.java Outdated Show resolved Hide resolved
opendcs-rest-api/src/main/java/org/opendcs/odcsapi/res/AppResources.java Outdated
ApiLoadingApp loadingApp1 = (ApiLoadingApp)obj;
};

// NOTE: Currently uses available primary key generator to obtain a PID value.
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll look into a better way to do this

opendcs-rest-api/src/main/java/org/opendcs/odcsapi/res/AppResources.java Outdated
// A better solution should be found.
TimeSeriesDb tsdb = getLegacyTimeseriesDB();
KeyGenerator keyGen = tsdb.getKeyGenerator();
DbKey key = keyGen.getKey("HDB_LOADING_APPLICATION", tsdb.getConnection());
Copy link
Contributor

Choose a reason for hiding this comment

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

We aren't implementing HDB so that's not going to be the right key

opendcs-rest-api/src/main/java/org/opendcs/odcsapi/res/AppResources.java Outdated
{
throw new WebAppException(ErrorCodes.DATABASE_ERROR,
String.format("Error attempting to start appId=%s", appId), ex);
}
}

static ApiLoadingApp mapLoading(CompAppInfo app) {
Copy link
Contributor

Choose a reason for hiding this comment

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

formatting: brackets should be on a new line

opendcs-rest-api/src/main/java/org/opendcs/odcsapi/res/AppResources.java Outdated Show resolved Hide resolved
opendcs-rest-api/src/test/java/org/opendcs/odcsapi/res/it/AppResourcesIT.java Outdated
appid = null;
sessionFilter = new SessionFilter();

given()
Copy link
Contributor

Choose a reason for hiding this comment

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

use the same indenting strategy we did for CDA

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated formatting; looks like it was lost when moving to the new branch

@adamkorynta adamkorynta linked an issue Dec 18, 2024 that may be closed by this pull request
OpenTSDB: Create Integration Tests for AppResources controller #220
Open
@zack-rma zack-rma force-pushed the feature/task_cwms_support_app_resources branch from c7c6a24 to 14aa521 Compare December 18, 2024 18:23
zack-rma
zack-rma commented Dec 18, 2024
View reviewed changes
Copy link
Collaborator Author

@zack-rma zack-rma left a comment

Choose a reason for hiding this comment

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

Made some updates to formatting

opendcs-rest-api/src/main/java/org/opendcs/odcsapi/res/AppResources.java Show resolved Hide resolved
opendcs-rest-api/src/main/java/org/opendcs/odcsapi/res/AppResources.java
CompAppInfo ret = new CompAppInfo();
if (app.getAppId() != null)
{
ret.setAppId(DbKey.createDbKey(app.getAppId()));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added NullKey as result if app id is null

opendcs-rest-api/src/test/java/org/opendcs/odcsapi/res/it/AppResourcesIT.java Outdated
appid = null;
sessionFilter = new SessionFilter();

given()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated formatting; looks like it was lost when moving to the new branch

opendcs-rest-api/src/main/java/org/opendcs/odcsapi/res/AppResources.java Outdated
ApiLoadingApp loadingApp1 = (ApiLoadingApp)obj;
};

// NOTE: Currently uses available primary key generator to obtain a PID value.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll look into a better way to do this

@zack-rma zack-rma mentioned this pull request Dec 18, 2024
Closed
6 tasks
@zack-rma zack-rma mentioned this pull request Dec 30, 2024
Open
6 tasks
@zack-rma zack-rma requested a review from rma-rripken January 1, 2025 00:34
@zack-rma zack-rma force-pushed the feature/task_cwms_support_app_resources branch from 27e5c29 to 0e3eb53 Compare January 3, 2025 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamkorynta adamkorynta adamkorynta requested changes

@rma-rripken rma-rripken Awaiting requested review from rma-rripken

Assignees
No one assigned
Labels
None yet
Projects
W912HQ24F0215 Task 2: OpenDCS REST API Expanded to CWMS DB
Status: In review
Milestone
F0215 Task 2.1 - Refactor controllers
Development

Successfully merging this pull request may close these issues.

OpenTSDB: Create Integration Tests for AppResources controller Refactor AppResources controller
2 participants
@zack-rma @adamkorynta