Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add validator used to enforce @clientOptional #1264

Merged
merged 1 commit into from
Jun 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions docs/source/1.0/guides/model-linters.rst
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,83 @@ Configuration
represent time.


.. _MissingClientOptionalTrait:

MissingClientOptionalTrait
==========================

Allows services to control backward compatibility guarantees for
members marked as :ref:`@required <required-trait>` and
:ref:`@default <default-trait>` by requiring the application of the
:ref:`@clientOptional <clientOptional-trait>` trait.

Rationale
Different service providers have different backward compatibility
guarantees for :ref:`@required <required-trait>` and
:ref:`@default <default-trait>` structure members. Some
services wish to reserve the right to remove the ``@required`` trait at
any time, while others are able to strictly follow the backward-compatibility
guarantees of the ``@required`` trait. For example, it is considered
backward compatible to remove the ``@required`` trait from a member and
replace it with the ``@default`` trait. However, this isn't possible for
members that target structure or union shapes because they have no zero
value. The risk associated with such members may be unacceptable for some
services.

Default severity
``DANGER``

Configuration
.. list-table::
:header-rows: 1
:widths: 20 20 60

* - Property
- Type
- Description
* - onRequiredOrDefault
- ``boolean``
- Requires that members marked with the ``@required`` or ``@default``
trait are also marked with the ``@clientOptional`` trait.
* - onRequiredStructureOrUnion
- ``boolean``
- Requires that ``@required`` members that target structure or union
shapes are also marked with the ``@clientOptional`` trait.
``@required`` members that target structures and unions are risky
because there is no backward compatible way to replace the
``@required`` trait with the ``@default`` trait if the member ever
needs to be made optional.

The following example requires that ``@required`` members that target a structure or
union are marked with the ``@clientOptional`` trait.

.. code-block:: smithy

metadata validators = [
{
name: "MissingClientOptionalTrait",

// Limit validation to a specific set of namespaces.
namespaces: ["smithy.example"],

configuration: {
onRequiredStructureOrUnion: true
}
}
]

This validation can be suppressed for any member that the service provider
decides is not at risk of ever needing to become optional in the future:

.. code-block:: smithy

structure Sprocket {
@required
@suppress(["MissingClientOptionalTrait"])
owner: OwnerStructure
}


-------------------------
Writing custom validators
-------------------------
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
* Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.smithy.linters;

import java.util.ArrayList;
import java.util.List;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.node.NodeMapper;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.traits.ClientOptionalTrait;
import software.amazon.smithy.model.traits.DefaultTrait;
import software.amazon.smithy.model.traits.RequiredTrait;
import software.amazon.smithy.model.validation.AbstractValidator;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.model.validation.ValidatorService;

/**
* Validates that the clientOptional trait is applied based on the rules
* defined in the validator.
*/
public final class MissingClientOptionalTrait extends AbstractValidator {

/**
* MissingClientOptionalTrait configuration settings.
*/
public static final class Config {
private boolean onRequiredStructureOrUnion;
private boolean onRequiredOrDefault;

/**
* Whether clientOptional is required for members marked as required that
* target structures or unions (shapes with no zero value, meaning it's
* impossible to later remove the required trait and replace it with the
* default trait).
*
* @return Returns true if required.
*/
public boolean onRequiredStructureOrUnion() {
return onRequiredStructureOrUnion;
}

public void onRequiredStructureOrUnion(boolean onRequiredStructuresOrUnion) {
this.onRequiredStructureOrUnion = onRequiredStructuresOrUnion;
}

/**
* Whether clientOptional is required for all members marked as required or default.
*
* @return Returns true if required.
*/
public boolean onRequiredOrDefault() {
return onRequiredOrDefault;
}

public void onRequiredOrDefault(boolean onDefault) {
this.onRequiredOrDefault = onDefault;
}
}

public static final class Provider extends ValidatorService.Provider {
public Provider() {
super(MissingClientOptionalTrait.class, configuration -> {
NodeMapper mapper = new NodeMapper();
Config config = mapper.deserialize(configuration, MissingClientOptionalTrait.Config.class);
return new MissingClientOptionalTrait(config);
});
}
}

private final Config config;

public MissingClientOptionalTrait(Config config) {
this.config = config;
}

@Override
public List<ValidationEvent> validate(Model model) {
List<ValidationEvent> events = new ArrayList<>();
for (MemberShape member : model.getMemberShapes()) {
if (member.hasTrait(ClientOptionalTrait.class)) {
continue;
}
if (member.hasTrait(DefaultTrait.class) && config.onRequiredOrDefault) {
events.add(danger(member, "@default members must also be marked with the @clientOptional trait"));
}
if (member.hasTrait(RequiredTrait.class)) {
if (config.onRequiredOrDefault) {
events.add(danger(member, "@required members must also be marked with the @clientOptional trait"));
} else if (config.onRequiredStructureOrUnion && isTargetingStructureOrUnion(model, member)) {
events.add(danger(member, "@required members that target a structure or union must be marked with "
+ "the @clientOptional trait. Not using the @clientOptional trait here "
+ "is risky because there is no backward compatible way to replace the "
+ "@required trait with the @default trait if the member ever needs to "
+ "be made optional."));
}
}
}
return events;
}

private boolean isTargetingStructureOrUnion(Model model, MemberShape member) {
Shape target = model.expectShape(member.getTarget());
return target.isStructureShape() || target.isUnionShape();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ software.amazon.smithy.linters.AbbreviationNameValidator$Provider
software.amazon.smithy.linters.CamelCaseValidator$Provider
software.amazon.smithy.linters.NoninclusiveTermsValidator$Provider
software.amazon.smithy.linters.InputOutputStructureReuseValidator$Provider
software.amazon.smithy.linters.MissingClientOptionalTrait$Provider
software.amazon.smithy.linters.MissingPaginatedTraitValidator$Provider
software.amazon.smithy.linters.RepeatedShapeNameValidator$Provider
software.amazon.smithy.linters.ReservedWordsValidator$Provider
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[DANGER] smithy.example#Foo$bar: @default members must also be marked with the @clientOptional trait | MissingClientOptionalTrait
[DANGER] smithy.example#Foo$baz: @required members must also be marked with the @clientOptional trait | MissingClientOptionalTrait
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
$version: "2"

metadata validators = [
{
name: "MissingClientOptionalTrait",
configuration: {
"onRequiredOrDefault": true
}
}
]

namespace smithy.example

structure Foo {
@default
bar: String,

@required
baz: String
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[DANGER] smithy.example#Foo$bam: @required members that target a structure or union must be marked with the @clientOptional trait. Not using the @clientOptional trait here is risky because there is no backward compatible way to replace the @required trait with the @default trait if the member ever needs to be made optional. | MissingClientOptionalTrait
[DANGER] smithy.example#Foo$boo: @required members that target a structure or union must be marked with the @clientOptional trait. Not using the @clientOptional trait here is risky because there is no backward compatible way to replace the @required trait with the @default trait if the member ever needs to be made optional. | MissingClientOptionalTrait
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
$version: "2"

metadata validators = [
{
name: "MissingClientOptionalTrait",
configuration: {
"onRequiredStructureOrUnion": true
}
}
]

namespace smithy.example

structure Foo {
@default
bar: String,

@required
baz: String,

@required
bam: Bam,

@required
boo: Boo
}

structure Bam {}

union Boo {
test: String
}