KYLIN-2180: refactor ProjectRequest Signed-off-by: Li Yang <liy...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kylin/repo Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/911bdd00 Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/911bdd00 Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/911bdd00 Branch: refs/heads/KYLIN-2131 Commit: 911bdd00afb9176af594fe97cbc0a639f376da2d Parents: 3003d9f Author: kangkaisen <kangkai...@live.com> Authored: Sun Dec 11 17:17:40 2016 +0800 Committer: Li Yang <liy...@apache.org> Committed: Fri Dec 16 11:18:15 2016 +0800 ---------------------------------------------------------------------- .../rest/controller/ProjectController.java | 48 ++++++++++---- .../rest/request/CreateProjectRequest.java | 57 ---------------- .../kylin/rest/request/ProjectRequest.java | 47 +++++++++++++ .../rest/request/UpdateProjectRequest.java | 66 ------------------- .../kylin/rest/service/ProjectService.java | 23 +++---- .../rest/controller/ProjectControllerTest.java | 69 ++++++++++++-------- webapp/app/js/controllers/page.js | 27 +++----- 7 files changed, 143 insertions(+), 194 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kylin/blob/911bdd00/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java b/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java index 05af82c..a6edece 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java +++ b/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java @@ -24,13 +24,13 @@ import java.util.List; import org.apache.commons.lang.StringUtils; import org.apache.kylin.common.persistence.AclEntity; +import org.apache.kylin.common.util.JsonUtil; import org.apache.kylin.cube.CubeInstance; import org.apache.kylin.metadata.project.ProjectInstance; import org.apache.kylin.rest.constant.Constant; import org.apache.kylin.rest.exception.BadRequestException; import org.apache.kylin.rest.exception.InternalErrorException; -import org.apache.kylin.rest.request.CreateProjectRequest; -import org.apache.kylin.rest.request.UpdateProjectRequest; +import org.apache.kylin.rest.request.ProjectRequest; import org.apache.kylin.rest.service.AccessService; import org.apache.kylin.rest.service.CubeService; import org.apache.kylin.rest.service.ProjectService; @@ -198,19 +198,22 @@ public class ProjectController extends BasicController { @RequestMapping(value = "", method = { RequestMethod.POST }) @ResponseBody - public ProjectInstance saveProject(@RequestBody CreateProjectRequest projectRequest) { - if (StringUtils.isEmpty(projectRequest.getName())) { - logger.info("Project name should not be empty."); - throw new BadRequestException("Project name should not be empty."); + + public ProjectInstance saveProject(@RequestBody ProjectRequest projectRequest) { + ProjectInstance projectDesc = deserializeProjectDesc(projectRequest); + + if (StringUtils.isEmpty(projectDesc.getName())) { + throw new InternalErrorException("A project name must be given to create a project"); } - if (!StringUtils.containsOnly(projectRequest.getName(), VALID_PROJECTNAME)) { - logger.info("Invalid Project name {}, only letters, numbers and underline supported.", projectRequest.getName()); + + if (!StringUtils.containsOnly(projectDesc.getName(), VALID_PROJECTNAME)) { + logger.info("Invalid Project name {}, only letters, numbers and underline supported.", projectDesc.getName()); throw new BadRequestException("Invalid Project name, only letters, numbers and underline supported."); } ProjectInstance createdProj = null; try { - createdProj = projectService.createProject(projectRequest); + createdProj = projectService.createProject(projectDesc); } catch (Exception e) { logger.error("Failed to deal with the request.", e); throw new InternalErrorException(e.getLocalizedMessage()); @@ -221,15 +224,22 @@ public class ProjectController extends BasicController { @RequestMapping(value = "", method = { RequestMethod.PUT }) @ResponseBody - public ProjectInstance updateProject(@RequestBody UpdateProjectRequest projectRequest) { - if (StringUtils.isEmpty(projectRequest.getFormerProjectName())) { + public ProjectInstance updateProject(@RequestBody ProjectRequest projectRequest) { + String formerProjectName = projectRequest.getFormerProjectName(); + if (StringUtils.isEmpty(formerProjectName)) { throw new InternalErrorException("A project name must be given to update a project"); } + ProjectInstance projectDesc = deserializeProjectDesc(projectRequest); + ProjectInstance updatedProj = null; try { - ProjectInstance currentProject = projectService.getProjectManager().getProject(projectRequest.getFormerProjectName()); - updatedProj = projectService.updateProject(projectRequest, currentProject); + ProjectInstance currentProject = projectService.getProjectManager().getProject(formerProjectName); + if (currentProject == null) { + throw new InternalErrorException("The project named " + formerProjectName + " does not exists"); + } + + updatedProj = projectService.updateProject(projectDesc, currentProject); } catch (Exception e) { logger.error("Failed to deal with the request.", e); throw new InternalErrorException(e.getLocalizedMessage()); @@ -238,6 +248,18 @@ public class ProjectController extends BasicController { return updatedProj; } + private ProjectInstance deserializeProjectDesc(ProjectRequest projectRequest) { + ProjectInstance projectDesc = null; + try { + logger.debug("Saving project " + projectRequest.getProjectDescData()); + projectDesc = JsonUtil.readValue(projectRequest.getProjectDescData(), ProjectInstance.class); + } catch (Exception e) { + logger.error("Failed to deal with the request.", e); + throw new InternalErrorException("Failed to deal with the request:" + e.getMessage(), e); + } + return projectDesc; + } + @RequestMapping(value = "/{projectName}", method = { RequestMethod.DELETE }) @ResponseBody public void deleteProject(@PathVariable String projectName) { http://git-wip-us.apache.org/repos/asf/kylin/blob/911bdd00/server-base/src/main/java/org/apache/kylin/rest/request/CreateProjectRequest.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/request/CreateProjectRequest.java b/server-base/src/main/java/org/apache/kylin/rest/request/CreateProjectRequest.java deleted file mode 100644 index 00fe1eb..0000000 --- a/server-base/src/main/java/org/apache/kylin/rest/request/CreateProjectRequest.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. -*/ - -package org.apache.kylin.rest.request; - -import java.util.LinkedHashMap; - -/** - */ -public class CreateProjectRequest { - private String name; - private String description; - private LinkedHashMap<String, String> overrideKylinProps; - - public CreateProjectRequest() { - } - - public String getName() { - return name; - } - - public void setName(String name) { - this.name = name; - } - - public String getDescription() { - return description; - } - - public void setDescription(String description) { - this.description = description; - } - - public LinkedHashMap<String, String> getOverrideKylinProps() { - return overrideKylinProps; - } - - public void setOverrideKylinProps(LinkedHashMap<String, String> overrideKylinProps) { - this.overrideKylinProps = overrideKylinProps; - } - -} http://git-wip-us.apache.org/repos/asf/kylin/blob/911bdd00/server-base/src/main/java/org/apache/kylin/rest/request/ProjectRequest.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/request/ProjectRequest.java b/server-base/src/main/java/org/apache/kylin/rest/request/ProjectRequest.java new file mode 100644 index 0000000..da6e190 --- /dev/null +++ b/server-base/src/main/java/org/apache/kylin/rest/request/ProjectRequest.java @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. +*/ + +package org.apache.kylin.rest.request; + +/** + */ +public class ProjectRequest { + private String formerProjectName; + + private String projectDescData; + + public ProjectRequest() { + } + + public String getProjectDescData() { + return projectDescData; + } + + public void setProjectDescData(String projectDescData) { + this.projectDescData = projectDescData; + } + + public String getFormerProjectName() { + return formerProjectName; + } + + public void setFormerProjectName(String formerProjectName) { + this.formerProjectName = formerProjectName; + } + +} http://git-wip-us.apache.org/repos/asf/kylin/blob/911bdd00/server-base/src/main/java/org/apache/kylin/rest/request/UpdateProjectRequest.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/request/UpdateProjectRequest.java b/server-base/src/main/java/org/apache/kylin/rest/request/UpdateProjectRequest.java deleted file mode 100644 index f253c9c..0000000 --- a/server-base/src/main/java/org/apache/kylin/rest/request/UpdateProjectRequest.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. -*/ - -package org.apache.kylin.rest.request; - -import java.util.LinkedHashMap; - -/** - */ -public class UpdateProjectRequest { - private String formerProjectName; - private String newProjectName; - private String newDescription; - private LinkedHashMap<String, String> overrideKylinProps; - - public UpdateProjectRequest() { - } - - public String getFormerProjectName() { - return formerProjectName; - } - - public void setFormerProjectName(String formerProjectName) { - - this.formerProjectName = formerProjectName; - } - - public String getNewDescription() { - return newDescription; - } - - public void setNewDescription(String newDescription) { - this.newDescription = newDescription; - } - - public String getNewProjectName() { - return newProjectName; - } - - public void setNewProjectName(String newProjectName) { - this.newProjectName = newProjectName; - } - - public LinkedHashMap<String, String> getOverrideKylinProps() { - return overrideKylinProps; - } - - public void setOverrideKylinProps(LinkedHashMap<String, String> overrideKylinProps) { - this.overrideKylinProps = overrideKylinProps; - } -} http://git-wip-us.apache.org/repos/asf/kylin/blob/911bdd00/server-base/src/main/java/org/apache/kylin/rest/service/ProjectService.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/ProjectService.java b/server-base/src/main/java/org/apache/kylin/rest/service/ProjectService.java index 283cf4a..e6f546c 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/ProjectService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/ProjectService.java @@ -27,8 +27,6 @@ import org.apache.kylin.metadata.project.ProjectInstance; import org.apache.kylin.metadata.project.ProjectManager; import org.apache.kylin.rest.constant.Constant; import org.apache.kylin.rest.exception.InternalErrorException; -import org.apache.kylin.rest.request.CreateProjectRequest; -import org.apache.kylin.rest.request.UpdateProjectRequest; import org.apache.kylin.rest.security.AclPermission; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,10 +49,10 @@ public class ProjectService extends BasicService { private AccessService accessService; @PreAuthorize(Constant.ACCESS_HAS_ROLE_ADMIN) - public ProjectInstance createProject(CreateProjectRequest projectRequest) throws IOException { - String projectName = projectRequest.getName(); - String description = projectRequest.getDescription(); - LinkedHashMap<String, String> overrideProps = projectRequest.getOverrideKylinProps(); + public ProjectInstance createProject(ProjectInstance newProject) throws IOException { + String projectName = newProject.getName(); + String description = newProject.getDescription(); + LinkedHashMap<String, String> overrideProps = newProject.getOverrideKylinProps(); ProjectInstance currentProject = getProjectManager().getProject(projectName); @@ -70,15 +68,10 @@ public class ProjectService extends BasicService { } @PreAuthorize(Constant.ACCESS_HAS_ROLE_ADMIN + " or hasPermission(#currentProject, 'ADMINISTRATION') or hasPermission(#currentProject, 'MANAGEMENT')") - public ProjectInstance updateProject(UpdateProjectRequest projectRequest, ProjectInstance currentProject) throws IOException { - String formerProjectName = projectRequest.getFormerProjectName(); - String newProjectName = projectRequest.getNewProjectName(); - String newDescription = projectRequest.getNewDescription(); - LinkedHashMap<String, String> overrideProps = projectRequest.getOverrideKylinProps(); - - if (currentProject == null) { - throw new InternalErrorException("The project named " + formerProjectName + " does not exists"); - } + public ProjectInstance updateProject(ProjectInstance newProject, ProjectInstance currentProject) throws IOException { + String newProjectName = newProject.getName(); + String newDescription = newProject.getDescription(); + LinkedHashMap<String, String> overrideProps = newProject.getOverrideKylinProps(); ProjectInstance updatedProject = getProjectManager().updateProject(currentProject, newProjectName, newDescription, overrideProps); http://git-wip-us.apache.org/repos/asf/kylin/blob/911bdd00/server/src/test/java/org/apache/kylin/rest/controller/ProjectControllerTest.java ---------------------------------------------------------------------- diff --git a/server/src/test/java/org/apache/kylin/rest/controller/ProjectControllerTest.java b/server/src/test/java/org/apache/kylin/rest/controller/ProjectControllerTest.java index 9a6dfbe..cd9a524 100644 --- a/server/src/test/java/org/apache/kylin/rest/controller/ProjectControllerTest.java +++ b/server/src/test/java/org/apache/kylin/rest/controller/ProjectControllerTest.java @@ -6,9 +6,9 @@ * to you under the Apache License, Version 2.0 (the * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -19,13 +19,12 @@ package org.apache.kylin.rest.controller; import java.io.IOException; -import java.util.List; +import java.io.StringWriter; import org.apache.kylin.metadata.project.ProjectInstance; import org.apache.kylin.metadata.project.ProjectManager; import org.apache.kylin.rest.exception.InternalErrorException; -import org.apache.kylin.rest.request.CreateProjectRequest; -import org.apache.kylin.rest.request.UpdateProjectRequest; +import org.apache.kylin.rest.request.ProjectRequest; import org.apache.kylin.rest.service.ProjectService; import org.apache.kylin.rest.service.ServiceTestBase; import org.junit.Assert; @@ -33,6 +32,8 @@ import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; +import com.fasterxml.jackson.databind.ObjectMapper; + /** */ public class ProjectControllerTest extends ServiceTestBase { @@ -48,6 +49,7 @@ public class ProjectControllerTest extends ServiceTestBase { projectController = new ProjectController(); projectController.setProjectService(projectService); + try { projectController.deleteProject("new_project"); } catch (InternalErrorException e) { @@ -58,38 +60,34 @@ public class ProjectControllerTest extends ServiceTestBase { } catch (InternalErrorException e) { //project doesn't exist } - } @Test public void testAddUpdateProject() throws IOException { + int originalProjectCount = projectController.getProjects(null, null).size(); - List<ProjectInstance> projects = projectController.getProjects(null, null); - - int originalProjectCount = projects.size(); - CreateProjectRequest request = new CreateProjectRequest(); - request.setName("new_project"); - ProjectInstance ret = projectController.saveProject(request); + //test add project + ProjectInstance project = new ProjectInstance(); + project.setName("new_project"); + ProjectInstance ret = projectController.saveProject(getProjectRequest(project, null)); Assert.assertEquals(ret.getOwner(), "ADMIN"); Assert.assertEquals(ProjectManager.getInstance(getTestConfig()).listAllProjects().size(), originalProjectCount + 1); - UpdateProjectRequest updateR = new UpdateProjectRequest(); - updateR.setFormerProjectName("new_project"); - updateR.setNewProjectName("new_project_2"); - projectController.updateProject(updateR); + //test update project + ProjectInstance newProject = new ProjectInstance(); + newProject.setName("new_project_2"); + projectController.updateProject(getProjectRequest(newProject, "new_project")); Assert.assertEquals(ProjectManager.getInstance(getTestConfig()).listAllProjects().size(), originalProjectCount + 1); Assert.assertEquals(ProjectManager.getInstance(getTestConfig()).getProject("new_project"), null); - Assert.assertNotEquals(ProjectManager.getInstance(getTestConfig()).getProject("new_project_2"), null); - // only update desc: - updateR = new UpdateProjectRequest(); - updateR.setFormerProjectName("new_project_2"); - updateR.setNewProjectName("new_project_2"); - updateR.setNewDescription("hello world"); - projectController.updateProject(updateR); + //test update project description only + ProjectInstance newProject2 = new ProjectInstance(); + newProject2.setName("new_project_2"); + newProject2.setDescription("hello world"); + projectController.updateProject(getProjectRequest(newProject2, "new_project_2")); Assert.assertEquals(ProjectManager.getInstance(getTestConfig()).listAllProjects().size(), originalProjectCount + 1); Assert.assertEquals(ProjectManager.getInstance(getTestConfig()).getProject("new_project"), null); @@ -99,8 +97,27 @@ public class ProjectControllerTest extends ServiceTestBase { @Test(expected = InternalErrorException.class) public void testAddExistingProject() throws IOException { - CreateProjectRequest request = new CreateProjectRequest(); - request.setName("default"); - projectController.saveProject(request); + ProjectInstance newProject = new ProjectInstance(); + newProject.setName("default"); + + projectController.saveProject(getProjectRequest(newProject, null)); + } + + private ProjectRequest getProjectRequest(ProjectInstance project, String formerProjectName) throws IOException { + ProjectRequest request = new ProjectRequest(); + request.setProjectDescData(getProjectDescData(project)); + request.setFormerProjectName(formerProjectName); + + return request; + } + + private String getProjectDescData(ProjectInstance project) throws IOException { + ObjectMapper projectMapper = new ObjectMapper(); + StringWriter projectWriter = new StringWriter(); + projectMapper.writeValue(projectWriter, project); + + System.err.println(projectWriter.toString()); + + return projectWriter.toString(); } } http://git-wip-us.apache.org/repos/asf/kylin/blob/911bdd00/webapp/app/js/controllers/page.js ---------------------------------------------------------------------- diff --git a/webapp/app/js/controllers/page.js b/webapp/app/js/controllers/page.js index 2b6bc64..056cc48 100644 --- a/webapp/app/js/controllers/page.js +++ b/webapp/app/js/controllers/page.js @@ -195,7 +195,7 @@ var projCtrl = function ($scope, $location, $modalInstance, ProjectService, Mess projectIdx: -1 }; $scope.isEdit = false; - $scope.proj = {name: '', description: '', overrideKylinProps: {}}; + $scope.proj = {name: '', description: '', override_kylin_properties: {}}; $scope.convertedProperties = []; if (project) { @@ -203,10 +203,10 @@ var projCtrl = function ($scope, $location, $modalInstance, ProjectService, Mess $scope.state.oldProjName = project.name; $scope.proj = project; - for (var key in $scope.proj.overrideKylinProps) { + for (var key in $scope.proj.override_kylin_properties) { $scope.convertedProperties.push({ name: key, - value: $scope.proj.overrideKylinProps[key] + value: $scope.proj.override_kylin_properties[key] }); } @@ -220,14 +220,7 @@ var projCtrl = function ($scope, $location, $modalInstance, ProjectService, Mess $scope.createOrUpdate = function () { if ($scope.state.isEdit) { - - var requestBody = { - formerProjectName: $scope.state.oldProjName, - newProjectName: $scope.proj.name, - newDescription: $scope.proj.description, - overrideKylinProps: $scope.proj.overrideKylinProps - }; - ProjectService.update({}, requestBody, function (newProj) { + ProjectService.update({}, {formerProjectName: $scope.state.oldProjName, projectDescData: angular.toJson($scope.proj)}, function (newProj) { SweetAlert.swal('Success!', 'Project update successfully!', 'success'); //update project in project model @@ -246,7 +239,7 @@ var projCtrl = function ($scope, $location, $modalInstance, ProjectService, Mess }); } else { - ProjectService.save({}, $scope.proj, function (newProj) { + ProjectService.save({}, {projectDescData: angular.toJson($scope.proj)}, function (newProj) { SweetAlert.swal('Success!', 'New project created successfully!', 'success'); $modalInstance.dismiss('cancel'); $cookieStore.put("project", newProj.name); @@ -271,10 +264,10 @@ var projCtrl = function ($scope, $location, $modalInstance, ProjectService, Mess }; $scope.addNewProperty = function () { - if ($scope.proj.overrideKylinProps.hasOwnProperty('')) { + if ($scope.proj.override_kylin_properties.hasOwnProperty('')) { return; } - $scope.proj.overrideKylinProps[''] = ''; + $scope.proj.override_kylin_properties[''] = ''; $scope.convertedProperties.push({ name: '', value: '' @@ -282,9 +275,9 @@ var projCtrl = function ($scope, $location, $modalInstance, ProjectService, Mess }; $scope.refreshPropertiesObj = function () { - $scope.proj.overrideKylinProps = {}; + $scope.proj.override_kylin_properties = {}; angular.forEach($scope.convertedProperties, function (item, index) { - $scope.proj.overrideKylinProps[item.name] = item.value; + $scope.proj.override_kylin_properties[item.name] = item.value; }) }; @@ -299,7 +292,7 @@ var projCtrl = function ($scope, $location, $modalInstance, ProjectService, Mess if (index > -1) { arr.splice(index, 1); } - delete $scope.proj.overrideKylinProps[item.name]; + delete $scope.proj.override_kylin_properties[item.name]; } };