Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,37 @@ public interface WorkflowOutboundCallsInterceptor {

final class ActivityInput<R> {
private final String activityName;
private final @Nullable String activityId;
private final Class<R> resultClass;
private final Type resultType;
private final Object[] args;
private final ActivityOptions options;
private final Header header;

/**
* @deprecated Kept only for backward compatibility.
*/
@Deprecated
public ActivityInput(
String activityName,
Class<R> resultClass,
Type resultType,
Object[] args,
ActivityOptions options,
Header header) {
this(activityName, null, resultClass, resultType, args, options, header);
}

public ActivityInput(
String activityName,
@Nullable String activityId,
Class<R> resultClass,
Type resultType,
Object[] args,
ActivityOptions options,
Header header) {
this.activityName = activityName;
this.activityId = activityId;
this.resultClass = resultClass;
this.resultType = resultType;
this.args = args;
Expand All @@ -66,6 +83,12 @@ public String getActivityName() {
return activityName;
}

/** Returns the caller-supplied Activity ID, or {@code null} if the SDK should generate one. */
@Nullable
public String getActivityId() {
return activityId;
}

public Class<R> getResultClass() {
return resultClass;
}
Expand Down Expand Up @@ -107,20 +130,37 @@ public Promise<R> getResult() {

final class LocalActivityInput<R> {
private final String activityName;
private final @Nullable String activityId;
private final Class<R> resultClass;
private final Type resultType;
private final Object[] args;
private final LocalActivityOptions options;
private final Header header;

/**
* @deprecated Kept only for backward compatibility.
*/
@Deprecated
public LocalActivityInput(
String activityName,
Class<R> resultClass,
Type resultType,
Object[] args,
LocalActivityOptions options,
Header header) {
this(activityName, null, resultClass, resultType, args, options, header);
}

public LocalActivityInput(
String activityName,
@Nullable String activityId,
Class<R> resultClass,
Type resultType,
Object[] args,
LocalActivityOptions options,
Header header) {
this.activityName = activityName;
this.activityId = activityId;
this.resultClass = resultClass;
this.resultType = resultType;
this.args = args;
Expand All @@ -132,6 +172,12 @@ public String getActivityName() {
return activityName;
}

/** Returns the caller-supplied Activity ID, or {@code null} if the SDK should generate one. */
@Nullable
public String getActivityId() {
return activityId;
}

public Class<R> getResultClass() {
return resultClass;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import io.temporal.activity.ActivityOptions;
import io.temporal.common.MethodRetry;
import io.temporal.common.interceptors.WorkflowOutboundCallsInterceptor;
import io.temporal.workflow.ActivityInvocationOptions;
import io.temporal.workflow.ActivityStub;
import io.temporal.workflow.Functions;
import java.lang.reflect.InvocationHandler;
Expand Down Expand Up @@ -46,7 +47,6 @@ private ActivityInvocationHandler(
@Override
protected Function<Object[], Object> getActivityFunc(
Method method, MethodRetry methodRetry, String activityName) {
Function<Object[], Object> function;
ActivityOptions merged =
ActivityOptions.newBuilder(options)
.mergeActivityOptions(this.activityMethodOptions.get(activityName))
Expand All @@ -58,10 +58,15 @@ protected Function<Object[], Object> getActivityFunc(
+ activityName
+ " activity. Please set at least one of the above through the ActivityStub or WorkflowImplementationOptions.");
}
ActivityInvocationOptions invocationOptions = ActivityInvocationInternal.consumeOptions();
ActivityStub stub = ActivityStubImpl.newInstance(merged, activityExecutor, assertReadOnly);
function =
(a) -> stub.execute(activityName, method.getReturnType(), method.getGenericReturnType(), a);
return function;
return (a) ->
stub.execute(
activityName,
method.getReturnType(),
method.getGenericReturnType(),
invocationOptions,
a);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
package io.temporal.internal.sync;

import io.temporal.workflow.ActivityInvocationOptions;
import io.temporal.workflow.Functions;
import io.temporal.workflow.Promise;
import java.util.Objects;

/** Applies options to one typed Activity proxy invocation. */
final class ActivityInvocationInternal {

private static final ThreadLocal<State> invocation = new ThreadLocal<>();
private static final ActivityInvocationOptions DEFAULT_OPTIONS =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this should be a member of ActivityInvocationOptions like for other option types.

ActivityInvocationOptions.newBuilder().build();

private ActivityInvocationInternal() {}

static ActivityInvocationOptions getDefaultOptions() {
return DEFAULT_OPTIONS;
}

static <R> R invoke(ActivityInvocationOptions options, Functions.Func<R> invocationFunction) {
State state = startInvocation(options, false);
try {
R result = invocationFunction.apply();
state.verifyConsumed();
return result;
} finally {
invocation.remove();
}
}

static void invoke(ActivityInvocationOptions options, Functions.Proc invocationFunction) {
State state = startInvocation(options, false);
try {
invocationFunction.apply();
state.verifyConsumed();
} finally {
invocation.remove();
}
}

static <R> Promise<R> invokeAsync(
ActivityInvocationOptions options, Functions.Proc invocationFunction) {
State state = startInvocation(options, true);
try {
invocationFunction.apply();
return state.getResult();
Comment on lines +45 to +47

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve transformations performed by activity lambdas

When the supplied function is a lambda that transforms the activity result, such as () -> activities.getNumber() + 1, the proxy captures the underlying Activity Promise but returns a default value to the lambda; this call then discards the lambda's computed result and returns the captured promise unchecked. Consequently the example resolves to the raw Activity value rather than the incremented value, and lambdas that dereference object results can throw NullPointerException. Either preserve the function's result semantics or reject anything other than a direct Activity invocation.

Useful? React with 👍 / 👎.

} finally {
invocation.remove();
}
}

private static State startInvocation(ActivityInvocationOptions options, boolean async) {
if (invocation.get() != null) {
throw new IllegalStateException("Already invoking an Activity with invocation options");
}

State state = new State(Objects.requireNonNull(options, "options"), async);
invocation.set(state);
return state;
}

static ActivityInvocationOptions consumeOptions() {
State state = invocation.get();
if (state == null) {
return DEFAULT_OPTIONS;
}
if (state.consumed) {
throw new IllegalStateException("ActivityInvocationOptions can apply to only one invocation");
}
state.consumed = true;
return state.options;
}

static <R> boolean captureResult(Promise<R> result) {
State state = invocation.get();
if (state == null || !state.async) {
return false;
}
if (state.result != null) {
throw new IllegalStateException("ActivityInvocationOptions can apply to only one invocation");
}
state.result = Objects.requireNonNull(result, "result");
return true;
}

private static final class State {
private final ActivityInvocationOptions options;
private final boolean async;
private boolean consumed;
private Promise<?> result;

private State(ActivityInvocationOptions options, boolean async) {
this.options = options;
this.async = async;
}

private void verifyConsumed() {
if (!consumed) {
throw invalidInvocation();
}
}

@SuppressWarnings("unchecked")
private <R> Promise<R> getResult() {
if (!consumed || result == null) {
throw invalidInvocation();
}
return (Promise<R>) result;
}

private IllegalArgumentException invalidInvocation() {
return new IllegalArgumentException(
"activityMethod must invoke an Activity stub created through Workflow.newActivityStub "
+ "or Workflow.newLocalActivityStub");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@

import com.google.common.base.Defaults;
import io.temporal.failure.ActivityFailure;
import io.temporal.workflow.ActivityInvocationOptions;
import io.temporal.workflow.ActivityStub;
import io.temporal.workflow.Promise;
import java.lang.reflect.Type;

/** Supports calling activity by name and arguments without its strongly typed interface. */
/** Supports calling an activity by name and arguments without its strongly typed interface. */
abstract class ActivityStubBase implements ActivityStub {

@Override
Expand All @@ -16,7 +17,46 @@ public <T> T execute(String activityName, Class<T> resultClass, Object... args)

@Override
public <T> T execute(String activityName, Class<T> resultClass, Type resultType, Object... args) {
Promise<T> result = executeAsync(activityName, resultClass, resultType, args);
return getResult(executeAsync(activityName, resultClass, resultType, args), resultClass);
}

@Override
public <R> Promise<R> executeAsync(String activityName, Class<R> resultClass, Object... args) {
return executeAsync(activityName, resultClass, resultClass, args);
}

@Override
public <R> Promise<R> executeAsync(
String activityName, Class<R> resultClass, Type resultType, Object... args) {
return executeAsync(
activityName,
resultClass,
resultType,
ActivityInvocationInternal.getDefaultOptions(),
args);
}

@Override
public <R> R execute(
String activityName,
Class<R> resultClass,
ActivityInvocationOptions options,
Object... args) {
return execute(activityName, resultClass, resultClass, options, args);
}
Comment on lines +39 to +46

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: better to keep overloads of the same method next to each other.


@Override
public abstract <R> R execute(
String activityName,
Class<R> resultClass,
Type resultType,
ActivityInvocationOptions options,
Object... args);

protected <R> R getResult(Promise<R> result, Class<R> resultClass) {
if (ActivityInvocationInternal.captureResult(result)) {
return Defaults.defaultValue(resultClass);
}
if (AsyncInternal.isAsync()) {
AsyncInternal.setAsyncResult(result);
return Defaults.defaultValue(resultClass);
Expand All @@ -33,11 +73,19 @@ public <T> T execute(String activityName, Class<T> resultClass, Type resultType,
}

@Override
public <R> Promise<R> executeAsync(String activityName, Class<R> resultClass, Object... args) {
return executeAsync(activityName, resultClass, resultClass, args);
public <R> Promise<R> executeAsync(
String activityName,
Class<R> resultClass,
ActivityInvocationOptions options,
Object... args) {
return executeAsync(activityName, resultClass, resultClass, options, args);
}

@Override
public abstract <R> Promise<R> executeAsync(
String activityName, Class<R> resultClass, Type resultType, Object... args);
String activityName,
Class<R> resultClass,
Type resultType,
ActivityInvocationOptions options,
Object... args);
}
Loading
Loading