Skip to content

Commit

Permalink
scrooge: generate a method to validate request and return the violati…
Browse files Browse the repository at this point in the history
…ons in Java

Problem/Solution
For each method defined in a service in the Thrift IDL, if any args in the
method contain validation annotations (starts with `validation.`), e.g.

```
struct Request {
  1: string email (validation.email = "")
}

service S {
  bool process (
    1: Request request // the arg of type Request has validation annotation
  )
}
```

generate a new interface `ServerValidationMixin` that extends
`ServiceIface`. The new interface leaves all APIs in `ServiceIface`
unchanged, and define and implement new APIs named
`violationReturning<method_name>`.

The new API will validate incoming request (of Struct, Union, Exception
types) by invoking the `ValidateInstanceValue` method on the request,
and return any violations (as method parameters <request_arg>Violations)
back to the users in the method API.

When there is validation annotation on the IDL, and user doesn't override
the implementation of the new method `violationReturning<method_name>`,
any call to the original method `method_name` will execute the implementation
of the method `method_name`, and throw an exception for invalid request; if
user implement the new method `violationReturning<method_name>`, and
any request arg failed validations, the call to the original method will execute
the implementation of the new method. **Please note there is no new RPC
method introduced, users can only issue request to the original method, we
will choose which method to execute based on user's implementation of both
methods.**

**Added a flag hasValidationAnnotation in the parser, all new APIs will only be
generated when the flag is true. There is no runtime behavior change on
existing services.**

Result
When `violationReturning<method_name>` is unimplemented, the call to
`<method_name>` will use the implementation of `<method_name>`,
and throw an exception when any request fails validation.

When `violationReturning<method_name>` is implemented, the call to
`<method_name>` will use the implementation of
`violationReturning<method_name>`, where no exception is thrown per
request validation violations.

JIRA Issues: CSL-11151

Differential Revision: https://phabricator.twitter.biz/D840524
  • Loading branch information
jyanJing authored and jenkins committed Mar 4, 2022
1 parent 79dd066 commit aab9146
Show file tree
Hide file tree
Showing 27 changed files with 3,154 additions and 104 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.rst
Expand Up @@ -7,6 +7,15 @@ Note that ``PHAB_ID=#`` and ``RB_ID=#`` correspond to associated messages in com
Unreleased
----------

New Features
~~~~~~~~~~~~

* scrooge-generator: for each method defined in a service in the Thrift IDL, if any request arg
of a method has annotations started with `validation.`, in Java template, generate a new trait
`ServerValidationMixin` with a new API `violationReturning<method_name>` which validates incoming
request (of Struct, Union, Exception types) and return any violations (as method parameters
`<request_variable>Violations`) back to the users in the method API. ``PHAB_ID=D840524``

22.2.0
------

Expand Down
Expand Up @@ -18,6 +18,15 @@ enum RequestType {
Read = 2,
} (enum.annotation = "false")

union RequestUnion {
1: i32 id (validation.positiveOrZero = "")
2: string name (validation.notEmpty = "")
}

exception RequestException {
1: string message (validation.notEmpty = "")
}

