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

MaxAsyncAfterSeconds Default Value Assignment #1960

Closed
wants to merge 4 commits into from
Closed
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 @@ -9,6 +9,8 @@
import static com.yahoo.elide.annotation.LifeCycleHookBinding.Operation.READ;
import static com.yahoo.elide.annotation.LifeCycleHookBinding.TransactionPhase.POSTCOMMIT;
import static com.yahoo.elide.annotation.LifeCycleHookBinding.TransactionPhase.PRESECURITY;

import com.yahoo.elide.ElideSettings;
import com.yahoo.elide.annotation.LifeCycleHookBinding;
import com.yahoo.elide.async.models.AsyncAPI;
import com.yahoo.elide.async.models.AsyncAPIResult;
Expand All @@ -30,11 +32,11 @@
@Data
public abstract class AsyncAPIHook<T extends AsyncAPI> implements LifeCycleHook<T> {
private final AsyncExecutorService asyncExecutorService;
private final Integer maxAsyncAfterSeconds;
private final ElideSettings elideSettings;

public AsyncAPIHook(AsyncExecutorService asyncExecutorService, Integer maxAsyncAfterSeconds) {
public AsyncAPIHook(AsyncExecutorService asyncExecutorService, ElideSettings elideSettings) {
this.asyncExecutorService = asyncExecutorService;
this.maxAsyncAfterSeconds = maxAsyncAfterSeconds;
this.elideSettings = elideSettings;
}

/**
Expand All @@ -44,7 +46,7 @@ public AsyncAPIHook(AsyncExecutorService asyncExecutorService, Integer maxAsyncA
* @throws InvalidValueException InvalidValueException
*/
protected void validateOptions(AsyncAPI query, RequestScope requestScope) {
if (query.getAsyncAfterSeconds() > maxAsyncAfterSeconds) {
if (query.getAsyncAfterSeconds() > elideSettings.getMaxAsyncAfterSeconds()) {
throw new InvalidValueException("Invalid Async After Seconds");
}
}
Expand All @@ -58,12 +60,29 @@ protected void validateOptions(AsyncAPI query, RequestScope requestScope) {
*/
protected void executeHook(LifeCycleHookBinding.Operation operation, LifeCycleHookBinding.TransactionPhase phase,
AsyncAPI query, RequestScope requestScope, Callable<AsyncAPIResult> queryWorker) {

moizarafat marked this conversation as resolved.
Show resolved Hide resolved
if (operation.equals(READ) && phase.equals(PRESECURITY)) {
// AsyncAfterSeconds is a transient property. When we update the STATUS of an AsyncRequest for
// CANCELLATION we do not have the property set. This fails the "validateOptions" method call.
// Below code sets the value based on ElideSettings if its null.
if (query.getAsyncAfterSeconds() == null) {
query.setAsyncAfterSeconds(elideSettings.getMaxAsyncAfterSeconds());
}

validateOptions(query, requestScope);
//We populate the result object when the initial mutation is executed, and then even after executing
//the hooks we return the same object back. QueryRunner.java#L190.
//In GraphQL, the only part of the body that is lazily returned is the ID.
//ReadPreSecurityHook - Those hooks get evaluated in line with the request processing.

// Grapqhl AsyncRequest which were submitted with non-zero asyncAfterSeconds were not returning correct
// Status. They came back with status QUEUED instead of COMPLETE.
// Root Cause:
// We populate the result object when the initial mutation is executed, and then even after executing
// the hooks we return the same object back. https://github.com/yahoo/elide/blob/
// d4901c6f07e57aa179c5afd640c9c67e90a8cdaf/elide-graphql/src/main/java/com/yahoo/elide/graphql/
// QueryRunner.java#L190. In GraphQL, the only part of the body that is lazily returned is the ID.
// Solution:
// We decided to implement the execution flow as a ReadPreSecurityHook.
// Those hooks get evaluated in line with the request processing.
// Every time its read, we check if the STATUS is Queued AND Result is NULL, then we continue with the
// Asynchronous execution call else we exit from the async flow.
executeAsync(query, queryWorker);
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
package com.yahoo.elide.async.hooks;

import com.yahoo.elide.ElideSettings;
import com.yahoo.elide.annotation.LifeCycleHookBinding.Operation;
import com.yahoo.elide.annotation.LifeCycleHookBinding.TransactionPhase;
import com.yahoo.elide.async.models.AsyncAPI;
Expand All @@ -27,8 +28,8 @@
*/
public class AsyncQueryHook extends AsyncAPIHook<AsyncQuery> {

public AsyncQueryHook (AsyncExecutorService asyncExecutorService, Integer maxAsyncAfterSeconds) {
super(asyncExecutorService, maxAsyncAfterSeconds);
public AsyncQueryHook (AsyncExecutorService asyncExecutorService, ElideSettings elideSettings) {
super(asyncExecutorService, elideSettings);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
package com.yahoo.elide.async.hooks;

import com.yahoo.elide.ElideSettings;
import com.yahoo.elide.annotation.LifeCycleHookBinding.Operation;
import com.yahoo.elide.annotation.LifeCycleHookBinding.TransactionPhase;
import com.yahoo.elide.async.export.formatter.TableExportFormatter;
Expand Down Expand Up @@ -32,9 +33,9 @@ public class TableExportHook extends AsyncAPIHook<TableExport> {
Map<ResultType, TableExportFormatter> supportedFormatters;
ResultStorageEngine engine;

public TableExportHook (AsyncExecutorService asyncExecutorService, Integer maxAsyncAfterSeconds,
public TableExportHook (AsyncExecutorService asyncExecutorService, ElideSettings settings,
Map<ResultType, TableExportFormatter> supportedFormatters, ResultStorageEngine engine) {
super(asyncExecutorService, maxAsyncAfterSeconds);
super(asyncExecutorService, settings);
this.supportedFormatters = supportedFormatters;
this.engine = engine;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public abstract class AsyncAPI implements PrincipalOwned {

@Transient
@ComputedAttribute
private Integer asyncAfterSeconds = 10;
private Integer asyncAfterSeconds;

/**
* Set Async API Result.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,5 @@ public class ElideSettings {
@Getter private final String jsonApiPath;
@Getter private final String graphQLApiPath;
@Getter private final String exportApiPath;
@Getter private final int maxAsyncAfterSeconds;
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public class ElideSettingsBuilder {
private String jsonApiPath;
private String graphQLApiPath;
private String exportApiPath;
private int maxAsyncAfterSeconds = 10;

/**
* A new builder used to generate Elide instances. Instantiates an {@link EntityDictionary} without
Expand Down Expand Up @@ -112,7 +113,8 @@ public ElideSettings build() {
baseUrl,
jsonApiPath,
graphQLApiPath,
exportApiPath);
exportApiPath,
maxAsyncAfterSeconds);
}

public ElideSettingsBuilder withAuditLogger(AuditLogger auditLogger) {
Expand Down Expand Up @@ -221,4 +223,9 @@ public ElideSettingsBuilder withStrictQueryParams(boolean enabled) {
this.strictQueryParams = enabled;
return this;
}

public ElideSettingsBuilder withMaxAsyncAfterSeconds(int maxAsyncAfterSeconds) {
this.maxAsyncAfterSeconds = maxAsyncAfterSeconds;
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ protected void configure() {
.withEntityDictionary(dictionary)
.withISO8601Dates("yyyy-MM-dd'T'HH:mm'Z'", Calendar.getInstance().getTimeZone())
.withExportApiPath("/export")
.withMaxAsyncAfterSeconds(10)
.build());
bind(elide).to(Elide.class).named("elide");

Expand All @@ -122,8 +123,8 @@ protected void configure() {
supportedFormatters.put(ResultType.JSON, new JSONExportFormatter(elide));

// Binding TableExport LifeCycleHook
TableExportHook tableExportHook = new TableExportHook(asyncExecutorService, 10, supportedFormatters,
resultStorageEngine);
TableExportHook tableExportHook = new TableExportHook(asyncExecutorService, elide.getElideSettings(),
supportedFormatters, resultStorageEngine);
dictionary.bindTrigger(TableExport.class, READ, PRESECURITY, tableExportHook, false);
dictionary.bindTrigger(TableExport.class, CREATE, POSTCOMMIT, tableExportHook, false);
dictionary.bindTrigger(TableExport.class, CREATE, PRESECURITY, tableExportHook, false);
Expand All @@ -142,7 +143,7 @@ public long purchase(Invoice invoice) {
bind(billingService).to(BillingService.class);

// Binding AsyncQuery LifeCycleHook
AsyncQueryHook asyncQueryHook = new AsyncQueryHook(asyncExecutorService, 10);
AsyncQueryHook asyncQueryHook = new AsyncQueryHook(asyncExecutorService, elide.getElideSettings());

InvoiceCompletionHook invoiceCompletionHook = new InvoiceCompletionHook(billingService);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import static com.yahoo.elide.annotation.LifeCycleHookBinding.TransactionPhase.PRESECURITY;

import com.yahoo.elide.Elide;
import com.yahoo.elide.ElideSettings;
import com.yahoo.elide.async.export.formatter.CSVExportFormatter;
import com.yahoo.elide.async.export.formatter.JSONExportFormatter;
import com.yahoo.elide.async.export.formatter.TableExportFormatter;
Expand Down Expand Up @@ -74,8 +75,7 @@ public AsyncExecutorService buildAsyncExecutorService(Elide elide, ElideConfigPr
AsyncExecutorService asyncExecutorService = new AsyncExecutorService(elide, executor, updater, asyncQueryDao);

// Binding AsyncQuery LifeCycleHook
AsyncQueryHook asyncQueryHook = new AsyncQueryHook(asyncExecutorService,
asyncProperties.getMaxAsyncAfterSeconds());
AsyncQueryHook asyncQueryHook = new AsyncQueryHook(asyncExecutorService, elide.getElideSettings());
dictionary.bindTrigger(AsyncQuery.class, READ, PRESECURITY, asyncQueryHook, false);
dictionary.bindTrigger(AsyncQuery.class, CREATE, POSTCOMMIT, asyncQueryHook, false);
dictionary.bindTrigger(AsyncQuery.class, CREATE, PRESECURITY, asyncQueryHook, false);
Expand Down Expand Up @@ -109,14 +109,14 @@ private TableExportHook getTableExportHook(AsyncExecutorService asyncExecutorSer
ElideConfigProperties settings, Map<ResultType, TableExportFormatter> supportedFormatters,
ResultStorageEngine resultStorageEngine) {
boolean exportEnabled = ElideAutoConfiguration.isExportEnabled(settings.getAsync());

ElideSettings elideSettings = asyncExecutorService.getElide().getElideSettings();
TableExportHook tableExportHook = null;
if (exportEnabled) {
tableExportHook = new TableExportHook(asyncExecutorService, settings.getAsync().getMaxAsyncAfterSeconds(),
supportedFormatters, resultStorageEngine);
tableExportHook = new TableExportHook(asyncExecutorService, elideSettings, supportedFormatters,
resultStorageEngine);
} else {
tableExportHook = new TableExportHook(asyncExecutorService, settings.getAsync().getMaxAsyncAfterSeconds(),
supportedFormatters, resultStorageEngine) {
tableExportHook = new TableExportHook(asyncExecutorService, elideSettings, supportedFormatters,
resultStorageEngine) {
@Override
public void validateOptions(AsyncAPI export, RequestScope requestScope) {
throw new InvalidOperationException("TableExport is not supported.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,12 @@ public Elide initializeElide(EntityDictionary dictionary,
.withJsonApiPath(settings.getJsonApi().getPath())
.withGraphQLApiPath(settings.getGraphql().getPath());

if (settings.getAsync() != null
&& settings.getAsync().getExport() != null
&& settings.getAsync().getExport().isEnabled()) {
builder.withExportApiPath(settings.getAsync().getExport().getPath());
if (settings.getAsync() != null) {
builder.withMaxAsyncAfterSeconds(settings.getAsync().getMaxAsyncAfterSeconds());

if (settings.getAsync().getExport() != null && settings.getAsync().getExport().isEnabled()) {
builder.withExportApiPath(settings.getAsync().getExport().getPath());
}
}

if (settings.getJsonApi() != null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,7 @@ protected void configure() {
}

// Binding AsyncQuery LifeCycleHook
AsyncQueryHook asyncQueryHook = new AsyncQueryHook(asyncExecutorService,
asyncProperties.getMaxAsyncAfterSeconds());
AsyncQueryHook asyncQueryHook = new AsyncQueryHook(asyncExecutorService, elideSettings);

dictionary.bindTrigger(AsyncQuery.class, READ, PRESECURITY, asyncQueryHook, false);
dictionary.bindTrigger(AsyncQuery.class, CREATE, POSTCOMMIT, asyncQueryHook, false);
Expand Down Expand Up @@ -272,13 +271,12 @@ public static HealthCheckRegistry getHealthCheckRegistry() {
private TableExportHook getTableExportHook(AsyncExecutorService asyncExecutorService,
ElideStandaloneAsyncSettings asyncProperties, Map<ResultType, TableExportFormatter> supportedFormatters,
ResultStorageEngine engine) {
ElideSettings elideSettings = asyncExecutorService.getElide().getElideSettings();
TableExportHook tableExportHook = null;
if (asyncProperties.enableExport()) {
tableExportHook = new TableExportHook(asyncExecutorService, asyncProperties.getMaxAsyncAfterSeconds(),
supportedFormatters, engine);
tableExportHook = new TableExportHook(asyncExecutorService, elideSettings, supportedFormatters, engine);
} else {
tableExportHook = new TableExportHook(asyncExecutorService, asyncProperties.getMaxAsyncAfterSeconds(),
supportedFormatters, engine) {
tableExportHook = new TableExportHook(asyncExecutorService, elideSettings, supportedFormatters, engine) {
@Override
public void validateOptions(AsyncAPI export, RequestScope requestScope) {
throw new InvalidOperationException("TableExport is not supported.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ default ElideSettings getElideSettings(EntityDictionary dictionary, DataStore da
builder.withExportApiPath(getAsyncProperties().getExportApiPathSpec().replaceAll("/\\*", ""));
}

if (getAsyncProperties().getMaxAsyncAfterSeconds() != null) {
builder.withMaxAsyncAfterSeconds(getAsyncProperties().getMaxAsyncAfterSeconds());
}

if (enableISO8601Dates()) {
builder.withISO8601Dates("yyyy-MM-dd'T'HH:mm'Z'", TimeZone.getTimeZone("UTC"));
}
Expand Down