wiki:APIDocumentationEfforts
Last modified 19 months ago Last modified on 09/17/12 13:33:26

API Documentation and cleanup

Before we start the api cleanup phase we have to document the current state first. It is easier (and faster) to divide the work amongst more people. So please select your favourite controller and drop some comments there. Following table will help with tracking who is working on documenting what controller, avoiding conflicts and make sure we cover all methods.

When one looks into a controller it's good time for reviewing if it is in line with our api conventions. If you find an issue, please mark it in the table down this page. It will be much easier to fix the problems listed in the table than to do a full audit again.

Also for each action look in the doc/restapi_examples.yml file and make sure it contains some example. If not, take a note here that CLI smoke tests is missing (bellow).

Controllers

  • name - file with the controller
  • owner - who is writing the docs for it
  • state - assigned/in progress/done
name owner state
activation_keys_controller.rb lzap done
api_controller.rb n/a n/a
candlepin_proxies_controller.rb n/a n/a
changesets_content_controller.rb bk done
changesets_controller.rb bk done
crls_controller.rb lzap done
distributions_controller.rb lzap done
environments_controller.rb lzap done
errata_controller.rb msuchy done
errors_controller.rb none api call n/a
filters_controller.rb mbacovsky done
gpg_keys_controller.rb lzap done
organizations_controller.rb tstrachota done
packages_controller.rb lzap done
permissions_controller.rb tstrachota done
ping_controller.rb lzap done
products_controller.rb lzap done
providers_controller.rb ppokorny done
proxies_controller.rb n/a n/a
pulp_proxies_controller.rb n/a n/a
repositories_controller.rb tstrachota done
role_ldap_groups_controller.rb mbacovsk done
roles_controller.rb tstrachota done
root_controller.rb n/a n/a
status_controller.rb lzap done
subscriptions_controller.rb ppokorny done
sync_controller.rb mbacovsky done
sync_plans_controller.rb ppokorny done
system_groups_controller.rb tstrachota done
system_packages_controller.rb ppokorny done
systems_controller.rb inecas done
tasks_controller.rb ppokorny done
templates_content_controller.rb ppokorny done
templates_controller.rb ppokorny done
uebercerts_controller.rb lzap done
users_controller.rb tstrachota done
system_group_errata_controller.rb mbacovsk done
system_group_packages_controller.rb mbacovsk done

Issues found

  • type - parameters, route, return message/object
  • where - which controller and methor or route
  • description - closer description of the problem if needed
type where description
route (EXAMPLE) get /api/organizations/:organization_id/products/:id/repositories a route too deep
params organizations#index query params not sliced
return msg organizations#destroy returns plain text message
return msg users#destroy returns plain text message
params users#index query params not sliced
return msg permission#destroy returns plain text message
params permission#index query params not sliced
activation_keys activation_key#index query params not verified by param_rules
params users, filters, systems, permissions, repositories, organizations, subscriptions, role_ldap_groups #create parameters not nested
params templates_content param names inconsistency (name vs. id)
rest POST SystemGroupsController#{add,remove}_systems not very resty, we should do POST /system_groups/:grp_id/systems/ and DELETE /system_groups/:grp_id/systems/:sys_id/
rest POST SystemGroupsController#{lock,unlock} should be probably moved to update PUT action
route POST /api/repositories/ {:org_id => ..., :product_id => ...} should go under /products/:product_id/repositories/
rest POST /api/repositories/:id/enable can be done via update
return msq changesets_content_controller entire api controller returns text instead of json
return code changesets_content_controller passing in a non existent product id still gets you a 200 return code. (look at render_after_removal)
route changeset_controller index and create routes can be moved to /environments/:env_id/changeset
rest changeset_controller#promote We are POSTING to /promote which is more of an RPC call. Can we do a POST to another environment instead?
logic changeset_controller#promote The promotion API automatically moves the changeset to REVIEW. Should we instead make that a unique call?
route systems_controller POST /consumers and POST /systems probably never used, it should always go under environment
route systems_controller POST /environment/:env_id/systems" for activation keys makes no sense
route systems_controller GET /systems/:id/pacakges vs.GET /systems/:id/profile - we should be ok just with one of them
unused code systems_controller actions subscriptions subsribe unsubscribe seem to be unreachable from routes - marked for removing
route system_group_pacakge GET /api/organizations/:organization_id/system_groups/:system_group_id/packages/new - make no sense in API, not implemented in controller
route system_group_pacakge GET /api/organizations/:organization_id/system_groups/:system_group_id/packages/edit - make no sense in API, not implemented in controller

Fixed

route post,put,delete /users/:user_id/roles/* apart from get the routes are redundant. Actions are also accessible from /roles/*
activation_keys activation_key#find_ find methods should be protected or something
uebercert uebercert#show method should also accept 'true' as well as 'True'
return code changeset_content_controller#render_after_removal not found should return 404 or some other error code.
params organizations#create parameters not in params_match

Missing CLI smoke tests

  • environment#show

Generic issues found

  • Fix incorrect regexp: /\A[\w!| |_|-]*\Z/ (katello name format validator AND docapi initializer)
  • For all our update methods, I would introduce new field called :see => "Class#method", :description => "Check out this method blah blah". For those, we could use :see => "create" which would create link with some commentary (default: "Also see XYZ"). Because we use the very same input for those.
  • "OK" responses should contain message wrapped in json (together with other info as id), or no body at all (HTTP status code is enough).
  • rfe for apipie - add option to set param required=true as default
  • parameters should be sliced in index queries. Eg. SystemGroup.where(params.slice(:name))
  • We often use :id for the last identificator in routes. We should use full names (eg. :product_id, :system_id) instead. It makes the code more readable