RepreZen / KaiZen-OpenApi-Parser

High-performance Parser, Validator, and Java Object Model for OpenAPI 3.x
130 stars 30 forks source link

ZEN-4409 Fix NPE thrown on any model with EncodingProperty element #222

Closed tfesenko closed 6 years ago

tfesenko commented 6 years ago

The NPE is thrown on any model with an EncodingProperty, even on a small model like this:

---
openapi: "3.0.0"
info:
  version: 1.0.0
  title: My API Spec
paths:
  /resourceUrl:
    get:
      responses: {}
      requestBody:
        content:
          application/x-www-form-urlencoded:
            encoding:
              expand:
                explode: true
                style: deepObject
            schema: {}

EncodingProperty#allowReserved is not declared in types3.yaml and, thus, doesn't exist as an entry pair in Map<String, JsonOverlay<?>> children in PropertiesOverlay which is used during validation:

  1. EncodingPropertyValidator.runObjectValidations() executes validateBooleanField(F_allowReserved, false);
  2. Which calls ValidatorBase.validateField(String, boolean, Class<F>, Validator<F>, Consumer<Overlay<F>>...) which executes
    Overlay<F> field = Overlay.of(propValue, name, fieldClass);
    checkJsonType(field, getAllowedJsonTypes(field), results);
  3. The field will have the null value for overlay as Overlay.of() will return null because it calls props._getOverlay(fieldName) which will delegate it to the children which doesn't have an entry for allowReserved
tfesenko commented 6 years ago

@andylowry , I did NOT commit the generated classes because I am getting a big diff. Every file in ovl3 and model3 packages is modified. Let's discuss how to generate OAS3 classes, e.g. which version of JsonOverlay to use.

andylowry commented 6 years ago

@tfesenko The generator does, generally, change every single generated class, even when the meaningful changes are limited to one or a few files. There are three common reasons:

  1. The generator adds imports to each source file based on a very conservative - and simple - strategy. So many of the files end up with unneeded, but harmless, imports.

  2. Even if the unneeded imports were not there, the other imports are not generated in an order that corresponds to Eclipse's "Organize imports" ordering.

  3. The generator does not generate Java with formatting that adheres to the default Eclipse formatting (which is what's used - not our formatting rules from API Studio).

My general process when regenerating the code is to follow up with "Organize Imports" and "Format" performed in Eclipse. These can easily be done for the entire project by selecting the project in Project Explorer and then invoking the operations from the context menu.

If you do this, you will probably see much smaller diffs.

BTW - there's one other "less common" reason that generation can change files you didn't expect. Suppose you manually edit one of the generated sources to add a method without "@Generated" annotation. Then you save that file and check it in, with proper formatting.

The next generation will most likely change that file, because the generator copies all the non-generated members from the current source file to the top of the newly generated version. Any intermingling of generated and non-generated members does not survive generation.

tfesenko commented 6 years ago

Thanks for the instructions, @andylowry ! I've regenerated the classes.

andylowry commented 6 years ago

@tfesenko Looks good.

Just to be 100% certain that things are in good shape, I reverted your regen/reformatting commits and then did my own. The correct formatter to use in JOvl and KZOP projects is the "Eclipse [built-in]" profile in eclipse.