From 1ea47c83612ce22bf9f3d4270b0dff52f63ebf6e Mon Sep 17 00:00:00 2001 From: lasanthaDLPDS Date: Thu, 20 Sep 2018 11:58:05 +0530 Subject: [PATCH] Improve review mnagement WIP --- .../mgt/core/dao/ApplicationReleaseDAO.java | 1 + .../GenericApplicationDAOImpl.java | 9 +-- .../GenericApplicationReleaseDAOImpl.java | 8 +++ .../application/mgt/store/api/APIUtil.java | 2 +- .../api/services/ReviewManagementAPI.java | 17 ++--- .../impl/ReviewManagementAPIImpl.java | 63 +++++++++---------- .../api/services/ReviewManagementAPITest.java | 8 +-- 7 files changed, 55 insertions(+), 53 deletions(-) diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/ApplicationReleaseDAO.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/ApplicationReleaseDAO.java index e3bcfecad2e..ec573b8d239 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/ApplicationReleaseDAO.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/ApplicationReleaseDAO.java @@ -18,6 +18,7 @@ */ package org.wso2.carbon.device.application.mgt.core.dao; +import org.wso2.carbon.device.application.mgt.common.Application; import org.wso2.carbon.device.application.mgt.common.ApplicationRelease; import org.wso2.carbon.device.application.mgt.common.Rating; import org.wso2.carbon.device.application.mgt.core.exception.ApplicationManagementDAOException; 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/application/GenericApplicationDAOImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/application/GenericApplicationDAOImpl.java index 12c80c4c525..586d6d92f02 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/application/GenericApplicationDAOImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/application/GenericApplicationDAOImpl.java @@ -380,7 +380,6 @@ public class GenericApplicationDAOImpl extends AbstractDAOImpl implements Applic Connection conn; PreparedStatement stmt = null; ResultSet rs = null; - boolean isAppExist = false; try { conn = this.getDBConnection(); String sql = @@ -396,13 +395,7 @@ public class GenericApplicationDAOImpl extends AbstractDAOImpl implements Applic if (log.isDebugEnabled()) { log.debug("Successfully retrieved basic details of the application with the application ID " + appId); } - - if (rs.next()) { - isAppExist = true; - } - - return isAppExist; - + return rs.next(); } catch (SQLException e) { throw new ApplicationManagementDAOException( "Error occurred while getting application details with app ID " + appId + " While executing query ", 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/application/release/GenericApplicationReleaseDAOImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/application/release/GenericApplicationReleaseDAOImpl.java index c86e7f86849..105d72fc252 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/application/release/GenericApplicationReleaseDAOImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/application/release/GenericApplicationReleaseDAOImpl.java @@ -19,12 +19,17 @@ package org.wso2.carbon.device.application.mgt.core.dao.impl.application.release; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.json.JSONException; +import org.wso2.carbon.device.application.mgt.common.Application; import org.wso2.carbon.device.application.mgt.common.ApplicationRelease; import org.wso2.carbon.device.application.mgt.common.Rating; import org.wso2.carbon.device.application.mgt.common.exception.DBConnectionException; import org.wso2.carbon.device.application.mgt.core.dao.ApplicationReleaseDAO; import org.wso2.carbon.device.application.mgt.core.dao.common.Util; import org.wso2.carbon.device.application.mgt.core.dao.impl.AbstractDAOImpl; +import org.wso2.carbon.device.application.mgt.core.dao.impl.application.GenericApplicationDAOImpl; import org.wso2.carbon.device.application.mgt.core.exception.ApplicationManagementDAOException; import java.sql.Connection; @@ -40,6 +45,9 @@ import java.util.List; */ public class GenericApplicationReleaseDAOImpl extends AbstractDAOImpl implements ApplicationReleaseDAO { + private static final Log log = LogFactory.getLog(GenericApplicationReleaseDAOImpl.class); + + /** * To insert the Application Release Details. * 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/APIUtil.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.store.api/src/main/java/org/wso2/carbon/device/application/mgt/store/api/APIUtil.java index b82802b85d1..1811c066d05 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.store.api/src/main/java/org/wso2/carbon/device/application/mgt/store/api/APIUtil.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.store.api/src/main/java/org/wso2/carbon/device/application/mgt/store/api/APIUtil.java @@ -92,7 +92,7 @@ public class APIUtil { * @return ReviewManager instance in the current osgi context. */ - public static ReviewManager getCommentsManager() { + public static ReviewManager getReviewManager() { PrivilegedCarbonContext ctx = PrivilegedCarbonContext.getThreadLocalCarbonContext(); ReviewManager reviewManager = (ReviewManager) ctx.getOSGiService(ReviewManager.class, null); if (reviewManager == null) { 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/ReviewManagementAPI.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/ReviewManagementAPI.java index a86d0d79011..c8cef67b321 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/ReviewManagementAPI.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/ReviewManagementAPI.java @@ -200,11 +200,10 @@ public interface ReviewManagementAPI { name="uuid", value="uuid of the release version of the application", required=true) - @PathParam("uuid") - String uuid); + @PathParam("uuid") String uuid); @PUT - @Path("/{CommentId}") + @Path("/{uuid}/{reviewId}") @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) @ApiOperation( @@ -238,18 +237,22 @@ public interface ReviewManagementAPI { message = "Internal Server Error. \n Error occurred while updating the new review.", response = ErrorResponse.class) }) - Response updateComment( + Response updateReview( @ApiParam( name = "review", value = "The review that need to be updated.", required = true) @Valid Review review, @ApiParam( - name="commentId", + name="uuid", + value = "uuid of the application release", + required = true) + @PathParam("uuid") String uuid, + @ApiParam( + name="reviewId", value = "review id of the updating review.", required = true) - @QueryParam("commentId") - int commentId); + @PathParam("reviewId") int reviewId); @DELETE @Path("/{commentId}") 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/ReviewManagementAPIImpl.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/ReviewManagementAPIImpl.java index 6252ddada73..e188d4861e7 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/ReviewManagementAPIImpl.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/ReviewManagementAPIImpl.java @@ -22,7 +22,6 @@ import io.swagger.annotations.ApiParam; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.wso2.carbon.device.application.mgt.common.Application; -import org.wso2.carbon.device.application.mgt.common.ApplicationRelease; import org.wso2.carbon.device.application.mgt.common.PaginationResult; import org.wso2.carbon.device.application.mgt.common.Rating; import org.wso2.carbon.device.application.mgt.common.Review; @@ -59,7 +58,7 @@ public class ReviewManagementAPIImpl implements ReviewManagementAPI { @PathParam("uuid") String uuid, @QueryParam("offset") int offSet, @QueryParam("limit") int limit) { - ReviewManager reviewManager = APIUtil.getCommentsManager(); + ReviewManager reviewManager = APIUtil.getReviewManager(); PaginationRequest request = new PaginationRequest(offSet, limit); try { PaginationResult paginationResult = reviewManager.getAllReviews(request, uuid); @@ -78,29 +77,34 @@ public class ReviewManagementAPIImpl implements ReviewManagementAPI { public Response addReview( @ApiParam Review review, @PathParam("uuid") String uuid) { - ReviewManager reviewManager = APIUtil.getCommentsManager(); + ReviewManager reviewManager = APIUtil.getReviewManager(); ApplicationManager applicationManager = APIUtil.getApplicationManager(); Application application; - ApplicationRelease applicationRelease; try { application = applicationManager.getApplicationByRelease(uuid); - applicationRelease = applicationManager.getAppReleaseIfExists(application.getId(), uuid); - boolean abc = reviewManager.addReview(review, application.getId(), applicationRelease.getId()); - if (abc) { + if (application.getApplicationReleases().isEmpty()){ + String msg = "Couldn't Found an one application release for the UUID: " + uuid; + return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build(); + } + if (application.getApplicationReleases().size()>1){ + String msg = "Found more than one application release for the UUID: " + uuid; + return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build(); + } + boolean isReviewCreated = reviewManager + .addReview(review, application.getId(), application.getApplicationReleases().get(0).getId()); + if (isReviewCreated) { return Response.status(Response.Status.CREATED).entity(review).build(); } else { - String msg = "Given review is not valid "; + String msg = "Given review is not valid. Please check the review payload."; log.error(msg); - return Response.status(Response.Status.BAD_REQUEST).build(); + return Response.status(Response.Status.BAD_REQUEST).entity(msg).build(); } } catch (ReviewManagementException e) { String msg = "Error occurred while creating the review"; log.error(msg, e); - return Response.status(Response.Status.INTERNAL_SERVER_ERROR) - .entity(msg).build(); + return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build(); } catch (ApplicationManagementException e) { -// todo - log.error(""); + log.error("Error occured while getting the application for application UUID: " + uuid); return Response.status(Response.Status.INTERNAL_SERVER_ERROR) .entity("").build(); } @@ -109,41 +113,35 @@ public class ReviewManagementAPIImpl implements ReviewManagementAPI { @Override @PUT @Consumes("application/json") - @Path("/review/{commentId}") - public Response updateComment( + @Path("/{uuid}/{reviewId}") + public Response updateReview( @ApiParam Review review, - @PathParam("commentId") int commentId) { - - ReviewManager reviewManager = APIUtil.getCommentsManager(); + @PathParam("uuid") String uuid, + @PathParam("reviewId") int reviewId) { + ReviewManager reviewManager = APIUtil.getReviewManager(); try { - if (commentId == 0) { - return Response.status(Response.Status.NOT_FOUND) - .entity("Review not found").build(); - } else if (review == null) { - String msg = "Given review is not valid "; - log.error(msg); - return Response.status(Response.Status.BAD_REQUEST).build(); - } else if (reviewManager.updateReview(review, commentId, true)) { + if (reviewManager.updateReview(review, reviewId, true)) { return Response.status(Response.Status.OK).entity(review).build(); } else { - return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity("").build(); + String msg = "Review updating failed. Please contact the administrator"; + log.error(msg); + return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build(); } } catch (ReviewManagementException e) { String msg = "Error occurred while retrieving comments."; log.error(msg, e); - return Response.status(Response.Status.INTERNAL_SERVER_ERROR) - .entity(msg).build(); + return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build(); } } @Override @DELETE - @Path("/comment/{commentId}") + @Path("/{commentId}") public Response deleteComment( @PathParam("commentId") int commentId, @QueryParam("username") String username) { - ReviewManager reviewManager = APIUtil.getCommentsManager(); + ReviewManager reviewManager = APIUtil.getReviewManager(); try { if (commentId == 0) { return Response.status(Response.Status.NOT_FOUND).entity("Review not found").build(); @@ -164,8 +162,7 @@ public class ReviewManagementAPIImpl implements ReviewManagementAPI { @Path("/{uuid}/rating") public Response getRating( @PathParam("uuid") String uuid) { - - ReviewManager reviewManager = APIUtil.getCommentsManager(); + ReviewManager reviewManager = APIUtil.getReviewManager(); Rating rating; try { rating = reviewManager.getRating(uuid); diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.store.api/src/test/java/org/wso2/carbon/device/application/mgt/store/api/services/ReviewManagementAPITest.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.store.api/src/test/java/org/wso2/carbon/device/application/mgt/store/api/services/ReviewManagementAPITest.java index f3412cab3c2..654abf6bdfc 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.store.api/src/test/java/org/wso2/carbon/device/application/mgt/store/api/services/ReviewManagementAPITest.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.store.api/src/test/java/org/wso2/carbon/device/application/mgt/store/api/services/ReviewManagementAPITest.java @@ -141,7 +141,7 @@ import static org.mockito.MockitoAnnotations.initMocks; public void testUpdateComment() throws Exception { Review review = CommentMgtTestHelper.getDummyComment("a", "a"); PowerMockito.stub(PowerMockito.method(APIUtil.class, "getReviewManager")).toReturn(this.reviewManager); - Response response = this.commentManagementAPI.updateComment(review, 1); + Response response = this.commentManagementAPI.updateReview(review, 1); Assert.assertNotNull(response, "The response object is null."); Assert.assertEquals(response.getStatus(), Response.Status.OK.getStatusCode(), "The response status should be 200."); @@ -150,7 +150,7 @@ import static org.mockito.MockitoAnnotations.initMocks; @Test public void testUpdateNullComment() throws Exception { PowerMockito.stub(PowerMockito.method(APIUtil.class, "getReviewManager")).toReturn(this.reviewManager); - Response response = this.commentManagementAPI.updateComment(null, 1); + Response response = this.commentManagementAPI.updateReview(null, 1); Assert.assertNotNull(response, "The response object is null."); Assert.assertEquals(response.getStatus(), Response.Status.BAD_REQUEST.getStatusCode(), "The response status should be 400."); @@ -160,7 +160,7 @@ import static org.mockito.MockitoAnnotations.initMocks; public void testUpdateCommentWhenNullCommentId() throws Exception { Review review = CommentMgtTestHelper.getDummyComment("a", "a"); PowerMockito.stub(PowerMockito.method(APIUtil.class, "getReviewManager")).toReturn(this.reviewManager); - Response response = this.commentManagementAPI.updateComment(review, 0); + Response response = this.commentManagementAPI.updateReview(review, 0); Assert.assertNotNull(response, "The response object is null."); Assert.assertEquals(response.getStatus(), Response.Status.NOT_FOUND.getStatusCode(), "The response status should be 404."); @@ -171,7 +171,7 @@ import static org.mockito.MockitoAnnotations.initMocks; Review review = CommentMgtTestHelper.getDummyComment("a", "a"); PowerMockito.stub(PowerMockito.method(APIUtil.class, "getReviewManager")).toReturn(this.reviewManager); Mockito.doThrow(new ReviewManagementException()).when(this.reviewManager).updateReview(review, 9, true); - Response response = this.commentManagementAPI.updateComment(review, 9); + Response response = this.commentManagementAPI.updateReview(review, 9); Assert.assertNotNull(response, "The response object is null."); Assert.assertEquals(response.getStatus(), Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), "The response status should be 500.");