From 4581238cf32ac1a45208b809f01b34c4e56113d7 Mon Sep 17 00:00:00 2001 From: lasanthaDLPDS Date: Tue, 18 Jun 2019 13:15:41 +0530 Subject: [PATCH 1/6] Make banner uploading as optional. --- .../mgt/core/impl/ApplicationManagerImpl.java | 6 ------ .../application/mgt/core/util/APIUtil.java | 5 ++++- .../ApplicationManagementPublisherAPI.java | 18 ++++++----------- ...ApplicationManagementPublisherAPIImpl.java | 20 ++++++++----------- .../impl/ApplicationManagementAPIImpl.java | 4 ---- .../dbscripts/cdm/application-mgt/h2.sql | 2 +- 6 files changed, 19 insertions(+), 36 deletions(-) diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java index 3bd1708a74..77190def2f 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java @@ -2559,12 +2559,6 @@ public class ApplicationManagerImpl implements ApplicationManager { log.error(msg); throw new RequestValidatingException(msg); } - //todo remove this check, because banner is not mandatory to have - if (bannerFile == null) { - String msg = "Banner file is not found with the application release creating request."; - log.error(msg); - throw new RequestValidatingException(msg); - } if (attachmentList == null || attachmentList.isEmpty()) { String msg = "Screenshots are not found with the application release creating request."; log.error(msg); diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/util/APIUtil.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/util/APIUtil.java index d53fb5d2d8..2458db3a30 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/util/APIUtil.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/util/APIUtil.java @@ -355,7 +355,10 @@ public class APIUtil { applicationRelease.setSupportedOsVersions(applicationReleaseDTO.getSupportedOsVersions()); applicationRelease.setRating(applicationReleaseDTO.getRating()); applicationRelease.setIconPath(basePath + applicationReleaseDTO.getIconName()); - applicationRelease.setBannerPath(basePath + applicationReleaseDTO.getBannerName()); + + if (!StringUtils.isEmpty(applicationReleaseDTO.getBannerName())){ + applicationRelease.setBannerPath(basePath + applicationReleaseDTO.getBannerName()); + } if (urlValidator.isValid(applicationReleaseDTO.getInstallerName())){ applicationRelease diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/ApplicationManagementPublisherAPI.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/ApplicationManagementPublisherAPI.java index 5efd2bec44..e2b50661e2 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/ApplicationManagementPublisherAPI.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/ApplicationManagementPublisherAPI.java @@ -320,8 +320,7 @@ public interface ApplicationManagementPublisherAPI { @Multipart(value = "icon") Attachment iconFile, @ApiParam( name = "banner", - value = "Banner of the uploading application", - required = true) + value = "Banner of the uploading application") @Multipart(value = "banner") Attachment bannerFile, @ApiParam( name = "screenshot1", @@ -385,8 +384,7 @@ public interface ApplicationManagementPublisherAPI { @Multipart(value = "icon") Attachment iconFile, @ApiParam( name = "banner", - value = "Banner of the uploading web app", - required = true) + value = "Banner of the uploading web app") @Multipart(value = "banner") Attachment bannerFile, @ApiParam( name = "screenshot1", @@ -450,8 +448,7 @@ public interface ApplicationManagementPublisherAPI { @Multipart(value = "icon") Attachment iconFile, @ApiParam( name = "banner", - value = "Banner of the uploading public app", - required = true) + value = "Banner of the uploading public app") @Multipart(value = "banner") Attachment bannerFile, @ApiParam( name = "screenshot1", @@ -523,8 +520,7 @@ public interface ApplicationManagementPublisherAPI { @Multipart(value = "icon") Attachment iconFile, @ApiParam( name = "banner", - value = "Banner of the uploading application", - required = true) + value = "Banner of the uploading application") @Multipart(value = "banner") Attachment bannerFile, @ApiParam( name = "screenshot1", @@ -635,8 +631,7 @@ public interface ApplicationManagementPublisherAPI { @Multipart(value = "icon") Attachment iconFile, @ApiParam( name = "banner", - value = "Banner of the uploading application", - required = true) + value = "Banner of the uploading application") @Multipart(value = "banner") Attachment bannerFile, @ApiParam( name = "screenshot1", @@ -776,8 +771,7 @@ public interface ApplicationManagementPublisherAPI { @Multipart(value = "icon") Attachment iconFile, @ApiParam( name = "banner", - value = "banner file of the application release.", - required = true) + value = "banner file of the application release.") @Multipart(value = "banner") Attachment bannerFile, @ApiParam( name = "screenshot1", diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/impl/ApplicationManagementPublisherAPIImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/impl/ApplicationManagementPublisherAPIImpl.java index a9c639b2ad..e5e6745dff 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/impl/ApplicationManagementPublisherAPIImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/impl/ApplicationManagementPublisherAPIImpl.java @@ -46,9 +46,9 @@ import org.wso2.carbon.device.application.mgt.core.exception.NotFoundException; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.TreeMap; import javax.activation.DataHandler; import javax.validation.Valid; import javax.ws.rs.Consumes; @@ -85,10 +85,6 @@ public class ApplicationManagementPublisherAPIImpl implements ApplicationManagem return Response.status(Response.Status.BAD_REQUEST).entity(msg).build(); } ApplicationList applications = applicationManager.getApplications(filter); - if (applications.getApplications().isEmpty()) { - return Response.status(Response.Status.OK) - .entity("Couldn't find any application for the requested query.").build(); - } return Response.status(Response.Status.OK).entity(applications).build(); } catch (BadRequestException e) { String msg = "Incompatible request payload is found. Please try with valid request payload."; @@ -175,7 +171,7 @@ public class ApplicationManagementPublisherAPIImpl implements ApplicationManagem @Multipart("application") ApplicationWrapper applicationWrapper, @Multipart("binaryFile") Attachment binaryFile, @Multipart("icon") Attachment iconFile, - @Multipart("banner") Attachment bannerFile, + @Multipart(value = "banner", required = false) Attachment bannerFile, @Multipart("screenshot1") Attachment screenshot1, @Multipart("screenshot2") Attachment screenshot2, @Multipart("screenshot3") Attachment screenshot3) { @@ -214,7 +210,7 @@ public class ApplicationManagementPublisherAPIImpl implements ApplicationManagem public Response createWebApp( @Multipart("webapp") WebAppWrapper webAppWrapper, @Multipart("icon") Attachment iconFile, - @Multipart("banner") Attachment bannerFile, + @Multipart(value = "banner", required = false) Attachment bannerFile, @Multipart("screenshot1") Attachment screenshot1, @Multipart("screenshot2") Attachment screenshot2, @Multipart("screenshot3") Attachment screenshot3) { @@ -252,7 +248,7 @@ public class ApplicationManagementPublisherAPIImpl implements ApplicationManagem public Response createPubApp( @Multipart("public-app") PublicAppWrapper publicAppWrapper, @Multipart("icon") Attachment iconFile, - @Multipart("banner") Attachment bannerFile, + @Multipart(value = "banner", required = false) Attachment bannerFile, @Multipart("screenshot1") Attachment screenshot1, @Multipart("screenshot2") Attachment screenshot2, @Multipart("screenshot3") Attachment screenshot3) { @@ -292,7 +288,7 @@ public class ApplicationManagementPublisherAPIImpl implements ApplicationManagem @Multipart("applicationRelease") ApplicationReleaseWrapper applicationReleaseWrapper, @Multipart("binaryFile") Attachment binaryFile, @Multipart("icon") Attachment iconFile, - @Multipart("banner") Attachment bannerFile, + @Multipart(value = "banner", required = false) Attachment bannerFile, @Multipart("screenshot1") Attachment screenshot1, @Multipart("screenshot2") Attachment screenshot2, @Multipart("screenshot3") Attachment screenshot3) { @@ -331,7 +327,7 @@ public class ApplicationManagementPublisherAPIImpl implements ApplicationManagem public Response updateApplicationImageArtifacts( @PathParam("uuid") String applicationReleaseUuid, @Multipart("icon") Attachment iconFile, - @Multipart("banner") Attachment bannerFile, + @Multipart(value = "banner", required = false) Attachment bannerFile, @Multipart("screenshot1") Attachment screenshot1, @Multipart("screenshot2") Attachment screenshot2, @Multipart("screenshot3") Attachment screenshot3) { @@ -436,7 +432,7 @@ public class ApplicationManagementPublisherAPIImpl implements ApplicationManagem @Multipart("applicationRelease") ApplicationReleaseWrapper applicationReleaseWrapper, @Multipart("binaryFile") Attachment binaryFile, @Multipart("icon") Attachment iconFile, - @Multipart("banner") Attachment bannerFile, + @Multipart(value = "banner", required = false) Attachment bannerFile, @Multipart("screenshot1") Attachment screenshot1, @Multipart("screenshot2") Attachment screenshot2, @Multipart("screenshot3") Attachment screenshot3) { @@ -825,7 +821,7 @@ public class ApplicationManagementPublisherAPIImpl implements ApplicationManagem } if (attachmentList != null) { - Map scrrenshotData = new HashMap<>(); + Map scrrenshotData = new TreeMap<>(); for (Attachment sc : attachmentList) { dataHandler = sc.getDataHandler(); String screenshotrFileName = dataHandler.getName(); diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.store.api/src/main/java/org/wso2/carbon/device/application/mgt/store/api/services/impl/ApplicationManagementAPIImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.store.api/src/main/java/org/wso2/carbon/device/application/mgt/store/api/services/impl/ApplicationManagementAPIImpl.java index accda96f57..754bd7a352 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.store.api/src/main/java/org/wso2/carbon/device/application/mgt/store/api/services/impl/ApplicationManagementAPIImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.store.api/src/main/java/org/wso2/carbon/device/application/mgt/store/api/services/impl/ApplicationManagementAPIImpl.java @@ -60,10 +60,6 @@ public class ApplicationManagementAPIImpl implements ApplicationManagementAPI { } filter.setAppReleaseState(applicationManager.getInstallableLifecycleState()); ApplicationList applications = applicationManager.getApplications(filter); - if (applications.getApplications().isEmpty()) { - return Response.status(Response.Status.OK) - .entity("Couldn't find any application for the requested query.").build(); - } return Response.status(Response.Status.OK).entity(applications).build(); } catch (BadRequestException e) { String msg = e.getMessage(); diff --git a/features/application-mgt/org.wso2.carbon.device.application.mgt.server.feature/src/main/resources/dbscripts/cdm/application-mgt/h2.sql b/features/application-mgt/org.wso2.carbon.device.application.mgt.server.feature/src/main/resources/dbscripts/cdm/application-mgt/h2.sql index 180aa9fcd9..c2d8157505 100644 --- a/features/application-mgt/org.wso2.carbon.device.application.mgt.server.feature/src/main/resources/dbscripts/cdm/application-mgt/h2.sql +++ b/features/application-mgt/org.wso2.carbon.device.application.mgt.server.feature/src/main/resources/dbscripts/cdm/application-mgt/h2.sql @@ -29,7 +29,7 @@ CREATE TABLE IF NOT EXISTS AP_APP_RELEASE( APP_PRICE DECIMAL(6, 2) NULL DEFAULT NULL, INSTALLER_LOCATION VARCHAR(100) NOT NULL, ICON_LOCATION VARCHAR(100) NOT NULL, - BANNER_LOCATION VARCHAR(100) NOT NULL, + BANNER_LOCATION VARCHAR(100) NULL DEFAULT NULL, SC_1_LOCATION VARCHAR(100) NOT NULL, SC_2_LOCATION VARCHAR(100) NULL DEFAULT NULL, SC_3_LOCATION VARCHAR(100) NULL DEFAULT NULL, From 6c0bc6238023391df35fa3fd9d3f95f368a783e5 Mon Sep 17 00:00:00 2001 From: lasanthaDLPDS Date: Wed, 19 Jun 2019 14:38:34 +0530 Subject: [PATCH 2/6] Fix review add DAO issue --- .../application/mgt/core/dao/impl/review/ReviewDAOImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/review/ReviewDAOImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/review/ReviewDAOImpl.java index e1864ecfbb..df6bf92627 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/review/ReviewDAOImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/review/ReviewDAOImpl.java @@ -110,7 +110,7 @@ public class ReviewDAOImpl extends AbstractDAOImpl implements ReviewDAO { try { conn = this.getDBConnection(); StringJoiner joiner = new StringJoiner(",", - "SELECT rv.ID FROM AP_APP_REVIEW " + "WHERE rv.AP_APP_RELEASE_ID IN (", + "SELECT rv.ID FROM AP_APP_REVIEW rv " + "WHERE rv.AP_APP_RELEASE_ID IN (", ") AND rv.USERNAME = ? AND rv.TENANT_ID = ?"); appReleaseIds.stream().map(ignored -> "?").forEach(joiner::add); String query = joiner.toString(); From 7eac984e0c51edb5755c721afa974821ace69840 Mon Sep 17 00:00:00 2001 From: lasanthaDLPDS Date: Wed, 19 Jun 2019 19:49:41 +0530 Subject: [PATCH 3/6] Improve app getting functionality in APPM publisher --- .../common/services/ApplicationManager.java | 2 +- .../application/mgt/core/dao/ReviewDAO.java | 2 +- .../core/dao/impl/review/ReviewDAOImpl.java | 3 +- .../mgt/core/impl/ApplicationManagerImpl.java | 45 ++++++++++--------- .../mgt/core/impl/ReviewManagerImpl.java | 3 +- .../ApplicationManagementPublisherAPI.java | 2 +- ...ApplicationManagementPublisherAPIImpl.java | 9 ++-- 7 files changed, 33 insertions(+), 33 deletions(-) diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/services/ApplicationManager.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/services/ApplicationManager.java index 3bbf1762f2..90f65a44a5 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/services/ApplicationManager.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/services/ApplicationManager.java @@ -122,7 +122,7 @@ public interface ApplicationManager { * @return the Application Release identified by the UUID * @throws ApplicationManagementException Application Management Exception. */ - ApplicationRelease getApplicationReleaseByUUID(String uuid) throws ApplicationManagementException; + Application getApplicationByUuid(String uuid) throws ApplicationManagementException; /** * To get the ApplicationDTO for given application relase UUID. diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/ReviewDAO.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/ReviewDAO.java index 2a508b8e31..b7d099423e 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/ReviewDAO.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/ReviewDAO.java @@ -90,7 +90,7 @@ import java.util.List; * @return {@link List}List of all reviews for the application release * @throws ReviewManagementDAOException Review management DAO exception **/ - List getAllActiveReleaseReviews(int releaseId, PaginationRequest request, int tenantId) + List getAllReleaseReviews(int releaseId, PaginationRequest request, int tenantId) throws ReviewManagementDAOException; List getAllActiveAppReviews(List releaseIds, PaginationRequest request, int tenantId) diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/review/ReviewDAOImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/review/ReviewDAOImpl.java index df6bf92627..b57486690a 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/review/ReviewDAOImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/review/ReviewDAOImpl.java @@ -258,7 +258,7 @@ public class ReviewDAOImpl extends AbstractDAOImpl implements ReviewDAO { @Override - public List getAllActiveReleaseReviews(int releaseId, PaginationRequest request, int tenantId) + public List getAllReleaseReviews(int releaseId, PaginationRequest request, int tenantId) throws ReviewManagementDAOException { if (log.isDebugEnabled()) { @@ -283,7 +283,6 @@ public class ReviewDAOImpl extends AbstractDAOImpl implements ReviewDAO { + "AP_APP_REVIEW.AP_APP_RELEASE_ID = AP_APP_RELEASE.ID " + "WHERE AP_APP_REVIEW.AP_APP_RELEASE_ID = ? AND " + "AP_APP_REVIEW.ROOT_PARENT_ID = ? AND " - + "AP_APP_REVIEW.ACTIVE_REVIEW = true AND " + "AP_APP_REVIEW.TENANT_ID = ? " + "LIMIT ? OFFSET ?"; try (PreparedStatement statement = conn.prepareStatement(sql)) { diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java index 77190def2f..dab6fdf57f 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java @@ -896,49 +896,50 @@ public class ApplicationManagerImpl implements ApplicationManager { } @Override - public ApplicationRelease getApplicationReleaseByUUID(String uuid) throws ApplicationManagementException{ + public Application getApplicationByUuid(String uuid) throws ApplicationManagementException{ int tenantId = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantId(true); String userName = PrivilegedCarbonContext.getThreadLocalCarbonContext().getUsername(); - boolean isVisibleAppRelease = false; + boolean isVisibleApp = false; + try { ConnectionManagerUtil.openDBConnection(); - ApplicationReleaseDTO applicationReleaseDTO = applicationReleaseDAO.getReleaseByUUID(uuid, tenantId); - if (applicationReleaseDTO == null) { - String msg = "Couldn't find an application release for the UUID: " + uuid; + ApplicationDTO applicationDTO = applicationDAO.getApplicationByUUID(uuid, tenantId); + + if (applicationDTO == null) { + String msg = "Couldn't found an application for application release UUID: " + uuid; log.error(msg); throw new NotFoundException(msg); } - if (applicationReleaseDTO.getCurrentState().equals(lifecycleStateManager.getEndState())) { - return null; - } - List unrestrictedRoles = this.visibilityDAO.getUnrestrictedRolesByUUID(uuid, tenantId); + List tags = this.applicationDAO.getAppTags(applicationDTO.getId(), tenantId); + List categories = this.applicationDAO.getAppCategories(applicationDTO.getId(), tenantId); + applicationDTO.setTags(tags); + applicationDTO.setAppCategories(categories); + + List unrestrictedRoles = this.visibilityDAO.getUnrestrictedRoles(applicationDTO.getId(), tenantId); if (!unrestrictedRoles.isEmpty()) { if (hasUserRole(unrestrictedRoles, userName)) { - isVisibleAppRelease = true; + isVisibleApp = true; } } else { - isVisibleAppRelease = true; + isVisibleApp = true; } - if (!isVisibleAppRelease) { - String msg = "You are trying to access release of visibility restricted application. You don't have " - + "required roles to view this application,"; + if (!isVisibleApp) { + String msg = "You are trying to access visibility restricted application. You don't have required " + + "roles to view this application,"; log.error(msg); throw new ForbiddenException(msg); } - return APIUtil.releaseDtoToRelease(applicationReleaseDTO); - } catch (LifecycleManagementException e) { - String msg = "Error occurred when getting the end state of the application lifecycle flow"; - log.error(msg); - throw new ApplicationManagementException(msg, e); + return APIUtil.appDtoToAppResponse(applicationDTO); } catch (UserStoreException e) { - String msg = "User-store exception while getting application with the application release UUID: " + uuid; + String msg = "User-store exception occurred while getting application for application release UUID " + uuid; log.error(msg); throw new ApplicationManagementException(msg, e); } catch (ApplicationManagementDAOException e) { - //todo - throw new ApplicationManagementException(""); + String msg = "Error occurred while getting dta which are related to Application."; + log.error(msg); + throw new ApplicationManagementException(msg); } finally { ConnectionManagerUtil.closeDBConnection(); } diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java index b550f6a64e..b31479c4ed 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java @@ -289,6 +289,7 @@ public class ReviewManagerImpl implements ReviewManager { log.error(msg); throw new ReviewManagementException(msg); } + uuid = reviewDTO.getReleaseUuid(); } else if (updatingReview.getRating() > 0 && updatingReview.getRating() != reviewDTO.getRating()) { Runnable task = () -> ReviewManagerImpl.this .calculateRating(updatingReview.getRating(), reviewDTO.getRating(), uuid, tenantId); @@ -353,7 +354,7 @@ public class ReviewManagerImpl implements ReviewManager { log.error(msg); throw new NotFoundException(msg); } - return getReviewTree(this.reviewDAO.getAllActiveReleaseReviews(releaseDTO.getId(), request, tenantId)); + return getReviewTree(this.reviewDAO.getAllReleaseReviews(releaseDTO.getId(), request, tenantId)); } catch (ReviewManagementDAOException e) { throw new ReviewManagementException("Error occured while getting all reviews for application uuid: " + uuid, e); diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/ApplicationManagementPublisherAPI.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/ApplicationManagementPublisherAPI.java index e2b50661e2..99355e5c96 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/ApplicationManagementPublisherAPI.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/ApplicationManagementPublisherAPI.java @@ -217,7 +217,7 @@ public interface ApplicationManagementPublisherAPI { message = "Internal Server Error. \n Error occurred while getting relevant application release.", response = ErrorResponse.class) }) - Response getApplicationRelease( + Response getApplicationByUUID( @ApiParam( name = "uuid", value = "application release uuid", diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/impl/ApplicationManagementPublisherAPIImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/impl/ApplicationManagementPublisherAPIImpl.java index e5e6745dff..d0d091df03 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/impl/ApplicationManagementPublisherAPIImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.publisher.api/src/main/java/org/wso2/carbon/device/application/mgt/publisher/api/services/impl/ApplicationManagementPublisherAPIImpl.java @@ -135,18 +135,17 @@ public class ApplicationManagementPublisherAPIImpl implements ApplicationManagem @GET @Consumes("application/json") @Path("/release/{uuid}") - public Response getApplicationRelease( + public Response getApplicationByUUID( @PathParam("uuid") String uuid) { ApplicationManager applicationManager = APIUtil.getApplicationManager(); try { - //todo return application - ApplicationRelease applicationRelease = applicationManager.getApplicationReleaseByUUID(uuid); - if (applicationRelease == null){ + Application application = applicationManager.getApplicationByUuid(uuid); + if (application == null){ String msg = "Application release is in the end state of the application lifecycle flow."; log.error(msg); return Response.status(Response.Status.OK).entity(msg).build(); } - return Response.status(Response.Status.OK).entity(applicationRelease).build(); + return Response.status(Response.Status.OK).entity(application).build(); } catch (NotFoundException e) { String msg = "Application Release with UUID: " + uuid + " is not found"; log.error(msg, e); From 6cc9d816c39b80bf3cbd6ce1d39e474d1b395fe0 Mon Sep 17 00:00:00 2001 From: lasanthaDLPDS Date: Wed, 19 Jun 2019 19:55:11 +0530 Subject: [PATCH 4/6] Fix minor issue in review management --- .../device/application/mgt/core/impl/ReviewManagerImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java index b31479c4ed..6b605ef9de 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java @@ -289,7 +289,6 @@ public class ReviewManagerImpl implements ReviewManager { log.error(msg); throw new ReviewManagementException(msg); } - uuid = reviewDTO.getReleaseUuid(); } else if (updatingReview.getRating() > 0 && updatingReview.getRating() != reviewDTO.getRating()) { Runnable task = () -> ReviewManagerImpl.this .calculateRating(updatingReview.getRating(), reviewDTO.getRating(), uuid, tenantId); From 34a14aa9e0884a23a1a4a41d66bd1a305ed282a2 Mon Sep 17 00:00:00 2001 From: lasanthaDLPDS Date: Wed, 19 Jun 2019 22:07:06 +0530 Subject: [PATCH 5/6] Improve review management DAO --- .../mgt/common/response/Review.java | 36 ++++++++----------- .../core/dao/impl/review/ReviewDAOImpl.java | 4 +-- .../mgt/core/impl/ReviewManagerImpl.java | 4 +-- 3 files changed, 18 insertions(+), 26 deletions(-) diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/response/Review.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/response/Review.java index e77bbacc06..1c0285fbb4 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/response/Review.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/response/Review.java @@ -32,12 +32,6 @@ public class Review { @ApiModelProperty(name = "content", value = "Review message.") private String content; - @ApiModelProperty(name = "rootParentId", value = "Root Parent id of the review") - private int rootParentId; - - @ApiModelProperty(name = "immediateParentId", value = "Immediate Parent id of the review") - private int immediateParentId; - @ApiModelProperty(name = "username", value = "Username odf the Review creator") private String username; @@ -50,6 +44,12 @@ public class Review { @ApiModelProperty(name = "rating", value = "Rating value of the application release") private int rating; + @ApiModelProperty(name = "releaseUuid", value = "UUID of the review associated application") + private String releaseUuid; + + @ApiModelProperty(name = "releaseVersion", value = "Version of the review associated application") + private String releaseVersion; + @ApiModelProperty(name = "replies", value = "Replying reviews") private List replies; @@ -69,22 +69,6 @@ public class Review { this.content = content; } - public int getRootParentId() { - return rootParentId; - } - - public void setRootParentId(int rootParentId) { - this.rootParentId = rootParentId; - } - - public int getImmediateParentId() { - return immediateParentId; - } - - public void setImmediateParentId(int immediateParentId) { - this.immediateParentId = immediateParentId; - } - public String getUsername() { return username; } @@ -120,4 +104,12 @@ public class Review { public List getReplies() { return replies; } public void setReplies(List replies) { this.replies = replies; } + + public String getReleaseUuid() { return releaseUuid; } + + public void setReleaseUuid(String releaseUuid) { this.releaseUuid = releaseUuid; } + + public String getReleaseVersion() { return releaseVersion; } + + public void setReleaseVersion(String releaseVersion) { this.releaseVersion = releaseVersion; } } diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/review/ReviewDAOImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/review/ReviewDAOImpl.java index b57486690a..73659390cc 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/review/ReviewDAOImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/review/ReviewDAOImpl.java @@ -110,7 +110,7 @@ public class ReviewDAOImpl extends AbstractDAOImpl implements ReviewDAO { try { conn = this.getDBConnection(); StringJoiner joiner = new StringJoiner(",", - "SELECT rv.ID FROM AP_APP_REVIEW rv " + "WHERE rv.AP_APP_RELEASE_ID IN (", + "SELECT rv.ID FROM AP_APP_REVIEW rv WHERE rv.AP_APP_RELEASE_ID IN (", ") AND rv.USERNAME = ? AND rv.TENANT_ID = ?"); appReleaseIds.stream().map(ignored -> "?").forEach(joiner::add); String query = joiner.toString(); @@ -118,7 +118,7 @@ public class ReviewDAOImpl extends AbstractDAOImpl implements ReviewDAO { for (Integer deviceId : appReleaseIds) { ps.setObject(index++, deviceId); } - ps.setInt(index++, tenantId); + ps.setString(index++, username); ps.setInt(index, tenantId); try (ResultSet rs = ps.executeQuery()) { return rs.next(); diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java index 6b605ef9de..01f384d288 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java @@ -238,8 +238,8 @@ public class ReviewManagerImpl implements ReviewManager { Review review = new Review(); review.setId(reviewDTO.getId()); review.setContent(reviewDTO.getContent()); - review.setRootParentId(reviewDTO.getRootParentId()); - review.setImmediateParentId(reviewDTO.getImmediateParentId()); + review.setReleaseUuid(reviewDTO.getReleaseUuid()); + review.setReleaseVersion(reviewDTO.getReleaseVersion()); review.setCreatedAt(reviewDTO.getCreatedAt()); review.setModifiedAt(reviewDTO.getModifiedAt()); review.setRating(reviewDTO.getRating()); From 756999a1e7fc108e450afb65f24ed6360f68b55d Mon Sep 17 00:00:00 2001 From: lasanthaDLPDS Date: Thu, 20 Jun 2019 00:56:24 +0530 Subject: [PATCH 6/6] Fix APPM deadlock issue in review management --- .../mgt/core/impl/ReviewManagerImpl.java | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java index 01f384d288..48260b269a 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java @@ -284,11 +284,6 @@ public class ReviewManagerImpl implements ReviewManager { if (reviewDTO.getRootParentId() == -1 && reviewDTO.getImmediateParentId() == -1) { if (!reviewDTO.getReleaseUuid().equals(uuid)) { isActiveReview = false; - if (!addReview(updatingReview, uuid, true)) { - String msg = "Review Updating Status: New review adding is failed."; - log.error(msg); - throw new ReviewManagementException(msg); - } } else if (updatingReview.getRating() > 0 && updatingReview.getRating() != reviewDTO.getRating()) { Runnable task = () -> ReviewManagerImpl.this .calculateRating(updatingReview.getRating(), reviewDTO.getRating(), uuid, tenantId); @@ -304,17 +299,35 @@ public class ReviewManagerImpl implements ReviewManager { } reviewDTO.setContent(updatingReview.getContent()); } - return updateReviewInDB(reviewDTO, uuid, reviewId, isActiveReview, tenantId); + + if (updateReviewInDB(reviewDTO, reviewId, isActiveReview, tenantId)) { + if (!isActiveReview) { + if (addReview(updatingReview, uuid, true)) { + return true; + } else { + if (updateReviewInDB(reviewDTO, reviewId, true, tenantId)) { + return false; + } else { + String msg = "Review Updating Status: Adding new Review for application release which has UUID: " + + "" + uuid + " is failed and the old review restoring is also failed."; + log.error(msg); + throw new ApplicationManagementException(msg); + } + } + } + return true; + } else { + String msg = "Review Updating is failed. Hence please contact the administrator."; + log.error(msg); + throw new ApplicationManagementException(msg); + } } - private boolean updateReviewInDB(ReviewDTO reviewDTO, String uuid, int reviewId, boolean isActiveReview, + private boolean updateReviewInDB(ReviewDTO reviewDTO, int reviewId, boolean isActiveReview, int tenantId) throws ReviewManagementException, ApplicationManagementException { try { ConnectionManagerUtil.beginDBTransaction(); if (this.reviewDAO.updateReview(reviewDTO, reviewId, isActiveReview, tenantId) == 1) { - if (!isActiveReview) { - updateAppRating(uuid, tenantId); - } ConnectionManagerUtil.commitDBTransaction(); return true; }