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

addTransaction-removeTransaction #1338

Merged
merged 70 commits into from
May 28, 2020
Merged
Show file tree
Hide file tree
Changes from 66 commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
47fc404
TANDS-19169
May 21, 2020
a574c9a
TANDS-19169
May 21, 2020
489f34a
TANDS-19169
May 21, 2020
18c0ea9
TANDS-19169
May 21, 2020
863b908
TANDS-19169
May 21, 2020
fe13372
TANDS-19169
May 21, 2020
f371289
TANDS-19169
May 21, 2020
fb5a5f9
TANDS-19169
May 21, 2020
536fd85
TANDS-19169
May 21, 2020
1080b23
TANDS-19169
May 21, 2020
d5fcfd2
TANDS-19169
May 21, 2020
c294b2e
TANDS-19169
May 21, 2020
76c32b0
TANDS-19169
May 21, 2020
4da0e0f
TANDS-19169
May 21, 2020
05bc1f9
TANDS-19169
May 21, 2020
b86a89a
TANDS-19169
May 21, 2020
8a63316
TANDS-19169
May 21, 2020
a4e13c1
TANDS-19169
May 21, 2020
8c67f5b
TANDS-19169
May 21, 2020
195a159
TANDS-19169
May 21, 2020
57a8d8a
TANDS-19169
May 21, 2020
ba25bb1
TANDS-19169
May 22, 2020
fcfdcb6
TANDS-19169-changing-CVSS-score
May 22, 2020
1558da4
addTransaction-removeTransaction
May 22, 2020
214e16d
addTransaction-removeTransaction
May 22, 2020
830f71c
addTransaction-removeTransaction
May 22, 2020
550e456
addTransaction-removeTransaction
May 22, 2020
947fc4a
addTransaction-removeTransaction
May 22, 2020
15d7f91
addTransaction-removeTransaction
May 22, 2020
1acc419
addressing-comments
May 22, 2020
c04c053
addressing-comments
May 22, 2020
cd110d6
addressing-comments
May 22, 2020
5461fdc
addressing-comments
May 22, 2020
fc7c63e
addressing-comments
May 22, 2020
6669c9e
addressing-comments
May 22, 2020
5fd1f6e
addressing-comments
May 22, 2020
8caa043
addressing-comments
May 22, 2020
53966a2
addressing-comments
May 22, 2020
4b384ab
making DataStore an abstract class
May 26, 2020
c5ddbb6
making DataStore an abstract class
May 26, 2020
9ee68ed
fixing bugs
May 26, 2020
d9689b8
fixing bugs
May 26, 2020
6de50df
fixing bugs
May 26, 2020
716e198
fixing bugs
May 26, 2020
959808a
fixing bugs
May 26, 2020
ed6d8e3
fixing bugs
May 26, 2020
b306a0b
fixing bugs
May 26, 2020
2091e66
fixing bugs
May 26, 2020
1a564e4
fixing bugs
May 26, 2020
e899393
fixing bugs
May 26, 2020
e1e1ee0
fixing bugs
May 26, 2020
5442e4d
fixing bugs
May 26, 2020
a9760c4
adding-transaction-id to transaction implementations
May 27, 2020
2152ee0
adding-transaction-id to transaction implementations
May 27, 2020
dd022c1
adding-transaction-id to transaction implementations
May 27, 2020
8e7794c
adding-transaction-id to transaction implementations
May 27, 2020
c027cbd
fixing bugs
May 27, 2020
862bbe5
fixing bugs
May 27, 2020
862dd9c
fixing bugs
May 27, 2020
c737e1c
fixing bugs
May 27, 2020
da89038
fixing bugs
May 27, 2020
a4ba910
fixing bugs
May 27, 2020
89be875
fixing bugs
May 27, 2020
e5016c6
fixing bugs
May 27, 2020
9a41c0c
fixing bugs
May 27, 2020
11c2708
fixing bugs
May 27, 2020
52b4c61
addressing comments
May 27, 2020
3c5cdcd
addressing comments
May 27, 2020
8e73ca6
addressing comments
May 27, 2020
c10cfd4
fixing bugs
May 28, 2020
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
9 changes: 8 additions & 1 deletion elide-core/src/main/java/com/yahoo/elide/Elide.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.yahoo.elide.core.DataStoreTransaction;
import com.yahoo.elide.core.HttpStatus;
import com.yahoo.elide.core.RequestScope;
import com.yahoo.elide.core.TransactionRegistry;
import com.yahoo.elide.core.datastore.inmemory.InMemoryDataStore;
import com.yahoo.elide.core.exceptions.ForbiddenAccessException;
import com.yahoo.elide.core.exceptions.HttpStatusException;
Expand Down Expand Up @@ -57,6 +58,7 @@
import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.util.Set;
import java.util.UUID;
import java.util.function.Supplier;

