Skip to content

Commit 96dfe1d

Browse files
shijieshengshijiesheng
authored
Fix(RequestMapper): Correctly map all fields (#1000)
What changed? GRPC fields are set correctly if it's set in Thrift Why? Fix #998 How did you test it? unit test
1 parent aaf9d1d commit 96dfe1d

File tree

4 files changed

+71
-13
lines changed

4 files changed

+71
-13
lines changed

‎src/main/java/com/uber/cadence/internal/compatibility/proto/RequestMapper.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ public static RequestCancelWorkflowExecutionRequest requestCancelWorkflowExecuti
169169
.setDomain(t.getDomain())
170170
.setWorkflowExecution(workflowExecution(t.getWorkflowExecution()))
171171
.setRequestId(t.getRequestId());
172+
if (t.getCause() != null) {
173+
builder.setCause(t.getCause());
174+
}
175+
if (t.getFirstExecutionRunID() != null) {
176+
builder.setFirstExecutionRunId(t.getFirstExecutionRunID());
177+
}
172178
if (t.getIdentity() != null) {
173179
builder.setIdentity(t.getIdentity());
174180
}
@@ -437,7 +443,8 @@ public static SignalWithStartWorkflowExecutionRequest signalWithStartWorkflowExe
437443
.setRequestId(t.getRequestId())
438444
.setMemo(memo(t.getMemo()))
439445
.setSearchAttributes(searchAttributes(t.getSearchAttributes()))
440-
.setHeader(header(t.getHeader()));
446+
.setHeader(header(t.getHeader()))
447+
.setJitterStart(secondsToDuration(t.getJitterStartSeconds()));
441448
if (t.getRetryPolicy() != null) {
442449
builder.setRetryPolicy(retryPolicy(t.getRetryPolicy()));
443450
}
@@ -521,7 +528,8 @@ public static StartWorkflowExecutionRequest startWorkflowExecutionRequest(
521528
.setMemo(memo(t.getMemo()))
522529
.setSearchAttributes(searchAttributes(t.getSearchAttributes()))
523530
.setHeader(header(t.getHeader()))
524-
.setDelayStart(secondsToDuration(t.getDelayStartSeconds()));
531+
.setDelayStart(secondsToDuration(t.getDelayStartSeconds()))
532+
.setJitterStart(secondsToDuration(t.getJitterStartSeconds()));
525533
if (t.getRetryPolicy() != null) {
526534
request.setRetryPolicy(retryPolicy(t.getRetryPolicy()));
527535
}
@@ -561,6 +569,9 @@ public static TerminateWorkflowExecutionRequest terminateWorkflowExecutionReques
561569
if (t.getIdentity() != null) {
562570
builder.setIdentity(t.getIdentity());
563571
}
572+
if (t.getFirstExecutionRunID() != null) {
573+
builder.setFirstExecutionRunId(t.getFirstExecutionRunID());
574+
}
564575
return builder.build();
565576
}
566577

‎src/test/java/com/uber/cadence/internal/compatibility/ProtoObjects.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,6 +874,16 @@ public final class ProtoObjects {
874874
.setRequestId("requestId")
875875
.setIdentity("identity")
876876
.build();
877+
public static final RequestCancelWorkflowExecutionRequest
878+
REQUEST_CANCEL_WORKFLOW_EXECUTION_REQUEST_FULL =
879+
RequestCancelWorkflowExecutionRequest.newBuilder()
880+
.setDomain("domain")
881+
.setWorkflowExecution(WORKFLOW_EXECUTION)
882+
.setRequestId("requestId")
883+
.setIdentity("identity")
884+
.setFirstExecutionRunId("firstExecutionRunID")
885+
.setCause("cancel cause")
886+
.build();
877887
public static final ResetStickyTaskListRequest RESET_STICKY_TASK_LIST_REQUEST =
878888
ResetStickyTaskListRequest.newBuilder()
879889
.setDomain("domain")
@@ -1005,6 +1015,7 @@ public final class ProtoObjects {
10051015
.setSearchAttributes(SEARCH_ATTRIBUTES)
10061016
.setHeader(HEADER)
10071017
.setDelayStart(seconds(3))
1018+
.setJitterStart(seconds(0))
10081019
.build();
10091020

10101021
public static final SignalWithStartWorkflowExecutionRequest SIGNAL_WITH_START_WORKFLOW_EXECUTION =
@@ -1044,6 +1055,16 @@ public final class ProtoObjects {
10441055
.setIdentity("identity")
10451056
.build();
10461057

1058+
public static final TerminateWorkflowExecutionRequest TERMINATE_WORKFLOW_EXECUTION_REQUEST_FULL =
1059+
TerminateWorkflowExecutionRequest.newBuilder()
1060+
.setDomain("domain")
1061+
.setWorkflowExecution(WORKFLOW_EXECUTION)
1062+
.setReason("reason")
1063+
.setDetails(payload("details"))
1064+
.setIdentity("identity")
1065+
.setFirstExecutionRunId("firstExecutionRunID")
1066+
.build();
1067+
10471068
public static final DeprecateDomainRequest DEPRECATE_DOMAIN_REQUEST =
10481069
DeprecateDomainRequest.newBuilder()
10491070
.setName("domain")

‎src/test/java/com/uber/cadence/internal/compatibility/ThriftObjects.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,15 @@ public final class ThriftObjects {
746746
.setWorkflowExecution(WORKFLOW_EXECUTION)
747747
.setRequestId("requestId")
748748
.setIdentity("identity");
749+
public static final RequestCancelWorkflowExecutionRequest
750+
REQUEST_CANCEL_WORKFLOW_EXECUTION_REQUEST_FULL =
751+
new RequestCancelWorkflowExecutionRequest()
752+
.setDomain("domain")
753+
.setWorkflowExecution(WORKFLOW_EXECUTION)
754+
.setRequestId("requestId")
755+
.setIdentity("identity")
756+
.setFirstExecutionRunID("firstExecutionRunID")
757+
.setCause("cancel cause");
749758
public static final ResetStickyTaskListRequest RESET_STICKY_TASK_LIST_REQUEST =
750759
new ResetStickyTaskListRequest().setDomain("domain").setExecution(WORKFLOW_EXECUTION);
751760
public static final ResetWorkflowExecutionRequest RESET_WORKFLOW_EXECUTION_REQUEST =
@@ -863,6 +872,7 @@ public final class ThriftObjects {
863872
.setMemo(MEMO)
864873
.setSearchAttributes(SEARCH_ATTRIBUTES)
865874
.setHeader(HEADER)
875+
.setJitterStartSeconds(0)
866876
.setDelayStartSeconds(3);
867877
public static final com.uber.cadence.SignalWithStartWorkflowExecutionRequest
868878
SIGNAL_WITH_START_WORKFLOW_EXECUTION =
@@ -885,7 +895,8 @@ public final class ThriftObjects {
885895
.setMemo(MEMO)
886896
.setSearchAttributes(SEARCH_ATTRIBUTES)
887897
.setHeader(HEADER)
888-
.setDelayStartSeconds(3);
898+
.setDelayStartSeconds(3)
899+
.setJitterStartSeconds(0);
889900

890901
public static final StartWorkflowExecutionAsyncRequest START_WORKFLOW_EXECUTION_ASYNC_REQUEST =
891902
new StartWorkflowExecutionAsyncRequest().setRequest(START_WORKFLOW_EXECUTION);
@@ -913,6 +924,15 @@ public final class ThriftObjects {
913924
.setDetails(utf8("details"))
914925
.setIdentity("identity");
915926

927+
public static final TerminateWorkflowExecutionRequest TERMINATE_WORKFLOW_EXECUTION_REQUEST_FULL =
928+
new TerminateWorkflowExecutionRequest()
929+
.setDomain("domain")
930+
.setWorkflowExecution(WORKFLOW_EXECUTION)
931+
.setReason("reason")
932+
.setDetails(utf8("details"))
933+
.setIdentity("identity")
934+
.setFirstExecutionRunID("firstExecutionRunID");
935+
916936
public static final DeprecateDomainRequest DEPRECATE_DOMAIN_REQUEST =
917937
new DeprecateDomainRequest().setName("domain").setSecurityToken("securityToken");
918938

‎src/test/java/com/uber/cadence/internal/compatibility/proto/RequestMapperTest.java‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,12 @@ public static Iterable<Object[]> cases() {
9696
ThriftObjects.REQUEST_CANCEL_WORKFLOW_EXECUTION_REQUEST,
9797
ProtoObjects.REQUEST_CANCEL_WORKFLOW_EXECUTION_REQUEST,
9898
RequestMapper::requestCancelWorkflowExecutionRequest,
99-
"firstExecutionRunID",
100-
"cause"),
99+
"firstExecutionRunID", // optional field
100+
"cause"), // optional field
101+
testCase(
102+
ThriftObjects.REQUEST_CANCEL_WORKFLOW_EXECUTION_REQUEST_FULL,
103+
ProtoObjects.REQUEST_CANCEL_WORKFLOW_EXECUTION_REQUEST_FULL,
104+
RequestMapper::requestCancelWorkflowExecutionRequest),
101105
testCase(
102106
ThriftObjects.RESET_STICKY_TASK_LIST_REQUEST,
103107
ProtoObjects.RESET_STICKY_TASK_LIST_REQUEST,
@@ -157,13 +161,11 @@ public static Iterable<Object[]> cases() {
157161
testCase(
158162
ThriftObjects.START_WORKFLOW_EXECUTION,
159163
ProtoObjects.START_WORKFLOW_EXECUTION,
160-
RequestMapper::startWorkflowExecutionRequest,
161-
"jitterStartSeconds"),
164+
RequestMapper::startWorkflowExecutionRequest),
162165
testCase(
163166
ThriftObjects.SIGNAL_WITH_START_WORKFLOW_EXECUTION,
164167
ProtoObjects.SIGNAL_WITH_START_WORKFLOW_EXECUTION,
165-
RequestMapper::signalWithStartWorkflowExecutionRequest,
166-
"jitterStartSeconds"),
168+
RequestMapper::signalWithStartWorkflowExecutionRequest),
167169
testCase(
168170
ThriftObjects.START_WORKFLOW_EXECUTION_ASYNC_REQUEST,
169171
ProtoObjects.START_WORKFLOW_EXECUTION_ASYNC_REQUEST,
@@ -180,7 +182,11 @@ public static Iterable<Object[]> cases() {
180182
ThriftObjects.TERMINATE_WORKFLOW_EXECUTION_REQUEST,
181183
ProtoObjects.TERMINATE_WORKFLOW_EXECUTION_REQUEST,
182184
RequestMapper::terminateWorkflowExecutionRequest,
183-
"firstExecutionRunID"),
185+
"firstExecutionRunID"), // optional field
186+
testCase(
187+
ThriftObjects.TERMINATE_WORKFLOW_EXECUTION_REQUEST_FULL,
188+
ProtoObjects.TERMINATE_WORKFLOW_EXECUTION_REQUEST_FULL,
189+
RequestMapper::terminateWorkflowExecutionRequest),
184190
testCase(
185191
ThriftObjects.DEPRECATE_DOMAIN_REQUEST,
186192
ProtoObjects.DEPRECATE_DOMAIN_REQUEST,
@@ -189,12 +195,12 @@ public static Iterable<Object[]> cases() {
189195
ThriftObjects.DESCRIBE_DOMAIN_BY_ID_REQUEST,
190196
ProtoObjects.DESCRIBE_DOMAIN_BY_ID_REQUEST,
191197
RequestMapper::describeDomainRequest,
192-
"name"),
198+
"name"), // Not needed for query by ID
193199
testCase(
194200
ThriftObjects.DESCRIBE_DOMAIN_BY_NAME_REQUEST,
195201
ProtoObjects.DESCRIBE_DOMAIN_BY_NAME_REQUEST,
196202
RequestMapper::describeDomainRequest,
197-
"uuid"),
203+
"uuid"), // Not needed for query by name
198204
testCase(
199205
ThriftObjects.LIST_DOMAINS_REQUEST,
200206
ProtoObjects.LIST_DOMAINS_REQUEST,
@@ -227,7 +233,7 @@ public static Iterable<Object[]> cases() {
227233
ThriftObjects.REGISTER_DOMAIN_REQUEST,
228234
ProtoObjects.REGISTER_DOMAIN_REQUEST,
229235
RequestMapper::registerDomainRequest,
230-
"emitMetric"),
236+
"emitMetric"), // Thrift has this field but proto doens't have it
231237
testCase(
232238
ThriftObjects.UPDATE_DOMAIN_REQUEST,
233239
// Data and replicationConfiguration are copied incorrectly due to a bug :(

0 commit comments

Comments
 (0)