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

Enable use of nullable maps for headers, params and query #694

Open
wants to merge 6 commits into
base: dev
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 @@ -9,4 +9,5 @@ private Constants(){}
public final static String JAVA_LIBRARY_DIRECTORY = "/annotationLib";
public final static String JAVA_LIBRARY_ARTIFACT_ID = "azure-functions-java-library";
public final static String HAS_IMPLICIT_OUTPUT_QUALIFIED_NAME = "com.microsoft.azure.functions.annotation.HasImplicitOutput";
public final static String NULLABLE_VALUES_ENABLED_APP_SETTING = "FUNCTIONS_WORKER_NULLABLE_VALUES_ENABLED";
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import com.microsoft.azure.functions.rpc.messages.NullableTypes;
import com.microsoft.azure.functions.worker.Constants;
import com.microsoft.azure.functions.worker.Util;
import org.apache.commons.lang3.reflect.TypeUtils;

import com.microsoft.azure.functions.HttpMethod;
Expand All @@ -22,8 +26,10 @@ public RpcHttpRequestDataSource(String name, RpcHttp value) {
super(name, null, HTTP_DATA_OPERATIONS);
this.httpPayload = value;
this.bodyDataSource = BindingDataStore.rpcSourceFromTypedData(null, this.httpPayload.getBody());
this.fields = Arrays.asList(this.httpPayload.getHeadersMap(), this.httpPayload.getQueryMap(),
this.httpPayload.getParamsMap());
this.fields = Arrays.asList(
convertFromNullableMap(this.httpPayload.getNullableHeadersMap()),
convertFromNullableMap(this.httpPayload.getNullableQueryMap()),
convertFromNullableMap(this.httpPayload.getNullableParamsMap()));
Copy link
Member

Choose a reason for hiding this comment

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

not related to this PR, but do we know difference between query map and param map?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looking at the host code, it looks like the three key differences between query map and param map are that:

  1. They come from different parts of the request object (i.e. request.Query vs request.HttpContext.Items.TryGetValue(HttpExtensionConstants.AzureWebJobsHttpRouteDataKey, out object routeData))
  2. We do not add null values to the parameters map, even if the UseNullableValueDictionaryForHttp capability is on
  3. We add empty strings to the parameters map, even if the UseNullableValueDictionaryForHttp capability is off

this.setValue(this);
}

Expand All @@ -45,12 +51,12 @@ public HttpMethod getHttpMethod() {

@Override
public Map<String, String> getHeaders() {
return this.parentDataSource.httpPayload.getHeadersMap();
return convertFromNullableMap(this.parentDataSource.httpPayload.getNullableHeadersMap());
}

@Override
public Map<String, String> getQueryParameters() {
return this.parentDataSource.httpPayload.getQueryMap();
return convertFromNullableMap(this.parentDataSource.httpPayload.getNullableQueryMap());
}

@Override
Expand Down Expand Up @@ -86,4 +92,16 @@ public Builder createResponseBuilder(HttpStatus status) {
return new HttpRequestMessageImpl(v, bodyData.getValue());
});
}

