SEBA-420 rename varchar to text; validation on max_length
Change-Id: I71791d27024260572e552936d39cb1f07ddaab38
diff --git a/lib/xos-genx/xosgenx/generator.py b/lib/xos-genx/xosgenx/generator.py
index 707f87b..d1dc99a 100644
--- a/lib/xos-genx/xosgenx/generator.py
+++ b/lib/xos-genx/xosgenx/generator.py
@@ -52,6 +52,7 @@
) # If neither include_models nor include_apps is specified, then all models will
default_include_apps = [] # be included.
default_strict_validation = False
+ default_lint = False
def __init__(self, **kwargs):
# set defaults
@@ -68,6 +69,7 @@
self.include_models = XOSProcessorArgs.default_include_models
self.include_apps = XOSProcessorArgs.default_include_apps
self.strict_validation = XOSProcessorArgs.default_strict_validation
+ self.lint = XOSProcessorArgs.default_lint
# override defaults with kwargs
for (k, v) in kwargs.items():
@@ -267,19 +269,6 @@
else:
raise Exception("[XosGenX] No inputs provided!")
- if not operator:
- operator = args.target
- template_path = XOSProcessor._get_template(operator)
- else:
- template_path = operator
-
- [template_folder, template_name] = os.path.split(template_path)
- os_template_loader = jinja2.FileSystemLoader(searchpath=[template_folder])
- os_template_env = jinja2.Environment(loader=os_template_loader)
- os_template_env = XOSProcessor._load_jinja2_extensions(
- os_template_env, args.attic
- )
- template = os_template_env.get_template(template_name)
context = XOSProcessor._add_context(args)
parser = plyxproto.ProtobufAnalyzer()
@@ -337,9 +326,27 @@
if validator.errors:
if args.strict_validation or (args.verbosity >= 0):
validator.print_errors()
- if args.strict_validation:
+ fatal_errors = [x for x in validator.errors if x["severity"] == "ERROR"]
+ if fatal_errors and args.strict_validation:
sys.exit(-1)
+ if args.lint:
+ return ""
+
+ if not operator:
+ operator = args.target
+ template_path = XOSProcessor._get_template(operator)
+ else:
+ template_path = operator
+
+ [template_folder, template_name] = os.path.split(template_path)
+ os_template_loader = jinja2.FileSystemLoader(searchpath=[template_folder])
+ os_template_env = jinja2.Environment(loader=os_template_loader)
+ os_template_env = XOSProcessor._load_jinja2_extensions(
+ os_template_env, args.attic
+ )
+ template = os_template_env.get_template(template_name)
+
if args.output is not None and args.write_to_file == "model":
rendered = {}
for i, model in enumerate(v.models):
diff --git a/lib/xos-genx/xosgenx/jinja2_extensions/base.py b/lib/xos-genx/xosgenx/jinja2_extensions/base.py
index 7e10e7d..fd4808e 100644
--- a/lib/xos-genx/xosgenx/jinja2_extensions/base.py
+++ b/lib/xos-genx/xosgenx/jinja2_extensions/base.py
@@ -318,15 +318,13 @@
def xproto_string_type(xptags):
- # FIXME: this try/except block assigns but never uses max_length?
- # try:
- # max_length = eval(xptags["max_length"])
- # except BaseException:
- # max_length = 1024
-
- if "varchar" not in xptags:
+ if "text" not in xptags:
+ # String fields have a mandatory maximum length.
+ # They are intended for relatively short strings.
return "string"
else:
+ # Text fields have an optional maximuim length.
+ # They are intended for long, potentially multiline strings.
return "text"
diff --git a/lib/xos-genx/xosgenx/jinja2_extensions/django.py b/lib/xos-genx/xosgenx/jinja2_extensions/django.py
index 6fac823..613462c 100644
--- a/lib/xos-genx/xosgenx/jinja2_extensions/django.py
+++ b/lib/xos-genx/xosgenx/jinja2_extensions/django.py
@@ -13,7 +13,7 @@
# limitations under the License.
from __future__ import absolute_import, print_function
-from .base import unquote
+from .base import unquote, xproto_string_type
import re
import sys
from six.moves import map
@@ -41,17 +41,23 @@
def django_string_type(xptags):
- try:
- max_length = eval(xptags["max_length"])
- except BaseException:
- max_length = 1024 * 1024
-
- if "content_type" in xptags:
- return django_content_type_string(xptags)
- elif int(max_length) < 1024 * 1024:
- return "CharField"
- else:
+ # xproto_string_type will return "string" if the options.text=False or "text" if options.text=True
+ xtype = xproto_string_type(xptags)
+ if xtype == "string":
+ if "content_type" in xptags:
+ return django_content_type_string(xptags)
+ else:
+ # TODO(smbaker): This is a workaround for incorrect xproto in many services. Prior behavior was to
+ # toggle between Charfield and Textfield when max_length was unspecified, rather than to require
+ # max_length to be specified. Remove this workaround as soon as services have been migrated.
+ if "max_length" in xptags:
+ return "CharField"
+ else:
+ return "TextField"
+ elif xtype == "text":
return "TextField"
+ else:
+ raise Exception("Unknown xproto_string type %s" % xtype, xptags=xptags)
def xproto_django_type(xptype, xptags):
diff --git a/lib/xos-genx/xosgenx/validator.py b/lib/xos-genx/xosgenx/validator.py
index bf73b84..d95c5d7 100644
--- a/lib/xos-genx/xosgenx/validator.py
+++ b/lib/xos-genx/xosgenx/validator.py
@@ -32,7 +32,7 @@
"feedback_state", "unique", "unique_with"]
# Options that must be either "True" or "False"
-BOOLEAN_OPTIONS = ["blank", "db_index", "feedback_state", "gui_hidden", "null", "tosca_key", "unique", "varchar"]
+BOOLEAN_OPTIONS = ["blank", "db_index", "feedback_state", "gui_hidden", "null", "tosca_key", "unique", "text"]
class XProtoValidator(object):
@@ -45,7 +45,7 @@
self.line_map = line_map
self.errors = []
- def error(self, model, field, message):
+ def error(self, model, field, message, severity="ERROR"):
if field and field.get("_linespan"):
error_first_line_number = field["_linespan"][0]
error_last_line_number = field["_linespan"][1]
@@ -61,7 +61,8 @@
error_filename = fn
error_line_offset = start_line
- self.errors.append({"model": model,
+ self.errors.append({"severity": severity,
+ "model": model,
"field": field,
"message": message,
"filename": error_filename,
@@ -69,6 +70,9 @@
"last_line_number": error_last_line_number - error_line_offset,
"absolute_line_number": error_first_line_number})
+ def warning(self, *args, **kwargs):
+ self.error(*args, severity="WARNING", **kwargs)
+
def print_errors(self):
# Sort by line number
for error in sorted(self.errors, key=lambda error: error["absolute_line_number"]):
@@ -83,12 +87,13 @@
else:
linestr = "%d" % first_line_number
- print("[ERROR] %s:%s %s.%s (Type %s): %s" % (os.path.basename(error["filename"]),
- linestr,
- model.get("name"),
- field.get("name"),
- field.get("type"),
- message), file=sys.stderr)
+ print("[%s] %s:%s %s.%s (Type %s): %s" % (error["severity"],
+ os.path.basename(error["filename"]),
+ linestr,
+ model.get("name"),
+ field.get("name"),
+ field.get("type"),
+ message), file=sys.stderr)
def is_option_true(self, field, name):
options = field.get("options")
@@ -185,7 +190,30 @@
self.check_modifier_consistent(model, field)
self.allow_options(model, field,
["blank", "choices", "content_type", "db_index", "default",
- "max_length", "modifier", "null", "varchar"])
+ "max_length", "modifier", "null", "text"])
+
+ # max_length is a mandatory argument of CharField.
+ if (content_type in [None]) and \
+ (not self.is_option_true(field, "text")) and \
+ ("max_length" not in field["options"]):
+ self.error(model, field, "String field should have a max_length or text=True")
+
+ if "max_length" in field["options"]:
+ max_length = field["options"]["max_length"]
+ try:
+ max_length = int(max_length)
+ if (max_length == 0):
+ self.error(model, field, "max_length should not be zero")
+
+ if 0 < abs(256-max_length) < 3:
+ self.warning(model, field,
+ "max_length of %s is close to suggested max_length of 256" % max_length)
+
+ if 0 < abs(1024-max_length) < 3:
+ self.warning(model, field,
+ "max_length of %s is close to suggested max_length of 1024" % max_length)
+ except ValueError:
+ self.error(model, field, "max_length must be a number")
def validate_field_bool(self, model, field):
self.check_modifier_consistent(model, field)
diff --git a/lib/xos-genx/xosgenx/xosgen.py b/lib/xos-genx/xosgenx/xosgen.py
index 608cae7..e223660 100755
--- a/lib/xos-genx/xosgenx/xosgen.py
+++ b/lib/xos-genx/xosgenx/xosgen.py
@@ -116,6 +116,14 @@
default=XOSProcessorArgs.default_checkers,
help="Comma-separated list of static checkers",
)
+group.add_argument(
+ "--lint",
+ dest="lint",
+ action="store_true",
+ default=XOSProcessorArgs.default_lint,
+ help="Parse the xproto but don't execute any xtargets",
+)
+
parse.add_argument(
"files",
@@ -135,6 +143,7 @@
CHECK = 1
GEN = 2
+LINT = 3
class XosGen:
@@ -148,17 +157,17 @@
if args.target:
op = GEN
subdir = "/targets/"
+ operators = [args.target]
elif args.checkers:
op = CHECK
subdir = "/checkers/"
+ operators = args.checkers
+ elif args.lint:
+ op = LINT
+ subdir = None
+ operators = []
else:
- parse.error("At least one of --target and --checkers is required")
-
- operators = (
- args.checkers.split(",")
- if hasattr(args, "checkers") and args.checkers
- else [args.target]
- )
+ parse.error("At least one of --target, --checkers, or --lint is required")
for i in range(len(operators)):
if "/" not in operators[i]:
@@ -202,9 +211,12 @@
'--dest-extension requires --write-to-file to be set to "model"'
)
- else:
+ elif op == CHECK:
if args.write_to_file or args.dest_extension:
parse.error("Checkers cannot write to files")
+ elif op == LINT:
+ # no outputs, so nothing to check
+ pass
inputs = []
@@ -238,3 +250,5 @@
exit(1)
else:
print("%s: OK" % o)
+ elif op == LINT:
+ XOSProcessor.process(args, None)