Hi, Added a few comments to the shared sheet.
Mentioned below are some common comments which would apply to almost all; 1. Should we introduce DELETE for all (applicable) types of resources ? - Of course before doing a delete a series of checks will have to be done and maybe we can add a specific permission to delete operations so as to make sure it's secured 2. Response codes - We should always set the correct response code for each call based on the operation (we have done this in some places but not consistently) - this should also incorporate common server and client codes such as 403/ 401 - Have added a column in the shared doc - please validate 3. Making the REST API more user friendly 1. Making HEAD and OPTIONS available 2. Allowing HTTP method overriding 3. Introducing an error message (Mentioned oriinally by Akila) in a consumable format - e.g. error json with a id, message and description WDYT ? Thank you, Shiro Ref: 1. https://restful-api-design.readthedocs.org/en/latest/intro.html 2. http://www.vinaysahni.com/best-practices-for-a-pragmatic-restful-api#method-override On Mon, Oct 6, 2014 at 8:11 AM, Shiroshica Kulatilake <sh...@wso2.com> wrote: > > > On Mon, Oct 6, 2014 at 8:00 AM, Nirmal Fernando <nirmal070...@gmail.com> > wrote: > >> I've enabled only comments since I am publishing it to dev list: >> https://docs.google.com/spreadsheets/d/1VP9-0xY2Oj1H1sv5yf5goIT6TakpwrCt5Z8QW1oFVOc/edit?usp=sharing >> > > Great will put in comments > > Thank you, > Shiro > > > >> On Mon, Oct 6, 2014 at 7:33 AM, Nirmal Fernando <nirmal070...@gmail.com> >> wrote: >> >>> Sure, I hope, I'll be able to track who suggested what? >>> >>> On Mon, Oct 6, 2014 at 7:31 AM, Shiroshica Kulatilake <sh...@wso2.com> >>> wrote: >>> >>>> Hi Nirmal, >>>> >>>> Can you share this in the form of a sheet ? >>>> Then we can have a suggested column to have the diffs >>>> >>>> Thank you, >>>> Shiro >>>> >>>> On Mon, Oct 6, 2014 at 7:23 AM, Nirmal Fernando <nirmal070...@gmail.com >>>> > wrote: >>>> >>>>> Thanks Akila for going through and identifying collisions. >>>>> >>>>> On Sun, Oct 5, 2014 at 11:26 PM, Akila Ravihansa Perera < >>>>> raviha...@wso2.com> wrote: >>>>> >>>>>> Hi Nirmal, >>>>>> >>>>>> Great work on re-factoring REST API! >>>>>> >>>>>> I think these request paths might cause collisions. >>>>>> >>>>>> 1. /cartridges/multiTenant, /cartridges/singleTenant, >>>>>> /cartridges/{cartridgeType} >>>>>> >>>>> >>>>> I don't think in a real world, we would have a {cartridgeType} called >>>>> 'singleTenant' or 'multiTenant'. Do you think? >>>>> >>>>> >>>>>> >>>>>> >>>>>> 2. /clusters/{cartridgeType}, /clusters/{subscriptionAlias}, >>>>>> /clusters/{clusterId} >>>>>> >>>>> >>>>> This is true, thanks again! So, we could refactor to something like: >>>>> >>>>> /clusters/cartridgeType/{cartridgeType} >>>>> /clusters/alias/{subscriptionAlias} >>>>> /clusters/{clusterId} >>>>> >>>>> Any other better way? >>>>> >>>>> >>>>>> >>>>>> I might be wrong...but just wanted to clarify :) >>>>>> >>>>>> Thanks. >>>>>> >>>>>> On Sun, Oct 5, 2014 at 9:06 PM, Udara Liyanage <ud...@wso2.com> >>>>>> wrote: >>>>>> >>>>>>> +1 for refactoring, some paths does not adhere to REST best practices >>>>>>> >>>>>>> On Sun, Oct 5, 2014 at 8:54 PM, Nirmal Fernando < >>>>>>> nirmal070...@gmail.com> wrote: >>>>>>> >>>>>>>> Hi Guys, >>>>>>>> >>>>>>>> As discussed previously, I've been going through all the API >>>>>>>> operations exposed via Stratos REST API and identified following set of >>>>>>>> APIs need to be refactored to have a RESTful design. While doing this, >>>>>>>> I've >>>>>>>> also identified sub-APIs that we have provided (under Entity column) >>>>>>>> and I >>>>>>>> suggest we could introduce sub-API classes as it makes the StratosAdmin >>>>>>>> class more readable. >>>>>>>> >>>>>>>> There're 40+ API operations ([?]), hence there could be places that >>>>>>>> I've not suggested proper refactoring. Please go through the list and >>>>>>>> provide improvements/comments, we can discuss/argue, then conclude and >>>>>>>> build a clean API for Stratos. >>>>>>>> >>>>>>>> IMO we also need to place a review process when adding a new API, >>>>>>>> it could save lot of time spending on cleaning things up. I propose, >>>>>>>> API >>>>>>>> introducer should initiate a discuss thread and a vote before >>>>>>>> committing >>>>>>>> them to the repository. >>>>>>>> >>>>>>>> I'll keep this thread open for 48hrs for comments and/or >>>>>>>> improvements. >>>>>>>> >>>>>>>> EntityHTTP OperationAPI Operation Name -> Suggested NameRequest >>>>>>>> PathProposed PathCartridgePOSTdeployCartridgeDefinition >>>>>>>> /cartridge/definition//cartridgesDELETEunDeployCartridgeDefinition >>>>>>>> /cartridge/definition/{cartridgeType}/cartridges/{cartridgeType}GETgetValidDeploymentPolicies >>>>>>>> -> getValidDeploymentPoliciesOfCartridge >>>>>>>> {cartridgeType}/policy/deployment >>>>>>>> /cartridges/{cartridgeType}/deploymentPolicyGETgetAvailableMultiTenantCartridges >>>>>>>> -> getMultiTenantCartridges/cartridge/tenanted/list >>>>>>>> /cartridges/multiTenantGETgetAvailableSingleTenantCartridges -> >>>>>>>> getSingleTenantCartridges/cartridge/list/cartridges/singleTenantGETgetAvailableCartridges >>>>>>>> -> >>>>>>>> getCartridges/cartridge/available/list/cartridgesGETgetAvailableSingleTenantCartridgeInfo >>>>>>>> -> getAvailableSingleTenantCartridge >>>>>>>> /cartridge/available/info/{cartridgeType} >>>>>>>> /cartridges/{cartridgeType}/singleTenantGETgetAvailableLbCartridges >>>>>>>> /cartridge/lb/cartridges/loadBalancerPartitionPOSTdeployPartition >>>>>>>> /policy/deployment/partition/partitionsGETgetPartitions/partition >>>>>>>> /partitionsGETgetPartition/partition/{partitionId} >>>>>>>> /partitions/{partitionId}Autoscaling PolicyPOST >>>>>>>> deployAutoscalingPolicyDefintion/policy/autoscale/autoscalePolicies >>>>>>>> GETgetAutoscalePolicies -> getAutoscalingPolicies/policy/autoscale >>>>>>>> /autoscalePoliciesGETgetAutoscalePolicies -> getAutoscalingPolicy >>>>>>>> /policy/autoscale/{autoscalePolicyId} >>>>>>>> /autoscalePolicies/{autoscalePolicyId}Deployment PolicyPOST >>>>>>>> deployDeploymentPolicyDefinition/policy/deployment >>>>>>>> /deploymentPoliciesGETgetDeploymentPolicies/policy/deployment >>>>>>>> /deploymentPoliciesGETgetDeploymentPolicies -> getDeploymentPolicy >>>>>>>> /policy/deployment/{deploymentPolicyId} >>>>>>>> /deploymentPolicies/{deploymentPolicyId}Partition GroupGET >>>>>>>> getPartitionGroups/partition/group/{deploymentPolicyId} >>>>>>>> /deploymentPolicies/{deploymentPolicyId}/partitionGroupGETgetPartitions >>>>>>>> -> getPartitionGroup >>>>>>>> /partition/{deploymentPolicyId}/{partitionGroupId} >>>>>>>> /deploymentPolicies/{deploymentPolicyId}/partitionGroup/{partitionGroupId} >>>>>>>> GETgetPartitionsOfPolicy/partition/{deploymentPolicyId} >>>>>>>> /deploymentPolicies/{deploymentPolicyId}/partitionSubscriptionsGET >>>>>>>> getSubscribedCartridges/cartridge/list/subscribed >>>>>>>> /subscriptions/cartridgesGETgetSubscribedCartridgesForServiceGroup >>>>>>>> -> getSubscribedCartridgesOfServiceGroup >>>>>>>> /cartridge/list/subscribed/group/{serviceGroup} >>>>>>>> /subscriptions/cartridges/groups/{serviceGroup}GETgetCartridgeInfo >>>>>>>> -> getSubscribedCartridgeInfo/cartridge/info/{subscriptionAlias} >>>>>>>> /subscriptions/{subscriptionAlias}/cartridgesGETgetActiveInstances >>>>>>>> -> getActiveMembersCountOfSubscription >>>>>>>> /cartridge/active/{cartridgeType}/{subscriptionAlias} >>>>>>>> /subscriptions/{subscriptionAlias}/cartridges/{cartridgeType}/active >>>>>>>> POSTsubscribe/cartridge/subscribe/subscriptionsDELETEunsubscribe >>>>>>>> /cartridge/unsubscribe/subscriptions/{subscriptionAlias}POST >>>>>>>> addSubscriptionDomains >>>>>>>> /cartridge/{cartridgeType}/subscription/{subscriptionAlias}/domains >>>>>>>> /subscriptions/{subscriptionAlias}/domainsGETgetSubscriptionDomains >>>>>>>> /cartridge/{cartridgeType}/subscription/{subscriptionAlias}/domains >>>>>>>> /subscriptions/{subscriptionAlias}/domainsGETgetSubscriptionDomain >>>>>>>> /cartridge/{cartridgeType}/subscription/{subscriptionAlias}/domains/{domainName} >>>>>>>> /subscriptions/{subscriptionAlias}/domains/{domainName}DELETE >>>>>>>> removeSubscriptionDomain >>>>>>>> /cartridge/{cartridgeType}/subscription/{subscriptionAlias}/domains/{domainName} >>>>>>>> /subscriptions/{subscriptionAlias}/domains/{domainName}ClustersGETgetClustersForTenant >>>>>>>> -> getClustersOfTenant/cluster//clustersGETgetClusters -> >>>>>>>> getClustersOfCartridge/cluster/{cartridgeType}/ >>>>>>>> /clusters/{cartridgeType}GETgetServiceClusters (This seems to do >>>>>>>> the same job as getClusters - I suggest we deprecate >>>>>>>> this.)GETgetCluster >>>>>>>> -> getClusterOfSubscription >>>>>>>> /cluster/{cartridgeType}/{subscriptionAlias} >>>>>>>> /clusters/{subscriptionAlias}GETgetCluster >>>>>>>> /cluster/clusterId/{clusterId}/clusters/{clusterId}GET >>>>>>>> getLoadBalancerCluster >>>>>>>> /cartridge/{cartridgeType}/subscription/{subscriptionAlias}/load-balancer-cluster >>>>>>>> /clusters/{subscriptionAlias}/loadBalancerTenantsGETretrieveTenants >>>>>>>> /tenant/list/tenants/list(All paths will be refactored to start >>>>>>>> from /tenants )Services POSTdeployService/service/definition >>>>>>>> /servicesDELETEunDeployService/service/definition/{serviceType} >>>>>>>> /services/{serviceType}(All paths will be refactored to start from >>>>>>>> /services )GIT RepositoriesPOSTgetRepoNotification -> >>>>>>>> notifyRepository/reponotification/repo/notifyPOSTsynchronizeRepository >>>>>>>> -> synchronizeRepositoryOfSubscription/cartridge/sync >>>>>>>> /repo/synchronize/{subscriptionAlias}UsersGETretrieveUsers >>>>>>>> /user/list/usersKubernetes HostsPOSTdeployKubernetesGroup -> >>>>>>>> deployKubernetesHostCluster/kubernetes/deploy/group >>>>>>>> /kubernetesClusterPUTdeployKubernetesHost >>>>>>>> /kubernetes/deploy/host/{kubernetesGroupId} >>>>>>>> /kubernetesCluster/{kubernetesClusterId}/minionPUT >>>>>>>> updateKubernetesMaster/kubernetes/update/master >>>>>>>> /kubernetesCluster/{kubernetesClusterId}/masterPATCH >>>>>>>> updateKubernetesHost/kubernetes/update/host >>>>>>>> /kubernetesCluster/{kubernetesClusterId}/minion/{minionId}GETgetKubernetesGroups >>>>>>>> -> >>>>>>>> getKubernetesHostClusters/kubernetes/group/kubernetesClusterGETgetKubernetesGroup >>>>>>>> -> getKubernetesHostCluster/kubernetes/group/{kubernetesGroupId} >>>>>>>> /kubernetesCluster/{kubernetesClusterId}GETgetKubernetesHosts -> >>>>>>>> getKubernetesHostsOfKubernetesCluster >>>>>>>> /kubernetes/hosts/{kubernetesGroupId} >>>>>>>> /kubernetesCluster/{kubernetesClusterId}/hostsGETgetKubernetesMaster >>>>>>>> -> getKubernetesMasterOfKubernetesCluster >>>>>>>> /kubernetes/master/{kubernetesGroupId} >>>>>>>> /kubernetesCluster/{kubernetesClusterId}/masterDELETEunDeployKubernetesGroup >>>>>>>> -> unDeployKubernetesHostCluster >>>>>>>> /kubernetes/group/{kubernetesGroupId} >>>>>>>> /kubernetesCluster/{kubernetesClusterId}DELETEunDeployKubernetesHost >>>>>>>> -> unDeployKubernetesHostOfKubernetesCluster >>>>>>>> /kubernetes/host/{kubernetesHostId} >>>>>>>> /kubernetesCluster/{kubernetesClusterId}/hosts/{hostId} >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Best Regards, >>>>>>>> Nirmal >>>>>>>> >>>>>>>> Nirmal Fernando. >>>>>>>> PPMC Member & Committer of Apache Stratos, >>>>>>>> Senior Software Engineer, WSO2 Inc. >>>>>>>> >>>>>>>> Blog: http://nirmalfdo.blogspot.com/ >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> >>>>>>> Udara Liyanage >>>>>>> Software Engineer >>>>>>> WSO2, Inc.: http://wso2.com >>>>>>> lean. enterprise. middleware >>>>>>> >>>>>>> web: http://udaraliyanage.wordpress.com >>>>>>> phone: +94 71 443 6897 >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Akila Ravihansa Perera >>>>>> Software Engineer, WSO2 >>>>>> >>>>>> Blog: http://ravihansa3000.blogspot.com >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Best Regards, >>>>> Nirmal >>>>> >>>>> Nirmal Fernando. >>>>> PPMC Member & Committer of Apache Stratos, >>>>> Senior Software Engineer, WSO2 Inc. >>>>> >>>>> Blog: http://nirmalfdo.blogspot.com/ >>>>> >>>> >>>> >>>> >>>> -- >>>> Shiroshica Kulatilake >>>> >>>> Architect, >>>> WSO2, Inc. http://wso2.com/ >>>> Phone: +94 776523867 >>>> >>> >>> >>> >>> -- >>> Best Regards, >>> Nirmal >>> >>> Nirmal Fernando. >>> PPMC Member & Committer of Apache Stratos, >>> Senior Software Engineer, WSO2 Inc. >>> >>> Blog: http://nirmalfdo.blogspot.com/ >>> >> >> >> >> -- >> Best Regards, >> Nirmal >> >> Nirmal Fernando. >> PPMC Member & Committer of Apache Stratos, >> Senior Software Engineer, WSO2 Inc. >> >> Blog: http://nirmalfdo.blogspot.com/ >> > > > > -- > Shiroshica Kulatilake > > Architect, > WSO2, Inc. http://wso2.com/ > Phone: +94 776523867 > -- Shiroshica Kulatilake Architect, WSO2, Inc. http://wso2.com/ Phone: +94 776523867
