Merge pull request #283 from ayyoob/rest-api-improvements

fixed api issues in policy

components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/beans/PolicyWrapper.java

@@ -21,21 +21,20 @@ package org.wso2.carbon.device.mgt.jaxrs.beans;
 import io.swagger.annotations.ApiModel;
 import io.swagger.annotations.ApiModelProperty;
 import org.wso2.carbon.device.mgt.common.Device;
+import org.wso2.carbon.device.mgt.common.DeviceIdentifier;
+
 import java.util.List;
 
 @ApiModel(value = "PolicyWrapper", description = "This class carries all information related to Policy "
         + "Wrappers")
 public class PolicyWrapper {
 
-    @ApiModelProperty(name = "id", value = "The policy ID", required = true)
-    private int id;
-    @ApiModelProperty(name = "profile", value = "Contains the details of the profile that is included in the"
-            + " policy", required = true)
-    private Profile profile;
     @ApiModelProperty(name = "policyName", value = "The name of the policy", required = true)
     private String policyName;
+
     @ApiModelProperty(name = "description", value = "Gives a description on the policy", required = true)
     private String description;
+
     @ApiModelProperty(name = "compliance", value = "Provides the non-compliance rules. WSO2 EMM provides the"
             + " following non-compliance rules:\n"
             + "Enforce - Forcefully enforce the policies on the devices\n"
@@ -44,8 +43,7 @@ public class PolicyWrapper {
             + "violation unknown to the user and the administrator can take the necessary actions with regard"
             + " to the reported", required = true)
     private String compliance;
-    @ApiModelProperty(name = "roles", value = "The roles to whom the policy is applied on", required = true)
+
-    private List<String> roles;
     @ApiModelProperty(name = "ownershipType", value = "The policy ownership type. It can be any of the "
             + "following values:\n"
             + "ANY - The policy will be applied on the BYOD and COPE device types\n"
@@ -53,26 +51,21 @@ public class PolicyWrapper {
             + "COPE (Corporate-Owned, Personally-Enabled) - The policy will only be applied on the COPE "
             + "device type", required = true)
     private String ownershipType;
-    @ApiModelProperty(name = "devices", value = "Lists out the devices the policy is enforced on",
+
+    @ApiModelProperty(name = "profile", value = "Contains the details of the profile that is included in the"
+            + " policy", required = true)
+    private Profile profile;
+
+    @ApiModelProperty(name = "roles", value = "The roles to whom the policy is applied on", required = true)
+    private List<String> roles;
+
+    @ApiModelProperty(name = "deviceIdentifiers", value = "Lists out the devices the policy is enforced on",
             required = true)
-    private List<Device> devices;
+    private List<DeviceIdentifier> deviceIdentifiers;
+
     @ApiModelProperty(name = "users", value = "Lists out the users on whose devices the policy is enforced",
             required = true)
     private List<String> users;
-    @ApiModelProperty(name = "tenantId", value = "The ID of the tenant that created the policy",
-            required = true)
-    private int tenantId;
-    @ApiModelProperty(name = "profileId", value = "The ID of each profile that is in the selected policy",
-            required = true)
-    private int profileId;
-
-    public int getId() {
-        return id;
-    }
-
-    public void setId(int id) {
-        this.id = id;
-    }
 
     public Profile getProfile() {
         return profile;
@@ -122,12 +115,12 @@ public class PolicyWrapper {
         this.ownershipType = ownershipType;
     }
 
-    public List<Device> getDevices() {
+    public List<DeviceIdentifier> getDeviceIdentifiers() {
-        return devices;
+        return deviceIdentifiers;
     }
 
-    public void setDevices(List<Device> devices) {
+    public void setDeviceIdentifier(List<DeviceIdentifier> deviceIdentifier) {
-        this.devices = devices;
+        this.deviceIdentifiers = deviceIdentifiers;
     }
 
     public List<String> getUsers() {
@@ -138,20 +131,4 @@ public class PolicyWrapper {
         this.users = users;
     }
 
-    public int getTenantId() {
-        return tenantId;
-    }
-
-    public void setTenantId(int tenantId) {
-        this.tenantId = tenantId;
-    }
-
-    public int getProfileId() {
-        return profileId;
-    }
-
-    public void setProfileId(int profileId) {
-        this.profileId = profileId;
-    }
-
 }

components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/PolicyManagementService.java

@@ -258,6 +258,7 @@ public interface PolicyManagementService {
                     required = true) PolicyWrapper policy);
 
     @POST
+    @Path("/remove-policy")
     @ApiOperation(
             consumes = MediaType.APPLICATION_JSON,
             produces = MediaType.APPLICATION_JSON,

components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/PolicyManagementServiceImpl.java

@@ -23,11 +23,13 @@ import org.apache.commons.logging.LogFactory;
 import org.wso2.carbon.context.PrivilegedCarbonContext;
 import org.wso2.carbon.device.mgt.common.Device;
 import org.wso2.carbon.device.mgt.common.DeviceIdentifier;
+import org.wso2.carbon.device.mgt.common.DeviceManagementException;
 import org.wso2.carbon.device.mgt.common.authorization.DeviceAccessAuthorizationException;
 import org.wso2.carbon.device.mgt.common.authorization.DeviceAccessAuthorizationService;
 import org.wso2.carbon.device.mgt.core.internal.DeviceManagementDataHolder;
 import org.wso2.carbon.device.mgt.jaxrs.beans.PolicyWrapper;
 import org.wso2.carbon.device.mgt.jaxrs.service.api.PolicyManagementService;
+import org.wso2.carbon.device.mgt.jaxrs.service.impl.util.PolicyFilteringUtil;
 import org.wso2.carbon.device.mgt.jaxrs.util.DeviceMgtAPIUtils;
 import org.wso2.carbon.device.mgt.jaxrs.util.DeviceMgtUtil;
 import org.wso2.carbon.policy.mgt.common.Policy;
@@ -51,49 +53,62 @@ public class PolicyManagementServiceImpl implements PolicyManagementService {
     @Override
     public Response addPolicy(PolicyWrapper policyWrapper) {
         PolicyManagerService policyManagementService = DeviceMgtAPIUtils.getPolicyManagementService();
-        Policy policy = this.getPolicyFromWrapper(policyWrapper);
+        try {
+            Policy policy = this.getPolicyFromWrapper(policyWrapper);
 
-        List<Device> devices = policy.getDevices();
+            List<Device> devices = policy.getDevices();
-        if (devices != null && devices.size() == 1) {
+            if (devices != null && devices.size() == 1) {
-            DeviceAccessAuthorizationService deviceAccessAuthorizationService =
+                DeviceAccessAuthorizationService deviceAccessAuthorizationService =
-                    DeviceManagementDataHolder.getInstance().getDeviceAccessAuthorizationService();
+                        DeviceManagementDataHolder.getInstance().getDeviceAccessAuthorizationService();
-            DeviceIdentifier deviceIdentifier = new DeviceIdentifier(devices.get(0).getDeviceIdentifier(),
+                DeviceIdentifier deviceIdentifier = new DeviceIdentifier(devices.get(0).getDeviceIdentifier(),
-                    devices.get(0).getType());
+                        devices.get(0).getType());
-            PrivilegedCarbonContext threadLocalCarbonContext = PrivilegedCarbonContext.getThreadLocalCarbonContext();
+                PrivilegedCarbonContext threadLocalCarbonContext = PrivilegedCarbonContext.getThreadLocalCarbonContext();
-            String username = threadLocalCarbonContext.getUsername();
+                String username = threadLocalCarbonContext.getUsername();
-            try {
+                try {
-                if (!deviceAccessAuthorizationService.isUserAuthorized(deviceIdentifier, username)) {
+                    if (!deviceAccessAuthorizationService.isUserAuthorized(deviceIdentifier, username)) {
-                    return Response.status(Response.Status.UNAUTHORIZED).entity("Current logged in user is " +
+                        return Response.status(Response.Status.UNAUTHORIZED).entity("Current logged in user is " +
-                            "not authorized to add policies").build();
+                                "not authorized to add policies").build();
+                    }
+                } catch (DeviceAccessAuthorizationException e) {
+                    String msg = "ErrorResponse occurred while checking if the current user is authorized to add a policy";
+                    log.error(msg, e);
+                    return javax.ws.rs.core.Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build();
                 }
-            } catch (DeviceAccessAuthorizationException e) {
-                String msg = "ErrorResponse occurred while checking if the current user is authorized to add a policy";
-                log.error(msg, e);
-                return javax.ws.rs.core.Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build();
             }
-        }
+
-        try {
             PolicyAdministratorPoint pap = policyManagementService.getPAP();
             pap.addPolicy(policy);
-            return Response.status(Response.Status.OK).entity("Policy has been added successfully").build();
+            return Response.status(Response.Status.CREATED).entity("Policy has been added successfully").build();
         } catch (PolicyManagementException e) {
             String msg = "ErrorResponse occurred while adding policy";
             log.error(msg, e);
             return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build();
+        } catch (DeviceManagementException e) {
+            String msg = "ErrorResponse occurred while retrieving device list.";
+            log.error(msg, e);
+            return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build();
         }
     }
 
-    private Policy getPolicyFromWrapper(PolicyWrapper policyWrapper) {
+    private Policy getPolicyFromWrapper(PolicyWrapper policyWrapper) throws DeviceManagementException {
-        Policy policy = new org.wso2.carbon.policy.mgt.common.Policy();
+        Policy policy = new Policy();
         policy.setPolicyName(policyWrapper.getPolicyName());
-        policy.setProfileId(policyWrapper.getProfileId());
         policy.setDescription(policyWrapper.getDescription());
         policy.setProfile(DeviceMgtUtil.convertProfile(policyWrapper.getProfile()));
         policy.setOwnershipType(policyWrapper.getOwnershipType());
         policy.setRoles(policyWrapper.getRoles());
         policy.setUsers(policyWrapper.getUsers());
-        policy.setTenantId(policyWrapper.getTenantId());
         policy.setCompliance(policyWrapper.getCompliance());
+        //TODO iterates the device identifiers to create the object. need to implement a proper DAO layer here.
+        List<Device> devices = null;
+        List<DeviceIdentifier> deviceIdentifiers = policyWrapper.getDeviceIdentifiers();
+        if (deviceIdentifiers != null) {
+            for (DeviceIdentifier id : deviceIdentifiers) {
+                devices.add(DeviceMgtAPIUtils.getDeviceManagementService().getDevice(id));
+            }
+        }
+        policy.setDevices(devices);
+        policy.setTenantId(PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantId());
         return policy;
     }
 
@@ -116,7 +131,8 @@ public class PolicyManagementServiceImpl implements PolicyManagementService {
             log.error(msg, e);
             return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build();
         }
-        return Response.status(Response.Status.OK).entity(policies).build();
+        return Response.status(Response.Status.OK).entity(PolicyFilteringUtil.getPolicies(policies, offset, limit))
+                .build();
     }
 
     @GET
@@ -144,19 +160,29 @@ public class PolicyManagementServiceImpl implements PolicyManagementService {
     @Override
     public Response updatePolicy(@PathParam("id") int id, PolicyWrapper policyWrapper) {
         PolicyManagerService policyManagementService = DeviceMgtAPIUtils.getPolicyManagementService();
-        Policy policy = this.getPolicyFromWrapper(policyWrapper);
         try {
+            Policy policy = this.getPolicyFromWrapper(policyWrapper);
+            policy.setId(id);
             PolicyAdministratorPoint pap = policyManagementService.getPAP();
+            Policy exisitingPolicy = pap.getPolicy(id);
+            if (exisitingPolicy == null) {
+                return Response.status(Response.Status.NOT_FOUND).entity("Policy not found.").build();
+            }
             pap.updatePolicy(policy);
             return Response.status(Response.Status.OK).entity("Policy has successfully been updated").build();
         } catch (PolicyManagementException e) {
             String msg = "ErrorResponse occurred while updating the policy";
             log.error(msg, e);
             return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build();
+        } catch (DeviceManagementException e) {
+            String msg = "ErrorResponse occurred while retrieving the device list.";
+            log.error(msg, e);
+            return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build();
         }
     }
 
     @POST
+    @Path("/remove-policy")
     @Override
     public Response removePolicies(List<Integer> policyIds) {
         PolicyManagerService policyManagementService = DeviceMgtAPIUtils.getPolicyManagementService();
@@ -164,8 +190,8 @@ public class PolicyManagementServiceImpl implements PolicyManagementService {
         try {
             PolicyAdministratorPoint pap = policyManagementService.getPAP();
             for (int i : policyIds) {
-                org.wso2.carbon.policy.mgt.common.Policy policy = pap.getPolicy(i);
+                Policy policy = pap.getPolicy(i);
-                if (!pap.deletePolicy(policy)) {
+                if (policy == null || !pap.deletePolicy(policy)) {
                     policyDeleted = false;
                 }
             }
@@ -182,41 +208,62 @@ public class PolicyManagementServiceImpl implements PolicyManagementService {
         }
     }
 
-    @POST
+    @PUT
     @Path("/activate-policy")
     @Override
     public Response activatePolicies(List<Integer> policyIds) {
+        boolean isPolicyActivated = false;
         try {
             PolicyManagerService policyManagementService = DeviceMgtAPIUtils.getPolicyManagementService();
             PolicyAdministratorPoint pap = policyManagementService.getPAP();
             for (int i : policyIds) {
-                pap.activatePolicy(i);
+                Policy policy = pap.getPolicy(i);
+                if (policy != null) {
+                    pap.activatePolicy(i);
+                    isPolicyActivated = true;
+                }
             }
         } catch (PolicyManagementException e) {
             String msg = "ErrorResponse occurred while activating policies";
             log.error(msg, e);
             return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build();
         }
-        return Response.status(Response.Status.OK).entity("Selected policies have been successfully activated").build();
+        if (isPolicyActivated) {
+            return Response.status(Response.Status.OK).entity("Selected policies have been successfully activated")
+                    .build();
+        } else {
+            return Response.status(Response.Status.NOT_FOUND).entity("Selected policies have not been activated")
+                    .build();
+        }
     }
 
-    @POST
+    @PUT
     @Path("/deactivate-policy")
     @Override
     public Response deactivatePolicies(List<Integer> policyIds) {
+        boolean isPolicyDeActivated = false;
         try {
             PolicyManagerService policyManagementService = DeviceMgtAPIUtils.getPolicyManagementService();
             PolicyAdministratorPoint pap = policyManagementService.getPAP();
             for (int i : policyIds) {
-                pap.inactivatePolicy(i);
+                Policy policy = pap.getPolicy(i);
+                if (policy != null) {
+                    pap.inactivatePolicy(i);
+                    isPolicyDeActivated = true;
+                }
             }
         } catch (PolicyManagementException e) {
             String msg = "Exception in inactivating policies.";
             log.error(msg, e);
             return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(msg).build();
         }
-        return Response.status(Response.Status.OK).entity("Selected policies have been successfully " +
+        if (isPolicyDeActivated) {
-                "deactivated").build();
+            return Response.status(Response.Status.OK).entity("Selected policies have been successfully " +
+                                                                      "deactivated").build();
+        } else {
+            return Response.status(Response.Status.NOT_FOUND).entity("Selected policies have not been deactivated")
+                    .build();
+        }
     }
 
 }

components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/util/PolicyFilteringUtil.java

@@ -0,0 +1,41 @@
+/*
+ *   Copyright (c) 2016, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
+ *
+ *   WSO2 Inc. 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.wso2.carbon.device.mgt.jaxrs.service.impl.util;
+
+import org.wso2.carbon.policy.mgt.common.Policy;
+
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * This is used instead of filtering from cache.
+ * Todo : need to implement proper pagination support on retrieving policies.
+ */
+public class PolicyFilteringUtil {
+
+    /**
+     * This is used to filter from the cached policies.
+     */
+    public static List<Policy> getPolicies(List<Policy> sourceList, int offset, int limit) {
+        if(sourceList == null || sourceList.size() < offset){
+            return Collections.emptyList();
+        }
+        return sourceList.subList(offset, Math.min(offset + limit, sourceList.size()));
+    }
+}

components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/webapp/META-INF/permissions.xml

@@ -736,7 +736,7 @@
     <Permission>
         <name>Remove policy</name>
         <path>/device-mgt/admin/policies/remove</path>
-        <url>/policies/bulk-remove</url>
+        <url>/policies/remove-policy</url>
         <method>POST</method>
     </Permission>