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
[AIRAVATA-2385] Checking for duplicate gateway request #111
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this PR Sneha! Please see my comments on the PR.
@@ -72,7 +72,9 @@ public String getAPIVersion(AuthzToken authzToken) throws TenantProfileServiceEx | |||
@SecurityCheck | |||
public String addGateway(AuthzToken authzToken, Gateway gateway) throws TenantProfileServiceException, AuthorizationException, TException { | |||
try { | |||
gateway = tenantProfileRepository.create(gateway); | |||
if (!checkDuplicate(gateway)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is a duplicate then I think the code should throw a TenantProfileServiceException with an error message explaining that there is a duplicate.
@@ -183,12 +185,25 @@ public boolean deleteGateway(AuthzToken authzToken, String gatewayId) throws Ten | |||
public boolean isGatewayExist(AuthzToken authzToken, String gatewayId) throws TenantProfileServiceException, AuthorizationException, TException { | |||
try { | |||
Gateway gateway = tenantProfileRepository.getGateway(gatewayId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work as is. getGateway() should query by gatewayId where status is APPROVED. This way if there is an APPROVED gateway with the gatewayId getGateway() should return it. That means too that the checks on lines 188-189 are unnecessary.
|
||
private boolean checkDuplicate(Gateway gateway) throws TenantProfileServiceException { | ||
try { | ||
Gateway duplicateGateway = tenantProfileRepository.getGateway(gateway.getGatewayId()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same concern here as above. This will work only if getGateway returns an APPROVED gateway with this gatewayId.
private boolean checkDuplicate(Gateway gateway) throws TenantProfileServiceException { | ||
try { | ||
Gateway duplicateGateway = tenantProfileRepository.getGateway(gateway.getGatewayId()); | ||
return ((duplicateGateway.getGatewayId() == gateway.getGatewayId()) && (duplicateGateway.getGatewayName() == gateway.getGatewayName()) && (duplicateGateway.getGatewayURL() == gateway.getGatewayURL()) && (duplicateGateway.getGatewayApprovalStatus().equals(GatewayApprovalStatus.APPROVED))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use .equals() to compare strings instead of ==
.
Changes made in the new commit - Added new method checkDuplicateGateway() in TenantProfileServiceHandler.java to identify duplicate requests for Gateway Ids that have already been approved Modified FIND_GATEWAY_BY_ID in QueryConstants.java to return only Approved Gateways Added new method getAllGatewaysForUser() in TenantProfileServiceHandler.java as a Thrift service to display gateways belonging to a particular user Modified workspace_model.thrift to expose the Gateway's airavataInternalId to Update and Delete Gateways and generated the stubs to reflect in Gateway.java |
👍 |
Modified TenantProfileServiceHandler.java to check for a duplicate add gateway request.
Checked if the gateway ID, name, URL was already present in the database with the status as approved.