Discussion: definitions name convention
In light of the discussion in https://github.com/ruby-grape/grape-swagger/pull/515
My proposal is to introduce less magic/assumptions and be more clear about the naming of the definitions.
def model_name(entity)
if entity.respond_to?(:entity_name)
entity.entity_name
else
entity.name
end
end
@JanStevens you should turn this into a pull request with tests, so we can see what it would break
@dblock … think it is better to talk about first, so the solution is based on more then one meaning 😉
@JanStevens … first thanks for starting it
think we should start by talking about: "what would be expected by a third party dev from a API documentation?"
the dev always have the route:
/api/<namespace>/<resource>
should the name of the response object be:
{
"<Resource>": {
"…"
}
}
or:
{
"<NamespaceResource>": {
"…"
}
}
next case
/api/v1/<namespace>/<resourceA>/:keyA/<resourceB>/:keyB
how should this be translated?
@LeFnord is the boss, I wouldn't listen to me :)
I tend to approach these problems in a very brute-force way and use tests as a way to learn about practical implications and expand the understanding of what a change will impact.
@LeFnord The namespaced resource might show too much information about the internal workings of the API endpoint. (And might become troublesome long for no good reason).
But again if we start to assume the name from the class name then we must take care that multiple class names don't clash together, some examples:
-
Public::Tiger&&Internal::Tigerboth reference Tiger but are different. Public shows only limited data and internal shows full data -
ZooEngine::V1::API::Tigerif I give this to an internal dev then great he/she will easily find the name of the related serializer / entity / whatever. For external dev I probably only want to showTiger
Hi @JanStevens, yeap the constraints are now quite clear, but for me the question comes up, how could it be achived in one API?
maybe something like this:
- take the first name, in your case
Tiger, - if it exist more then once, concat it with the name before (
Internal,Public) - and so on
could this be a step in this direction?
Hi,
in my project I have quite a few models and so I use a lot the namespacing and I find this PR very useful.
I want to have all my entities having their name with the namespace, so instead of defining a method entity_name in each model I would prefer to just inherit from a class, but in the code https://github.com/ruby-grape/grape-swagger/blob/master/lib/grape-swagger/doc_methods/data_type.rb#L44
you did
def parse_entity_name(model)
if model.methods(false).include?(:entity_name)
model.entity_name
elsif model.to_s.end_with?('::Entity', '::Entities')
model.to_s.split('::')[-2]
elsif model.respond_to?(:name)
model.name.demodulize.camelize
else
model.to_s.split('::').last
end
end
If you could just change model.methods(false).include?(:entity_name) by model.methods.include?(:entity_name) it would be very nice.
I think this is more natural and more logic and it avoid to defining the entity_name everywhere like in my case.
By the way, I think showing the full name is a better behaviour than just the name. Having an internal and external entity seem a little strange, (for what I understand) entities are used for expose data through an API so they are not supposed to be private / internal and it's seems to be a lot more improbable than having two classes with the same name in different namespace.