Skip to content

Commit

Permalink
fix(type-clashes): protect from nested-type-clash
Browse files Browse the repository at this point in the history
It was possible for a nested type to be generated with a name that in
fact CLASHED with an existing schema type. What are the odds !

The clash-check added will just verify against clashes with schema
types, which seems to be doing it for now.
  • Loading branch information
Byron committed Mar 11, 2015
1 parent 32145e6 commit 614539a
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 67 deletions.
90 changes: 45 additions & 45 deletions gen/youtube3/src/lib.rs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/mako/README.md.mako
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<%
from util import (markdown_comment, new_context)
c = new_context(resources)
c = new_context(schemas, resources)
%>\
<%namespace name="lib" file="lib/lib.mako"/>\
<%namespace name="util" file="lib/util.mako"/>\
Expand Down
2 changes: 1 addition & 1 deletion src/mako/lib.rs.mako
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
nested_schemas = list()
if schemas:
nested_schemas = list(iter_nested_types(schemas))
c = new_context(resources)
c = new_context(schemas, resources)
hub_type = hub_type(schemas, util.canonical_name())
ht_params = hub_type_params_s()
%>\
Expand Down
8 changes: 4 additions & 4 deletions src/mako/lib/mbuild.mako
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ pub struct ${ThisType}
% for p in params:
${property(p.name)}:\
% if is_required_property(p):
${activity_rust_type(p, allow_optionals=False)},
${activity_rust_type(schemas, p, allow_optionals=False)},
% else:
${activity_rust_type(p)},
${activity_rust_type(schemas, p)},
% endif
% endfor
## A generic map for additinal parameters. Sometimes you can set some that are documented online only
Expand Down Expand Up @@ -161,7 +161,7 @@ ${self._setter_fn(resource, method, m, p, part_prop, ThisType, c)}\
###############################################################################################
<%def name="_setter_fn(resource, method, m, p, part_prop, ThisType, c)">\
<%
InType = activity_input_type(p)
InType = activity_input_type(schemas, p)
def show_part_info(m, p):
if p.name != 'part':
Expand Down Expand Up @@ -217,7 +217,7 @@ ${self._setter_fn(resource, method, m, p, part_prop, ThisType, c)}\
is_string_value = lambda v: v.endswith('"')
# to rust value
trv = lambda spn, sp, sn=None: to_rust_type(sn, spn, sp, allow_optionals=False)
trv = lambda spn, sp, sn=None: to_rust_type(schemas, sn, spn, sp, allow_optionals=False)
# rvfrt = random value for rust type
rvfrt = lambda spn, sp, sn=None: rnd_arg_val_for_type(trv(spn, sp, sn))
Expand Down
6 changes: 3 additions & 3 deletions src/mako/lib/rbuild.mako
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,13 @@ impl${rb_params} ${ThisType} {
method_args = ''
if required_props:
method_args = ', ' + ', '.join('%s: %s' % (mangle_ident(p.name), activity_input_type(p)) for p in required_props)
method_args = ', ' + ', '.join('%s: %s' % (mangle_ident(p.name), activity_input_type(schemas, p)) for p in required_props)
mb_tparams = mb_type_params_s(m)
# we would could have information about data requirements for each property in it's dict.
# for now, we just hardcode it, and treat the entries as way to easily change param names
assert len(api.properties) == 2, "Hardcoded for now, thanks to scope requirements"
type_params = ''
if mb_additional_type_params(m):
type_params = '<%s>' % ', '.join(mb_additional_type_params(m))
Expand All @@ -82,7 +82,7 @@ impl${rb_params} ${ThisType} {
${RType} {
hub: self.hub,
% for p in required_props:
${property(p.name)}: ${rust_copy_value_s(mangle_ident(p.name), activity_input_type(p), p)},
${property(p.name)}: ${rust_copy_value_s(mangle_ident(p.name), activity_input_type(schemas, p), p)},
% endfor
## auto-generate parts from request resources
% if part_prop and request_value:
Expand Down
8 changes: 4 additions & 4 deletions src/mako/lib/schema.mako
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub struct ${s.id}\
{
% for pn, p in properties.iteritems():
${p.get('description', 'no description provided') | rust_doc_comment, indent_all_but_first_by(1)}
pub ${mangle_ident(pn)}: ${to_rust_type(s.id, pn, p)},
pub ${mangle_ident(pn)}: ${to_rust_type(schemas, s.id, pn, p)},
% endfor
}
% else: ## it's an empty struct, i.e. struct Foo;
Expand All @@ -35,7 +35,7 @@ ${doc(s, c)}\
${_new_object(s, s.get('properties'), c)}\
% elif s.type == 'array':
% if s.items.get('type') != 'object':
pub struct ${s.id}(${to_rust_type(s.id, NESTED_TYPE_SUFFIX, s)});
pub struct ${s.id}(${to_rust_type(schemas, s.id, NESTED_TYPE_SUFFIX, s)});
% else:
${_new_object(s, s.items.get('properties'), c)}\
% endif ## array item != 'object'
Expand All @@ -57,7 +57,7 @@ impl ${s.id} {
% for pn, p in s.properties.iteritems():
<%
mn = 'self.' + mangle_ident(pn)
rt = to_rust_type(s.id, pn, p)
rt = to_rust_type(schemas, s.id, pn, p)
check = 'is_some()'
if rt.startswith('Vec') or rt.startswith('HashMap'):
check = 'len() > 0'
Expand Down Expand Up @@ -92,6 +92,6 @@ This type is not used in any activity, and only used as *part* of another schema
% if s.type != 'object':
## for some reason, it's not shown in rustdoc ...
The contained type is `${to_rust_type(s.id, s.id, s)}`.
The contained type is `${to_rust_type(schemas, s.id, s.id, s)}`.
%endif
</%def>
25 changes: 16 additions & 9 deletions src/mako/lib/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,12 +263,18 @@ def mangle_ident(n):
def _is_map_prop(p):
return 'additionalProperties' in p

def _assure_unique_type_name(schemas, tn):
if tn in schemas:
tn += 'Internal'
assert tn not in schemas
return tn

# map a json type to an rust type
# sn = schema name
# pn = property name
# t = type dict
# NOTE: In case you don't understand how this algorithm really works ... me neither - THE AUTHOR
def to_rust_type(sn, pn, t, allow_optionals=True):
def to_rust_type(schemas, sn, pn, t, allow_optionals=True):
def nested_type(nt):
if 'items' in nt:
nt = nt.items
Expand All @@ -277,8 +283,8 @@ def nested_type(nt):
else:
assert(is_nested_type_property(nt))
# It's a nested type - we take it literally like $ref, but generate a name for the type ourselves
return nested_type_name(sn, pn)
return to_rust_type(sn, pn, nt, allow_optionals=False)
return _assure_unique_type_name(schemas, nested_type_name(sn, pn))
return to_rust_type(schemas, sn, pn, nt, allow_optionals=False)

def wrap_type(tn):
if allow_optionals:
Expand Down Expand Up @@ -323,10 +329,10 @@ def is_nested_type(s):

# convert a rust-type to something that would be taken as input of a function
# even though our storage type is different
def activity_input_type(p):
def activity_input_type(schemas, p):
if 'input_type' in p:
return p.input_type
n = activity_rust_type(p, allow_optionals=False)
n = activity_rust_type(schemas, p, allow_optionals=False)
if n == 'String':
n = 'str'
# pods are copied anyway
Expand Down Expand Up @@ -370,6 +376,7 @@ def iter_nested_properties(prefix, properties):
if 'properties' not in s:
continue
for np in iter_nested_properties(s.id, s.properties):
np.id = _assure_unique_type_name(schemas, np.id)
yield np
# end for aech schma

Expand Down Expand Up @@ -409,8 +416,8 @@ def activity_split(fqan):
return t[0], t[1], '.'.join(t[2:])

# Shorthand to get a type from parameters of activities
def activity_rust_type(p, allow_optionals=True):
return to_rust_type(None, p.name, p, allow_optionals=allow_optionals)
def activity_rust_type(schemas, p, allow_optionals=True):
return to_rust_type(schemas, None, p.name, p, allow_optionals=allow_optionals)

# the inverse of activity-split, but needs to know the 'name' of the API
def to_fqan(name, resource, method):
Expand Down Expand Up @@ -563,7 +570,7 @@ def build_all_params(schemas, c, m, n, npn):
Context = collections.namedtuple('Context', ['sta_map', 'fqan_map', 'rta_map', 'rtc_map'])

# return a newly build context from the given data
def new_context(resources):
def new_context(schemas, resources):
if not resources:
return Context(dict(), dict(), dict(), dict())
# Returns (A, B) where
Expand All @@ -587,7 +594,7 @@ def build_activity_mappings(activities, res = None, fqan = None):
t = m.get(in_out_type_name, None)
if t is None:
continue
tn = to_rust_type(None, None, t, allow_optionals=False)
tn = to_rust_type(schemas, None, None, t, allow_optionals=False)
info = res.setdefault(tn, dict())
io_info = info.setdefault(m.id, [])
io_info.append(in_out_type_name)
Expand Down

0 comments on commit 614539a

Please sign in to comment.