import javax.validation.ConstraintViolationException;
Expand All @@ -76,7 +78,7 @@ public class Elide {
@Getter private final AuditLogger auditLogger;
@Getter private final DataStore dataStore;
@Getter private final JsonApiMapper mapper;

@Getter private final TransactionRegistry transactionRegistry;
/**
* Instantiates a new Elide instance.
*
Expand All @@ -88,6 +90,7 @@ public Elide(ElideSettings elideSettings) {
this.dataStore = new InMemoryDataStore(elideSettings.getDataStore());
this.dataStore.populateEntityDictionary(elideSettings.getDictionary());
this.mapper = elideSettings.getMapper();
this.transactionRegistry = new TransactionRegistry();

elideSettings.getSerdes().forEach((targetType, serde) -> {
CoerceUtil.register(targetType, serde);
Expand Down Expand Up @@ -278,7 +281,10 @@ protected ElideResponse handleRequest(boolean isReadOnly, User user,
Supplier<DataStoreTransaction> transaction,
Handler<DataStoreTransaction, User, HandlerResult> handler) {
boolean isVerbose = false;
UUID requestId = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

We are storing requestId as a string in requestScope. Can we do the same here? or is there an advantage to storing it like this?
https://github.com/yahoo/elide/pull/1331/files

Copy link
Contributor Author

@Rkr1992 Rkr1992 May 22, 2020

Choose a reason for hiding this comment

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

We are using it as a key in a Map so I guess we can store it either way

try (DataStoreTransaction tx = transaction.get()) {
requestId = tx.getRequestId();
transactionRegistry.addRunningTransaction(requestId, tx);
HandlerResult result = handler.handle(tx, user);
RequestScope requestScope = result.getRequestScope();
isVerbose = requestScope.getPermissionExecutor().isVerbose();
Expand Down Expand Up @@ -347,6 +353,7 @@ protected ElideResponse handleRequest(boolean isReadOnly, User user,
throw e;

} finally {
transactionRegistry.removeRunningTransaction(requestId);
auditLogger.clear();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import java.io.Serializable;
import java.util.Iterator;
import java.util.Set;

import java.util.UUID;
/**
* Wraps the Database Transaction type.
*/
Expand All @@ -31,7 +31,6 @@ public enum FeatureSupport {
PARTIAL,
NONE
}

/**
* Save the updated object.
*
Expand Down Expand Up @@ -229,7 +228,7 @@ default Object getAttribute(Object entity,
* @param entity - The object which owns the attribute.
* @param attribute - the attribute to set.
* @param scope - contains request level metadata.
*/
*/
default void setAttribute(Object entity,
Attribute attribute,
RequestScope scope) {
Expand Down Expand Up @@ -263,4 +262,6 @@ default boolean supportsSorting(Class<?> entityClass, Sorting sorting) {
default boolean supportsPagination(Class<?> entityClass, FilterExpression expression) {
return true;
}

UUID getRequestId();
Copy link
Member

Choose a reason for hiding this comment

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

Needs javadoc.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright 2020, Yahoo Inc.
* Licensed under the Apache License, Version 2.0
* See LICENSE file in project root for terms.
*/
package com.yahoo.elide.core;

import lombok.Getter;

import java.util.UUID;

public abstract class DataStoreTransactionImplementation implements DataStoreTransaction {
Copy link
Member

Choose a reason for hiding this comment

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

Class needs javadoc.

@Getter private final UUID requestId = UUID.randomUUID();

@Override
public UUID getRequestId() {
return requestId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,44 +5,33 @@
*/
package com.yahoo.elide.core;

import lombok.Data;
import lombok.Getter;

import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
/**
* Transaction Registry interface to surface transaction details to other parts of Elide.
*/

public interface TransactionRegistry {
/**
* @see RequestScope
* @see DataStoreTransaction
*/
@Data
public static class TransactionEntry {
public RequestScope request;
public DataStoreTransaction transaction;
* Transaction Registry class.
*/
@Getter
public class TransactionRegistry {
private Map<UUID, DataStoreTransaction> transactionMap = new HashMap<>();
public Set<DataStoreTransaction> getRunningTransactions() {
Set<DataStoreTransaction> transactions = transactionMap.values().stream().collect(Collectors.toSet());
return transactions;
}

/**
* @return all running transactions
*/
Set<TransactionEntry> getRunningTransactions();

/**
* @param requestId
* @return matching running transaction
*/
Set<TransactionEntry> getRunningTransaction(String requestId);
public DataStoreTransaction getRunningTransaction(UUID requestId) {
return transactionMap.get(requestId);
}

/**
* Adds running transaction
* @param transactionEntry TransactionEntry transactionEntry
*/
void addRunningTransaction(TransactionEntry transactionEntry);
public void addRunningTransaction(UUID requestId, DataStoreTransaction tx) {
transactionMap.put(requestId, tx);
}

/**
* Removes running transaction when we call cancel on it
* @param transactionEntry TransactionEntry transactionEntry
*/
void removeRunningTransaction(TransactionEntry transactionEntry);
public void removeRunningTransaction(UUID requestId) {
transactionMap.remove(requestId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package com.yahoo.elide.core.datastore.inmemory;

import com.yahoo.elide.core.DataStoreTransaction;
import com.yahoo.elide.core.DataStoreTransactionImplementation;
import com.yahoo.elide.core.EntityDictionary;
import com.yahoo.elide.core.RequestScope;
import com.yahoo.elide.core.exceptions.TransactionException;
Expand All @@ -27,7 +28,7 @@
/**
* HashMapDataStore transaction handler.
*/
public class HashMapStoreTransaction implements DataStoreTransaction {
public class HashMapStoreTransaction extends DataStoreTransactionImplementation {
private final Map<Class<?>, Map<String, Object>> dataStore;
private final List<Operation> operations;
private final EntityDictionary dictionary;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package com.yahoo.elide.core.datastore.inmemory;

import com.yahoo.elide.core.DataStoreTransaction;
import com.yahoo.elide.core.DataStoreTransactionImplementation;
import com.yahoo.elide.core.Path;
import com.yahoo.elide.core.PersistentResource;
import com.yahoo.elide.core.RequestScope;
Expand Down Expand Up @@ -40,10 +41,9 @@
* Data Store Transaction that wraps another transaction and provides in-memory filtering, soring, and pagination
* when the underlying transaction cannot perform the equivalent function.
*/
public class InMemoryStoreTransaction implements DataStoreTransaction {
public class InMemoryStoreTransaction extends DataStoreTransactionImplementation {

private final DataStoreTransaction tx;

private static final Comparator<Object> NULL_SAFE_COMPARE = (a, b) -> {
if (a == null && b == null) {
return 0;
Expand Down Expand Up @@ -119,7 +119,7 @@ public Object loadObject(EntityProjection projection,
projection.getFilterExpression()) == FeatureSupport.FULL) {
return tx.loadObject(projection, id, scope);
} else {
return DataStoreTransaction.super.loadObject(projection, id, scope);
return super.loadObject(projection, id, scope);
Copy link
Member

Choose a reason for hiding this comment

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

I think this is wrong. This was calling the default method defined in the interface. You changed this to call the super class - which is something else - and will cause problems.

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package com.yahoo.elide.core.datastore.wrapped;

import com.yahoo.elide.core.DataStoreTransaction;
import com.yahoo.elide.core.DataStoreTransactionImplementation;
import com.yahoo.elide.core.RequestScope;
import com.yahoo.elide.core.filter.expression.FilterExpression;
import com.yahoo.elide.request.Attribute;
Expand All @@ -25,7 +26,7 @@
*/
@Data
@AllArgsConstructor
public abstract class TransactionWrapper implements DataStoreTransaction {
public abstract class TransactionWrapper extends DataStoreTransactionImplementation {
protected DataStoreTransaction tx;

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import java.io.Serializable;
import java.util.Arrays;

public class DataStoreTransactionTest implements DataStoreTransaction {
public class DataStoreTransactionTest extends DataStoreTransactionImplementation implements DataStoreTransaction {
private static final String NAME = "name";
private static final Attribute NAME_ATTRIBUTE = Attribute.builder().name(NAME).type(String.class).build();
private static final String ENTITY = "entity";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@

import com.google.common.annotations.VisibleForTesting;

import java.io.IOException;
import lombok.Getter;

import java.io.IOException;
import java.util.UUID;
/**
* Transaction handler for {@link AggregationDataStore}.
*/
public class AggregationDataStoreTransaction implements DataStoreTransaction {
private QueryEngine queryEngine;

@Getter private final UUID requestId = UUID.randomUUID();
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason this can't extend DataStoreTransactionImpl?

public AggregationDataStoreTransaction(QueryEngine queryEngine) {
this.queryEngine = queryEngine;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package com.yahoo.elide.datastores.hibernate3;

import com.yahoo.elide.core.DataStoreTransaction;
import com.yahoo.elide.core.DataStoreTransactionImplementation;
import com.yahoo.elide.core.EntityDictionary;
import com.yahoo.elide.core.Path;
import com.yahoo.elide.core.RequestScope;
Expand Down Expand Up @@ -48,13 +49,12 @@
* Hibernate Transaction implementation.
*/
@Slf4j
public class HibernateTransaction implements DataStoreTransaction {
public class HibernateTransaction extends DataStoreTransactionImplementation {

private final Session session;
private final SessionWrapper sessionWrapper;
private final LinkedHashSet<Runnable> deferredTasks = new LinkedHashSet<>();
private final boolean isScrollEnabled;

/**
* Constructor.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package com.yahoo.elide.datastores.hibernate5;

import com.yahoo.elide.core.DataStoreTransaction;
import com.yahoo.elide.core.DataStoreTransactionImplementation;
import com.yahoo.elide.core.EntityDictionary;
import com.yahoo.elide.core.Path;
import com.yahoo.elide.core.RequestScope;
Expand Down Expand Up @@ -48,13 +49,12 @@
* Hibernate Transaction implementation.
*/
@Slf4j
public class HibernateTransaction implements DataStoreTransaction {
public class HibernateTransaction extends DataStoreTransactionImplementation {

private final Session session;
private final SessionWrapper sessionWrapper;
private final LinkedHashSet<Runnable> deferredTasks = new LinkedHashSet<>();
private final boolean isScrollEnabled;

/**
* Constructor.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@

import com.yahoo.elide.core.EntityDictionary;

import lombok.Getter;

import java.util.Map;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicLong;

/**
Expand All @@ -16,6 +19,7 @@
*/
@Deprecated
public class HashMapStoreTransaction extends com.yahoo.elide.core.datastore.inmemory.HashMapStoreTransaction {
@Getter private final UUID requestId = UUID.randomUUID();
public HashMapStoreTransaction(Map<Class<?>, Map<String, Object>> dataStore,
EntityDictionary dictionary, Map<Class<?>, AtomicLong> typeIds) {
super(dataStore, dictionary, typeIds);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package com.yahoo.elide.datastores.jpa.transaction;

import com.yahoo.elide.core.DataStoreTransaction;
import com.yahoo.elide.core.DataStoreTransactionImplementation;
import com.yahoo.elide.core.EntityDictionary;
import com.yahoo.elide.core.Path;
import com.yahoo.elide.core.RequestScope;
Expand Down Expand Up @@ -47,7 +48,7 @@
* Base JPA transaction implementation class.
*/
@Slf4j
public abstract class AbstractJpaTransaction implements JpaTransaction {
public abstract class AbstractJpaTransaction extends DataStoreTransactionImplementation implements JpaTransaction {
private static final Predicate<Collection<?>> IS_PERSISTENT_COLLECTION =
new PersistentCollectionChecker();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
@Slf4j
public class JtaTransaction extends AbstractJpaTransaction {
private final UserTransaction transaction;

public JtaTransaction(EntityManager entityManager) {
this(entityManager, lookupUserTransaction());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
@Slf4j
public class NonJtaTransaction extends AbstractJpaTransaction {
private final EntityTransaction transaction;

public NonJtaTransaction(EntityManager entityManager) {
super(entityManager);
this.transaction = entityManager.getTransaction();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
* Multiplex transaction handler.
*/
public class MultiplexReadTransaction extends MultiplexTransaction {

public MultiplexReadTransaction(MultiplexManager multiplexManager) {
super(multiplexManager);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import com.yahoo.elide.core.DataStore;
import com.yahoo.elide.core.DataStoreTransaction;
import com.yahoo.elide.core.DataStoreTransactionImplementation;
import com.yahoo.elide.core.EntityDictionary;
import com.yahoo.elide.core.RelationshipType;
import com.yahoo.elide.core.RequestScope;
Expand All @@ -32,7 +33,7 @@
* Multiplex transaction handler. Process each sub-database transactions within a single transaction.
* If any commit fails in process, reverse any commits already completed.
*/
public abstract class MultiplexTransaction implements DataStoreTransaction {
public abstract class MultiplexTransaction extends DataStoreTransactionImplementation {
protected final LinkedHashMap<DataStore, DataStoreTransaction> transactions;
protected final MultiplexManager multiplexManager;
protected final DataStoreTransaction lastDataStoreTransaction;
Expand Down
Loading