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.

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

Add support to connect to the LoginFlow AI service #5971

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

sahandilshan
Copy link
Contributor

Proposed changes in this pull request

With this PR, the product-is will be able to connect the loginflow AI microservice and generate AI results accordingly

Related Issue(s)

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link

sonarcloud bot commented Oct 18, 2024

@@ -0,0 +1,23 @@
package org.wso2.carbon.identity.application.mgt.ai;
Copy link
Contributor

Choose a reason for hiding this comment

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

missing copywrite text

<module>org.wso2.carbon.ai.service.mgt.server.feature</module>
</modules>

</project>
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing new line


<artifactId>ai-services-mgt-feature</artifactId>
<packaging>pom</packaging>
<name>WSO2 Carbon - API Resource Management Feature</name>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this name correct

<version>${maven.surefire.plugin.version}</version>
<configuration>
<!--suppress UnresolvedMavenProperty -->
<argLine>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why these are added?

Copy link

codecov bot commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 50.84746% with 116 lines in your changes missing coverage. Please review.

Project coverage is 40.19%. Comparing base (1608510) to head (ddfaf3c).
Report is 39 commits behind head on master.

Files with missing lines Patch % Lines
...2/carbon/ai/service/mgt/util/AIHttpClientUtil.java 49.29% 30 Missing and 6 partials ⚠️
...bon/ai/service/mgt/token/AIAccessTokenManager.java 71.26% 20 Missing and 5 partials ⚠️
...ity/application/mgt/ai/LoginFlowAIManagerImpl.java 0.00% 19 Missing ⚠️
...lication/mgt/ai/constant/LoginFlowAIConstants.java 0.00% 17 Missing ⚠️
...n/ai/service/mgt/exceptions/AIClientException.java 33.33% 8 Missing ⚠️
...n/ai/service/mgt/exceptions/AIServerException.java 50.00% 7 Missing ⚠️
...2/carbon/ai/service/mgt/constants/AIConstants.java 80.00% 3 Missing ⚠️
...nternal/ApplicationManagementServiceComponent.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5971      +/-   ##
============================================
- Coverage     40.33%   40.19%   -0.15%     
+ Complexity    14346    14217     -129     
============================================
  Files          1739     1747       +8     
  Lines        119425   117361    -2064     
  Branches      20773    20168     -605     
============================================
- Hits          48167    47170     -997     
+ Misses        63917    62957     -960     
+ Partials       7341     7234     -107     
Flag Coverage Δ
unit 24.04% <50.84%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

</plugins>
</build>

</project>
Copy link
Contributor

Choose a reason for hiding this comment

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

missing new line


@Test
public void testGetAuthenticationSequenceGenerationStatus_Success() throws Exception {
Map<String, Object> response = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

missing new line

import static org.wso2.carbon.base.MultitenantConstants.SUPER_TENANT_DOMAIN_NAME;
import static org.wso2.carbon.base.MultitenantConstants.SUPER_TENANT_ID;

public class LoginFlowAIManagerTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Class description missing

SERVER_ERROR_WHILE_CONNECTING_TO_LOGINFLOW_AI_SERVICE("AILF_10009", "Server error occurred " +
"for %s tenant while generating authentication sequence.");


Copy link
Contributor

Choose a reason for hiding this comment

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

extra new line

<limit implementation="org.jacoco.report.check.Limit">
<counter>COMPLEXITY</counter>
<value>COVEREDRATIO</value>
<minimum>0.50</minimum>
Copy link
Contributor

Choose a reason for hiding this comment

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

This test coverage threshold is not enough.

Comment on lines +31 to +32
MAXIMUM_RETRIES_EXCEEDED("AI_10000", "Maximum retries exceeded to retrieve the access token."),
UNABLE_TO_ACCESS_AI_SERVICE_WITH_RENEW_ACCESS_TOKEN("AI_10003", "Unable to access the " +
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make the error codes in consistent oreder

"for %s tenant while connecting to AI service."),
SERVER_ERROR_WHILE_CONNECTING_TO_AI_SERVICE("AI_10009", "Server error occurred " +
"for %s tenant while connecting to AI service.");

Copy link
Contributor

Choose a reason for hiding this comment

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

extra new line

public void completed(HttpResponse response) {

try {
if (response.getStatusLine().getStatusCode() == HttpStatus.SC_OK) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this line null safe?

}
}
} catch (IOException e) {
throw new AIServerException("Failed to close HTTP client: " + e.getMessage(), e);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this error message correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants