Skip to content

Commit

Permalink
Allow passing call context to Executor
Browse files Browse the repository at this point in the history 
With the delete service, the issue was that the call itself
(queueDelete) had the context but the action was taking place
in a separate thread without having passed the value in.

Now, we can optionally pass current.ctx into the DeleteHandleI
and have the value applied via Executor.execute. This same change
will need to happen anywhere where this type of background activity
is taking place.
  • Loading branch information
@joshmoore
joshmoore committed Feb 22, 2012
1 parent 1f522ea commit 7f31776
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 23 deletions.
23 changes: 19 additions & 4 deletions components/blitz/src/ome/services/blitz/impl/DeleteHandleI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicReference;
Expand Down Expand Up @@ -138,21 +139,34 @@ private static class Report extends DeleteReport {

private final ServiceFactoryI sf;

private final Map<String, String> callContext;

/**
* Call the main constructor with a null call context.
*/
public DeleteHandleI(final ApplicationContext ctx, final Ice.Identity id, final ServiceFactoryI sf,
final AbstractFileSystemService afs, final DeleteCommand[] commands, int cancelTimeoutMs) {
this(ctx, id, sf, afs, commands, cancelTimeoutMs, null);
}

/**
* Create and
* Main constructor.
*
* @param id
* @param sf
* @param factory
* @param commands
* @param cancelTimeoutMs
* @param callContext
*/
public DeleteHandleI(final ApplicationContext ctx, final Ice.Identity id, final ServiceFactoryI sf,
final AbstractFileSystemService afs, final DeleteCommand[] commands, int cancelTimeoutMs) {
final AbstractFileSystemService afs, final DeleteCommand[] commands, int cancelTimeoutMs,
Map<String, String> callContext) {
super(null, null);
this.id = id;
this.sf = sf;
this.afs = afs;
this.callContext = callContext;
this.principal = sf.getPrincipal();
this.executor = sf.getExecutor();
this.cancelTimeoutMs = cancelTimeoutMs;
Expand Down Expand Up @@ -323,8 +337,9 @@ public void run() {

StopWatch sw = new CommonsLogStopWatch();
try {
executor.execute(principal, new Executor.SimpleWork(this, "run",
Ice.Util.identityToString(id), "size=" + commands.length) {
executor.execute(callContext, principal,
new Executor.SimpleWork(this, "run",
Ice.Util.identityToString(id), "size=" + commands.length) {
@Transactional(readOnly = false)
public Object doWork(Session session, ServiceFactory sf) {
try {
Expand Down
33 changes: 21 additions & 12 deletions components/blitz/src/ome/services/blitz/impl/DeleteI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,13 @@ public void checkImageDelete_async(AMD_IDelete_checkImageDelete __cb,
}

public void deleteImage_async(AMD_IDelete_deleteImage __cb, final long imageId,
boolean force, Current __current) throws ApiUsageException,
boolean force, final Current __current) throws ApiUsageException,
SecurityViolation, ServerError, ValidationException {

safeRunnableCall(__current, __cb, true, new Callable<Object>() {
public Object call() throws Exception {
DeleteCommand dc = new DeleteCommand("/Image", imageId, null);
makeAndRun(handleId(), dc);
makeAndRun(__current, handleId(), dc);
return null;
}});

Expand All @@ -101,7 +101,7 @@ public void previewImageDelete_async(AMD_IDelete_previewImageDelete __cb,
}

public void deleteImages_async(AMD_IDelete_deleteImages __cb,
final List<Long> ids, boolean force, Current __current)
final List<Long> ids, boolean force, final Current __current)
throws ApiUsageException, SecurityViolation, ServerError,
ValidationException {

Expand All @@ -114,7 +114,7 @@ public Object call() throws Exception {
for (int i = 0; i < ids.size(); i++) {
commands[i] = new DeleteCommand("/Image", ids.get(i), null);
}
makeAndRun(handleId(), commands);
makeAndRun(__current, handleId(), commands);
return null;
}});

Expand All @@ -128,23 +128,23 @@ public void deleteImagesByDataset_async(
}

public void deleteSettings_async(AMD_IDelete_deleteSettings __cb,
final long imageId, Current __current) throws ServerError {
final long imageId, final Current __current) throws ServerError {

safeRunnableCall(__current, __cb, true, new Callable<Object>() {
public Object call() throws Exception {
DeleteCommand dc = new DeleteCommand("/Image/Pixels/RenderingDef", imageId, null);
makeAndRun(handleId(), dc);
makeAndRun(__current, handleId(), dc);
return null;
}});
}

public void deletePlate_async(AMD_IDelete_deletePlate __cb,
final long plateId, Current __current) throws ServerError {
final long plateId, final Current __current) throws ServerError {

safeRunnableCall(__current, __cb, true, new Callable<Object>() {
public Object call() throws Exception {
DeleteCommand dc = new DeleteCommand("/Plate", plateId, null);
makeAndRun(handleId(), dc);
makeAndRun(__current, handleId(), dc);
return null;
}});

Expand All @@ -157,14 +157,14 @@ public void queueDelete_async(final AMD_IDelete_queueDelete __cb,
safeRunnableCall(__current, __cb, false, new Callable<DeleteHandlePrx>() {
public DeleteHandlePrx call() throws Exception {
Ice.Identity id = handleId();
DeleteHandleI handle = makeAndLaunchHandle(id, commands);
DeleteHandleI handle = makeAndLaunchHandle(__current, id, commands);
DeleteHandlePrx prx = DeleteHandlePrxHelper.
uncheckedCast(sf.registerServant(id,
new _DeleteHandleTie(handle)));
return prx;
}});
}

public void availableCommands_async(final AMD_IDelete_availableCommands __cb,
final Current __current)
throws ServerError {
Expand All @@ -188,13 +188,22 @@ public DeleteCommand[] call() throws Exception {
}

public DeleteHandleI makeAndLaunchHandle(final Ice.Identity id, final DeleteCommand...commands) {
DeleteHandleI handle = new DeleteHandleI(loadSpecs(), id, sf, afs, commands, cancelTimeoutMs);
return makeAndLaunchHandle(null, id, commands);
}

public DeleteHandleI makeAndLaunchHandle(final Ice.Current current, final Ice.Identity id,
final DeleteCommand...commands) {
DeleteHandleI handle = new DeleteHandleI(loadSpecs(), id, sf, afs, commands, cancelTimeoutMs, current.ctx);
threadPool.getExecutor().execute(handle);
return handle;
}

public void makeAndRun(final Ice.Identity id, final DeleteCommand...commands) {
DeleteHandleI handle = new DeleteHandleI(loadSpecs(), id, sf, afs, commands, cancelTimeoutMs);
makeAndRun(null, id, commands);
}

public void makeAndRun(final Ice.Current current, final Ice.Identity id, final DeleteCommand...commands) {
DeleteHandleI handle = new DeleteHandleI(loadSpecs(), id, sf, afs, commands, cancelTimeoutMs, current.ctx);
handle.run();
}

Expand Down
44 changes: 37 additions & 7 deletions components/server/src/ome/services/util/Executor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
Expand Down Expand Up @@ -69,25 +70,38 @@ public interface Executor extends ApplicationContextAware {
*/
public Principal principal();

/**
* Call {@link #execute(Map<String, String>, Principal, Work)} with
* a null call context.
*/
public Object execute(final Principal p, final Work work);

/**
* Executes a {@link Work} instance wrapped in two layers of AOP. The first
* is intended to acquire the proper arguments for
* {@link Work#doWork(Session, ServiceFactory)} from the
* {@link OmeroContext}, and the second performs all the standard service
* actions for any normal method call.
*
*
* If the {@link Map<String, String>} argument is not null, then additionally,
* setContext will be called in a try/finally block. The first login
* within this thread will then pick up this delayed context.
*
* If the {@link Principal} argument is not null, then additionally, a
* login/logout sequence will be performed in a try/finally block.
*
*
* {@link Work} implementation must be annotated with {@link Transactional}
* in order to properly specify isolation, read-only status, etc.
*
*
* @param callContext
* Possibly null.
* @param p
* Possibly null.
* @param work
* Not null.
*/
public Object execute(final Principal p, final Work work);
public Object execute(final Map<String, String> callContext,
final Principal p, final Work work);

/**
* Simple submission method which can be used in conjunction with a call to
Expand Down Expand Up @@ -311,20 +325,30 @@ public Principal principal() {
}
}

/**
* Call {@link #execute(Map<String, String>, Principal, Work)}
* with a null call context.
*/
public Object execute(final Principal p, final Work work) {
return execute(null, p, work);
}

/**
* Executes a {@link Work} instance wrapped in two layers of AOP. The
* first is intended to acquire the proper arguments for
* {@link Work#doWork(TransactionStatus, Session, ServiceFactory)} for
* the {@link OmeroContext}, and the second performs all the standard
* service actions for any normal method call.
*
*
* If the {@link Principal} argument is not null, then additionally, a
* login/logout sequence will be performed in a try/finally block.
*
*
* @param callContext Possibly null key-value map. See #3529
* @param p
* @param work
*/
public Object execute(final Principal p, final Work work) {
public Object execute(final Map<String, String> callContext,
final Principal p, final Work work) {

if (work instanceof SimpleWork) {
((SimpleWork) work).setSqlAction(sqlAction);
Expand Down Expand Up @@ -356,6 +380,9 @@ public Object execute(final Principal p, final Work work) {
if (p != null) {
this.principalHolder.login(p);
}
if (callContext != null) {
this.principalHolder.setContext(callContext);
}

try {
// Arguments will be replaced after hibernate is in effect
Expand All @@ -364,6 +391,9 @@ public Object execute(final Principal p, final Work work) {
if (p != null) {
this.principalHolder.logout();
}
if (callContext != null) {
this.principalHolder.setContext(null);
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions components/server/test/ome/server/utests/DummyExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package ome.server.utests;

import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
Expand Down Expand Up @@ -44,6 +45,10 @@ public DummyExecutor(org.hibernate.Session session, ServiceFactory sf,
}

public Object execute(Principal p, Work work) {
return execute(null, p, work);
}

public Object execute(Map<String, String> callContext, Principal p, Work work) {
return work.doWork(session, sf);
}

Expand Down

0 comments on commit 7f31776

Please sign in to comment.