Skip to content

Di pipeline #10

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

Open
wants to merge 2 commits into
base: kaibocai/dc-middleware
Choose a base branch
from
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 @@ -3,5 +3,5 @@
import com.microsoft.azure.functions.ExecutionContext;

public interface FunctionWorkerChain {
public void doNext(ExecutionContext context) throws Exception;
public void doNext(MiddlewareExecutionContext context) throws Exception;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
import com.microsoft.azure.functions.ExecutionContext;

public interface FunctionWorkerMiddleware {
public void invoke (ExecutionContext context, FunctionWorkerChain next) throws Exception;
public void invoke (MiddlewareExecutionContext context, FunctionWorkerChain next) throws Exception;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.microsoft.azure.functions.middleware;

import com.microsoft.azure.functions.ExecutionContext;

/**
* Expands {@link ExecutionContext} so that {@link FunctionWorkerMiddleware} implementations
* can modify target function instance or get more information about the target function.
*/
public interface MiddlewareExecutionContext extends ExecutionContext {
public ClassLoader getFunctionClassLoader();
public Class getFunctionClass();

public Object getFunctionInstance();
Copy link
Member

Choose a reason for hiding this comment

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

For Quarkus, we don't need getFunctionClassLoader and getFunctionInstance right. Please correct me if I am wrong.

Copy link
Author

Choose a reason for hiding this comment

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

getFunctionClassLoader() is removable so long as getFunctionClass().getClassLoader() returns the correct loader.

getFunctionInstance() you'd want to have to see if another middleware has overriden the instance already and proxy that instance, output a warning, or abort. An alternative to that is for setFunctionInstance() to throw an exception if the instance has already been set. What do you think is the best approach?

Copy link
Member

@kaibocai kaibocai Sep 13, 2022

Choose a reason for hiding this comment

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

Make sense, I was just wondering if we would have such a use case. The only middleware that require access to FunctionInstance will be DI framework like Quarkus or Spring, etc. But cx will not use both Quarkus and Spring at the same time.
However, I can see this is valid point. I will explore this more from our side.
Personally, I prefer second direction has setFunctionInstance() do the check and throw the exception if this case happens.

Copy link
Author

Choose a reason for hiding this comment

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

middleware would be pulled in via a maven dependency. So, if an app developer accidently pulled in both a Quarkus and Spring middleware dependency by accident (or on purpose) you could have conflict. These sort of dependency conflicts are more common than you think based on my experience.

Not that important though. Just something to think about.


/**
* Allows middleware to override target function instance for this invocation.
* The default behavior is for the instance to be creatd by the Azure Functions runtime.
*
* @param obj
*/
public void setFunctionInstance(Object obj);
}