Juan Hernandez has posted comments on this change. Change subject: restapi: CRUD for OpenStackVolume authentication keys ......................................................................
Patch Set 7: (4 comments) https://gerrit.ovirt.org/#/c/41969/7/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/openstack/OpenStackVolumeAuthenticationKeyResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/openstack/OpenStackVolumeAuthenticationKeyResource.java: Line 35: @Consumes({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) Line 36: public OpenstackVolumeAuthenticationKey update(OpenstackVolumeAuthenticationKey resource); Line 37: Line 38: @DELETE Line 39: Response remove(); Consider using "public" in all the methods, or in none of them. IDEs complain less if none of the methods of interfaces have the "public" modifier. https://gerrit.ovirt.org/#/c/41969/7/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/openstack/OpenStackVolumeAuthenticationKeysResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/openstack/OpenStackVolumeAuthenticationKeysResource.java: Line 38: @Consumes({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) Line 39: public Response add(OpenstackVolumeAuthenticationKey authenticationKey); Line 40: Line 41: @Path("{id}") Line 42: OpenStackVolumeAuthenticationKeyResource getOpenStackVolumeAuthenticationKey(@PathParam("id") String id); Consider adding "public" to all or none of the methods of the interface. https://gerrit.ovirt.org/#/c/41969/7/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml File backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml: Line 3924: request: Line 3925: body: Line 3926: parameterType: OpenstackVolumeAuthenticationKey Line 3927: signatures: Line 3928: - mandatoryArguments: {} This shouldn't be needed, but if you are having trouble we can leave it. Line 3929: optionalArguments: Line 3930: openstack_volume_authentication_key.value: xs:string Line 3931: openstack_volume_authentication_key.usage_type: xs:string Line 3932: openstack_volume_authentication_key.description: xs:string https://gerrit.ovirt.org/#/c/41969/7/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/openstack/OpenStackVolumeAuthenticationKeyMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/openstack/OpenStackVolumeAuthenticationKeyMapper.java: Line 70: entity.setUsageType(map(usageType, null)); Line 71: } Line 72: } Line 73: if (model.isSetOpenstackVolumeProvider()) { Line 74: entity.setProviderId(GuidUtils.asGuid(model.getOpenstackVolumeProvider().getId())); The id of the provider may be missing, as the user may send the following: <openstack_volume_provider/> That is useless, but can happen, and in that case this will trigger a NPE inside the "Guid(String)" constructor. Consider checking if the id has a value before using it. Line 75: } Line 76: return entity; Line 77: } Line 78: -- To view, visit https://gerrit.ovirt.org/41969 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1041a9760c29919d86da5479ce29db666d18a924 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
