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.

Sign up for GitHub

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

allow reexecutions only for the owner #2038

Merged
merged 8 commits into from
Aug 27, 2021
Merged

allow reexecutions only for the owner #2038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a0d9ed8
allow reexecutions only for the owner
thoniTUB Aug 20, 2021
877331f
Update AutoDoc
github-actions[bot] Aug 20, 2021
a595f10
adds documentation
thoniTUB Aug 20, 2021
12ede6e
removes dead code
thoniTUB Aug 24, 2021
29861be
creates a copy of the execution if it is not reused by someone else a…
thoniTUB Aug 24, 2021
9a062b7
Merge branch 'develop' into feature/reexecute-only-for-owner
thoniTUB Aug 26, 2021
beed9ab
simplify condition
thoniTUB Aug 27, 2021
bdfca7f
also test that ids are different
thoniTUB Aug 27, 2021
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
24 changes: 14 additions & 10 deletions backend/src/main/java/com/bakdata/conquery/apiv1/QueryProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@
import static com.bakdata.conquery.models.auth.AuthorizationHelper.buildDatasetAbilityMap;

import java.net.URL;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.*;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -68,8 +64,6 @@ public class QueryProcessor {
/**
* Creates a query for all datasets, then submits it for execution on the
* intended dataset.
*
* @return
*/
public ManagedExecution<?> postQuery(Dataset dataset, QueryDescription query, User user) {

Expand Down Expand Up @@ -114,7 +108,7 @@ public ManagedExecution<?> postQuery(Dataset dataset, QueryDescription query, Us
{
final Optional<ManagedExecutionId> executionId = visitors.getInstance(QueryUtils.OnlyReusingChecker.class).getOnlyReused();

final Optional<ManagedExecution<?>> execution = executionId.map(id -> tryReuse(query, id, datasetRegistry, config, executionManager));
final Optional<ManagedExecution<?>> execution = executionId.map(id -> tryReuse(query, id, datasetRegistry, config, executionManager, user));

if (execution.isPresent()) {
return execution.get();
Expand All @@ -136,14 +130,15 @@ public ManagedExecution<?> postQuery(Dataset dataset, QueryDescription query, Us
/**
* Determine if the submitted query does reuse ONLY another query and restart that instead of creating another one.
*/
private ManagedExecution<?> tryReuse(QueryDescription query, ManagedExecutionId executionId, DatasetRegistry datasetRegistry, ConqueryConfig config, ExecutionManager executionManager) {
private ManagedExecution<?> tryReuse(QueryDescription query, ManagedExecutionId executionId, DatasetRegistry datasetRegistry, ConqueryConfig config, ExecutionManager executionManager, User user) {

final ManagedExecution<?> execution = datasetRegistry.getMetaRegistry().resolve(executionId);
ManagedExecution<?> execution = datasetRegistry.getMetaRegistry().resolve(executionId);

if (execution == null) {
return null;
}


// Direct reuse only works if the queries are of the same type (As reuse reconstructs the Query for different types)
if (!query.getClass().equals(execution.getSubmitted().getClass())) {
return null;
Expand All @@ -159,6 +154,15 @@ private ManagedExecution<?> tryReuse(QueryDescription query, ManagedExecutionId
}
}

// If the user is not the owner of the execution, we definitely create a new Execution, so the owner can cancel it
if (!user.isOwner(execution)) {
final ManagedExecution<?> newExecution = executionManager.createExecution(datasetRegistry,execution.getSubmitted(),user,execution.getDataset());
newExecution.setLabel(execution.getLabel());
newExecution.setTags(execution.getTags().clone());
storage.updateExecution(newExecution);
execution = newExecution;
}

ExecutionState state = execution.getState();
if (state.equals(ExecutionState.RUNNING)) {
log.trace("The Execution[{}] was already started and its state is: {}", execution.getId(), state);
Expand Down
44 changes: 41 additions & 3 deletions backend/src/test/java/com/bakdata/conquery/integration/tests/ReusedQueryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.net.URI;
import java.util.List;
import java.util.Map;
import java.util.Set;

import javax.ws.rs.client.Entity;
import javax.ws.rs.core.MediaType;
Expand All @@ -15,6 +16,9 @@
import com.bakdata.conquery.integration.common.LoadingUtil;
import com.bakdata.conquery.integration.json.JsonIntegrationTest;
import com.bakdata.conquery.integration.json.QueryTest;
import com.bakdata.conquery.io.storage.MetaStorage;
import com.bakdata.conquery.models.auth.entities.User;
import com.bakdata.conquery.models.auth.permissions.Ability;
import com.bakdata.conquery.models.common.Range;
import com.bakdata.conquery.models.datasets.concepts.Concept;
import com.bakdata.conquery.models.datasets.concepts.Connector;
Expand All @@ -23,6 +27,7 @@
import com.bakdata.conquery.models.exceptions.ValidatorHelper;
import com.bakdata.conquery.models.execution.ExecutionState;
import com.bakdata.conquery.apiv1.FullExecutionStatus;
import com.bakdata.conquery.models.execution.ManagedExecution;
import com.bakdata.conquery.models.identifiable.CentralRegistry;
import com.bakdata.conquery.models.identifiable.ids.specific.ConceptId;
import com.bakdata.conquery.models.identifiable.ids.specific.ConnectorId;
Expand All @@ -44,6 +49,7 @@
import com.bakdata.conquery.util.support.TestConquery;
import com.github.powerlibraries.io.In;
import lombok.extern.slf4j.Slf4j;
import net.sourceforge.argparse4j.impl.Arguments;


@Slf4j
Expand Down Expand Up @@ -85,7 +91,8 @@ public void execute(String name, TestConquery testConquery) throws Exception {

assertThat(id).isNotNull();

final ManagedQuery execution = (ManagedQuery) conquery.getMetaStorage().getExecution(id);
final MetaStorage metaStorage = conquery.getMetaStorage();
final ManagedQuery execution = (ManagedQuery) metaStorage.getExecution(id);

// Normal reuse
{
Expand Down Expand Up @@ -165,7 +172,7 @@ public void execute(String name, TestConquery testConquery) throws Exception {
reused1.setSecondaryId(query.getSecondaryId());

final ManagedExecutionId reused1Id = IntegrationUtils.assertQueryResult(conquery, reused1, 4L, ExecutionState.DONE, conquery.getTestUser(), 201);
final ManagedQuery execution1 = (ManagedQuery) conquery.getMetaStorage().getExecution(reused1Id);
final ManagedQuery execution1 = (ManagedQuery) metaStorage.getExecution(reused1Id);
{
final SecondaryIdQuery reused2 = new SecondaryIdQuery();
reused2.setRoot(new CQReusedQuery(execution1.getId()));
Expand All @@ -175,7 +182,7 @@ public void execute(String name, TestConquery testConquery) throws Exception {
final ManagedExecutionId
reused2Id =
IntegrationUtils.assertQueryResult(conquery, reused2, 4L, ExecutionState.DONE, conquery.getTestUser(), 201);
final ManagedQuery execution2 = (ManagedQuery) conquery.getMetaStorage().getExecution(reused2Id);
final ManagedQuery execution2 = (ManagedQuery) metaStorage.getExecution(reused2Id);

assertThat(reused2Id)
.as("Query should be reused.")
Expand Down Expand Up @@ -204,6 +211,37 @@ public void execute(String name, TestConquery testConquery) throws Exception {
.as("Query should NOT be reused.")
.isNotEqualTo(reused1Id);
}

{
// Reuse by another user (create a copy of the actual query)

final SecondaryIdQuery reused = new SecondaryIdQuery();
reused.setRoot(new CQReusedQuery(execution.getId()));

reused.setSecondaryId(query.getSecondaryId());

User shareHolder = new User("shareholder", "ShareHolder");
conquery.getMetaProcessor().addUser(shareHolder);

shareHolder.addPermissions(metaStorage, Set.of(
dataset.createPermission(Set.of(Ability.READ)),
execution.createPermission(Set.of(Ability.READ))
));

ManagedExecutionId copyId = IntegrationUtils.assertQueryResult(conquery, reused, 4L, ExecutionState.DONE, shareHolder, 201);

ManagedExecution<?> copy = metaStorage.getExecution(copyId);


// Contentwise the label and tags should be the same
awildturtok marked this conversation as resolved.
Show resolved Hide resolved
assertThat(copy).usingRecursiveComparison().comparingOnlyFields("label","tags").isEqualTo(execution);

// However the Object holding the tags must be different, so the two are not linked here
assertThat(copy.getTags()).isNotSameAs(execution.getTags());

// And the ids must be different
assertThat(copy.getId()).isNotSameAs(execution.getId());
}
}
}
}