union ResponseUnion {
1: i64 id
2: string details (u.field.annotation = "x")
Expand Down Expand Up @@ -84,6 +93,8 @@ service GoldService {
/** Hello, I'm a comment. */
Response doGreatThings(
1: Request request
2: RequestUnion unionRequest
3: RequestException exceptionRequest
) throws (
1: OverCapacityException ex
) (some.annotation = "false")
Expand Down
@@ -0,0 +1,324 @@
/**
* Autogenerated by Scrooge
*
* DO NOT EDIT UNLESS YOU ARE SURE THAT YOU KNOW WHAT YOU ARE DOING
*/
package com.twitter.scrooge.test.gold.thriftandroid;

import java.util.List;
import java.util.ArrayList;
import java.util.Map;
import java.util.HashMap;
import java.util.EnumMap;
import java.util.Set;
import java.util.HashSet;
import java.util.EnumSet;
import java.util.Collections;
import java.util.BitSet;
import java.nio.ByteBuffer;
import java.util.Arrays;

import org.apache.thrift.*;
import org.apache.thrift.meta_data.*;
import org.apache.thrift.transport.*;
import org.apache.thrift.protocol.*;

// No additional import required for struct/union.

public class RequestException extends Exception implements TBase<RequestException, RequestException._Fields>, java.io.Serializable, Cloneable {
private static final TStruct STRUCT_DESC = new TStruct("RequestException");

private static final TField MESSAGE_FIELD_DESC = new TField("message", TType.STRING, (short)1);


private String message;

/** The set of fields this struct contains, along with convenience methods for finding and manipulating them. */
public enum _Fields implements TFieldIdEnum {
MESSAGE((short)1, "message");

private static final Map<String, _Fields> byName = new HashMap<String, _Fields>();

static {
for (_Fields field : EnumSet.allOf(_Fields.class)) {
byName.put(field.getFieldName(), field);
}
}

/**
* Find the _Fields constant that matches fieldId, or null if its not found.
*/
public static _Fields findByThriftId(int fieldId) {
switch(fieldId) {
case 1: // MESSAGE
return MESSAGE;
default:
return null;
}
}

/**
* Find the _Fields constant that matches fieldId, throwing an exception
* if it is not found.
*/
public static _Fields findByThriftIdOrThrow(int fieldId) {
_Fields fields = findByThriftId(fieldId);
if (fields == null) throw new IllegalArgumentException("Field " + fieldId + " doesn't exist!");
return fields;
}

private final short _thriftId;
private final String _fieldName;

_Fields(short thriftId, String fieldName) {
_thriftId = thriftId;
_fieldName = fieldName;
}

public short getThriftFieldId() {
return _thriftId;
}

public String getFieldName() {
return _fieldName;
}
}


// isset id assignments

public static final Map<_Fields, FieldMetaData> metaDataMap;
static {
Map<_Fields, FieldMetaData> tmpMap = new EnumMap<_Fields, FieldMetaData>(_Fields.class);
tmpMap.put(_Fields.MESSAGE, new FieldMetaData("message", TFieldRequirementType.DEFAULT,
new FieldValueMetaData(TType.STRING)));
metaDataMap = Collections.unmodifiableMap(tmpMap);
FieldMetaData.addStructMetaDataMap(RequestException.class, metaDataMap);
}


public RequestException() {
}

public RequestException(
String message
) {
this();
if(message != null) {
this.message = message;
}
}


/**
* Performs a deep copy on <i>other</i>.
*/
public RequestException(RequestException other) {
if (other.isSet(_Fields.MESSAGE)) {
this.message = other.message;
}
}

public static List<String> validateNewInstance(RequestException item) {
final List<String> buf = new ArrayList<String>();
return buf;
}

public RequestException deepCopy() {
return new RequestException(this);
}

@java.lang.Override
public void clear() {
this.message = null;
}



@SuppressWarnings("unchecked")
public void setFieldValue(_Fields field, Object value) {
switch (field) {
case MESSAGE:
if (value == null) {
this.message = null;
} else {
this.message = (String) value;
}
break;
}
}

public Object getFieldValue(_Fields field) {
switch (field) {
case MESSAGE:
return this.message;
}
throw new IllegalStateException();
}

@SuppressWarnings("unchecked")
public <Any> Any get(_Fields field) {
switch(field) {
case MESSAGE:
Any rval_message = (Any)((String) getFieldValue(field));
return rval_message;
default:
throw new IllegalStateException("Invalid field type");
}
}

/** Returns true if field corresponding to field is set (has been assigned a value) and false otherwise */
public boolean isSet(_Fields field) {
switch (field) {
case MESSAGE:
return message != null;
}
throw new IllegalStateException();
}

@java.lang.Override
public boolean equals(Object that) {
if (that == null)
return false;
if (that instanceof RequestException)
return this.equals((RequestException)that);
return false;
}

public boolean equals(RequestException that) {
if (that == null)
return false;
boolean this_present_message = true && this.isSet(_Fields.MESSAGE);
boolean that_present_message = true && that.isSet(_Fields.MESSAGE);
if (this_present_message || that_present_message) {
if (!(this_present_message && that_present_message))
return false;
if (!this.message.equals(that.message))
return false;
}

return true;
}

@java.lang.Override
@SuppressWarnings("unchecked")
public int hashCode() {
int hashCode = 1;
if (true && (isSet(_Fields.MESSAGE))) {
hashCode = 31 * hashCode + message.hashCode();
}
return hashCode;
}

public int compareTo(RequestException other) {
if (!getClass().equals(other.getClass())) {
return getClass().getName().compareTo(other.getClass().getName());
}

int lastComparison = 0;
RequestException typedOther = (RequestException)other;

lastComparison = Boolean.valueOf(isSet(_Fields.MESSAGE)).compareTo(typedOther.isSet(_Fields.MESSAGE));
if (lastComparison != 0) {
return lastComparison;
}
if (isSet(_Fields.MESSAGE)) {
lastComparison = TBaseHelper.compareTo(this.message, typedOther.message);
if (lastComparison != 0) {
return lastComparison;
}
}
return 0;
}

public _Fields fieldForId(int fieldId) {
return _Fields.findByThriftId(fieldId);
}


public void read(TProtocol iprot) throws TException {
TField field;
iprot.readStructBegin();
while (true)
{
field = iprot.readFieldBegin();
if (field.type == TType.STOP) {
break;
}
switch (field.id) {
case 1: // MESSAGE
if (field.type == TType.STRING) {
this.message = iprot.readString();
} else {
TProtocolUtil.skip(iprot, field.type);
}
break;
default:
TProtocolUtil.skip(iprot, field.type);
}
iprot.readFieldEnd();
}
iprot.readStructEnd();

// check for required fields of primitive type, which can't be checked in the validate method
validate();
}

public void write(TProtocol oprot) throws TException {
validate();

oprot.writeStructBegin(STRUCT_DESC);
if (this.message != null) {
oprot.writeFieldBegin(MESSAGE_FIELD_DESC);
oprot.writeString(this.message);
oprot.writeFieldEnd();
}
oprot.writeFieldStop();
oprot.writeStructEnd();
}

@java.lang.Override
public String toString() {
StringBuilder sb = new StringBuilder("RequestException(");
boolean first = true;
sb.append("message:");
if (this.message == null) {
sb.append("null");
} else {
sb.append(this.message);
}
first = false;
sb.append(")");
return sb.toString();
}

public void validate() throws TException {
// check for required fields
}

public static final _Fields MESSAGE = _Fields.MESSAGE;

public static class Builder {
private String message;
@SuppressWarnings("unchecked")
public Builder set (_Fields field, Object value) {
switch(field) {
case MESSAGE: {
if (value != null) {
this.message = (String) value;
}
break;
}
default: {
break;
}
}
return this;
}
public RequestException build() {
// check for required fields
return new RequestException(message);
}
}
}

0 comments on commit aab9146

Please sign in to comment.