private static Map<String, String> convertFromNullableMap(Map<String, NullableTypes.NullableString> nullableMap) {
if (Util.isTrue(System.getenv(Constants.NULLABLE_VALUES_ENABLED_APP_SETTING))) {
return nullableMap.entrySet().stream().collect(
Collectors.toMap(Map.Entry::getKey, e -> e.getValue().getValue())
);
} else {
return nullableMap.entrySet().stream()
.filter(e -> e.getValue().getValue() != null && !e.getValue().getValue().trim().isEmpty())
.collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().getValue()));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ String execute(WorkerInitRequest request, WorkerInitResponse.Builder response) {
response.putCapabilities("TypedDataCollection", "TypedDataCollection");
response.putCapabilities("WorkerStatus", "WorkerStatus");
response.putCapabilities("RpcHttpBodyOnly", "RpcHttpBodyOnly");
response.putCapabilities("UseNullableValueDictionaryForHttp", "UseNullableValueDictionaryForHttp");
response.putCapabilities("RpcHttpTriggerMetadataRemoved", "RpcHttpTriggerMetadataRemoved");
response.putCapabilities("HandlesWorkerTerminateMessage", "HandlesWorkerTerminateMessage");
response.setWorkerMetadata(composeWorkerMetadata());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,20 @@
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.*;

import com.microsoft.azure.functions.rpc.messages.NullableTypes.NullableString;
import com.microsoft.azure.functions.HttpRequestMessage;
import com.microsoft.azure.functions.HttpStatus;
import com.microsoft.azure.functions.rpc.messages.RpcHttp;
import com.microsoft.azure.functions.rpc.messages.TypedData;
import com.microsoft.azure.functions.worker.Constants;
import com.microsoft.azure.functions.worker.binding.*;
import com.microsoft.azure.functions.worker.broker.JavaFunctionBroker;
import com.microsoft.azure.functions.worker.handler.FunctionEnvironmentReloadRequestHandler;
import com.microsoft.azure.functions.worker.reflect.DefaultClassLoaderProvider;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;

import static org.junit.jupiter.api.Assertions.*;

public class RpcHttpRequestDataSourceTest {

Expand All @@ -26,13 +31,23 @@ public void HttpRequestIntBody(HttpRequestMessage<Integer> request) {
public void HttpRequestBinaryBody(HttpRequestMessage<byte[]> request) {
}

public static RpcHttp getTestRpcHttp(Object inputBody) throws Exception {
public static RpcHttp getTestRpcHttp(
Object inputBody,
Map<String, String> headersMap,
Map<String, String> queryMap) throws Exception {
TypedData.Builder dataBuilder = TypedData.newBuilder();
RpcHttp.Builder httpBuilder = RpcHttp.newBuilder()
.setStatusCode(Integer.toString(HttpStatus.OK.value()));
Map<String, String> headers = new HashMap<>();
headers.put("header", "testHeader");
headers.forEach(httpBuilder::putHeaders);
if (headersMap != null) {
for (String key : headersMap.keySet()) {
httpBuilder.putNullableHeaders(key, NullableString.newBuilder().setValue(headersMap.get(key)).build());
}
}
if (queryMap != null) {
for (String key : queryMap.keySet()) {
httpBuilder.putNullableQuery(key, NullableString.newBuilder().setValue(queryMap.get(key)).build());
}
}
RpcUnspecifiedDataTarget bodyTarget = new RpcUnspecifiedDataTarget();
Object body = inputBody;
bodyTarget.setValue(body);
Expand All @@ -42,13 +57,81 @@ public static RpcHttp getTestRpcHttp(Object inputBody) throws Exception {
}

@Test
public void rpcHttpDataSource_To_HttpRequestMessage_StringBody() throws Exception {
public void rpcHttpDataSourceToHttpRequestMessageEnvSettingEnabled() throws Exception {
DefaultClassLoaderProvider classLoader = new DefaultClassLoaderProvider();
JavaFunctionBroker broker = new JavaFunctionBroker(classLoader);
FunctionEnvironmentReloadRequestHandler envHandler = new FunctionEnvironmentReloadRequestHandler(broker);
Map<String, String> existingVariables = System.getenv();
Map<String, String> newEnvVariables = new HashMap<>();
newEnvVariables.putAll(existingVariables);
newEnvVariables.put(Constants.NULLABLE_VALUES_ENABLED_APP_SETTING, "true");
envHandler.setEnv(newEnvVariables);
Method httpRequestMessageStringBodyMethod = getFunctionMethod("HttpRequestStringBody");
Map<String, String> queryMap = new HashMap<String, String>() {{
put("name", "");
put("count", "1");
}};
Map<String, String> headersMap = new HashMap<String, String>() {{
put("cookie", "");
put("accept-encoding", "gzip, deflate, br");
}};
Parameter[] parameters = httpRequestMessageStringBodyMethod.getParameters();
String sourceKey = "testRpcHttp";
RpcHttp input = getTestRpcHttp(null, headersMap, queryMap);
RpcHttpRequestDataSource rpcHttp = new RpcHttpRequestDataSource(sourceKey, input);
Optional<BindingData> actualBindingData = rpcHttp.computeByName(sourceKey,
parameters[0].getParameterizedType());
BindingData actualArg = actualBindingData.orElseThrow(WrongMethodTypeException::new);
HttpRequestMessage<?> requestMsg = (HttpRequestMessage<?>) actualArg.getValue();

assertTrue(requestMsg.getQueryParameters().get("name").isEmpty());
assertEquals("1", requestMsg.getQueryParameters().get("count"));
assertTrue(requestMsg.getHeaders().get("cookie").isEmpty());
assertEquals("gzip, deflate, br", requestMsg.getHeaders().get("accept-encoding"));
}

@Test
public void rpcHttpDataSourceToHttpRequestMessageEnvSettingDisabled() throws Exception {
DefaultClassLoaderProvider classLoader = new DefaultClassLoaderProvider();
JavaFunctionBroker broker = new JavaFunctionBroker(classLoader);
FunctionEnvironmentReloadRequestHandler envHandler = new FunctionEnvironmentReloadRequestHandler(broker);
Map<String, String> existingVariables = System.getenv();
Map<String, String> newEnvVariables = new HashMap<>();
newEnvVariables.putAll(existingVariables);
newEnvVariables.put(Constants.NULLABLE_VALUES_ENABLED_APP_SETTING, "false");
envHandler.setEnv(newEnvVariables);
Method httpRequestMessageStringBodyMethod = getFunctionMethod("HttpRequestStringBody");
Map<String, String> queryMap = new HashMap<String, String>() {{
put("name", "");
put("count", "1");
}};
Map<String, String> headersMap = new HashMap<String, String>() {{
put("cookie", "");
put("accept-encoding", "gzip, deflate, br");
}};
Parameter[] parameters = httpRequestMessageStringBodyMethod.getParameters();
String sourceKey = "testRpcHttp";
RpcHttp input = getTestRpcHttp(null, headersMap, queryMap);
RpcHttpRequestDataSource rpcHttp = new RpcHttpRequestDataSource(sourceKey, input);
Optional<BindingData> actualBindingData = rpcHttp.computeByName(sourceKey,
parameters[0].getParameterizedType());
BindingData actualArg = actualBindingData.orElseThrow(WrongMethodTypeException::new);
HttpRequestMessage<?> requestMsg = (HttpRequestMessage<?>) actualArg.getValue();

assertNull(requestMsg.getQueryParameters().get("name"));
assertEquals("1", requestMsg.getQueryParameters().get("count"));
assertNull(requestMsg.getHeaders().get("cookie"));
assertEquals("gzip, deflate, br", requestMsg.getHeaders().get("accept-encoding"));
}

@Test
public void rpcHttpDataSourceToHttpRequestMessageStringBody() throws Exception {

Method httpRequestMessageStringBodyMethod = getFunctionMethod("HttpRequestStringBody");

Parameter[] parameters = httpRequestMessageStringBodyMethod.getParameters();
String sourceKey = "testRpcHttp";
RpcHttp input = getTestRpcHttp("testStringBody");
RpcHttp input = getTestRpcHttp("testStringBody", null, null);
RpcHttpRequestDataSource rpcHttp = new RpcHttpRequestDataSource(sourceKey, input);
Optional<BindingData> actualBindingData = rpcHttp.computeByName(sourceKey,
parameters[0].getParameterizedType());
Expand All @@ -58,13 +141,13 @@ public void rpcHttpDataSource_To_HttpRequestMessage_StringBody() throws Exceptio
}

@Test
public void rpcHttpDataSource_To_HttpRequestMessage_IntegerBody() throws Exception {
public void rpcHttpDataSourceToHttpRequestMessageIntegerBody() throws Exception {

Method httpRequestMessageStringBodyMethod = getFunctionMethod("HttpRequestIntBody");

Parameter[] parameters = httpRequestMessageStringBodyMethod.getParameters();
String sourceKey = "testRpcHttp";
RpcHttp input = getTestRpcHttp(1234);
RpcHttp input = getTestRpcHttp(1234, null, null);
RpcHttpRequestDataSource rpcHttp = new RpcHttpRequestDataSource(sourceKey, input);
Optional<BindingData> actualBindingData = rpcHttp.computeByName(sourceKey,
parameters[0].getParameterizedType());
Expand All @@ -74,15 +157,15 @@ public void rpcHttpDataSource_To_HttpRequestMessage_IntegerBody() throws Excepti
}

@Test
public void rpcHttpDataSource_To_HttpRequestMessage_byteArrayBody() throws Exception {
public void rpcHttpDataSourceToHttpRequestMessageByteArrayBody() throws Exception {

Method httpRequestMessageStringBodyMethod = getFunctionMethod("HttpRequestBinaryBody");

Parameter[] parameters = httpRequestMessageStringBodyMethod.getParameters();
String sourceKey = "testRpcHttp";
String expectedString = "Example String";
byte[] inputBytes = expectedString.getBytes();
RpcHttp input = getTestRpcHttp(inputBytes);
RpcHttp input = getTestRpcHttp(inputBytes, null, null);
RpcHttpRequestDataSource rpcHttp = new RpcHttpRequestDataSource(sourceKey, input);
Optional<BindingData> actualBindingData = rpcHttp.computeByName(sourceKey,
parameters[0].getParameterizedType());
Expand Down