chamikaramj commented on code in PR #31406:
URL: https://github.com/apache/beam/pull/31406#discussion_r1622869069
##########
sdks/java/expansion-service/src/main/java/org/apache/beam/sdk/expansion/service/ExpansionServiceOptions.java:
##########
@@ -78,17 +79,17 @@ public AllowList create(PipelineOptions options) {
if (allowListFile.equals("*")) {
return AllowList.everything();
}
- ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
File allowListFileObj = new File(allowListFile);
if (!allowListFileObj.exists()) {
throw new IllegalArgumentException(
"Allow list file " + allowListFile + " does not exist");
}
- try {
- return mapper.readValue(allowListFileObj, AllowList.class);
+ try (InputStream stream = new FileInputStream(allowListFileObj)) {
+ return AllowList.parseFromYamlStream(stream);
+ } catch (FileNotFoundException e) {
+ throw new RuntimeException(e);
Review Comment:
Done.
##########
sdks/java/expansion-service/src/main/java/org/apache/beam/sdk/expansion/service/ExpansionServiceOptions.java:
##########
@@ -104,16 +105,16 @@ class ExpansionServiceConfigFactory implements
DefaultValueFactory<ExpansionServ
public ExpansionServiceConfig create(PipelineOptions options) {
String configFile =
options.as(ExpansionServiceOptions.class).getExpansionServiceConfigFile();
if (configFile != null) {
- ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
File configFileObj = new File(configFile);
if (!configFileObj.exists()) {
throw new IllegalArgumentException("Config file " + configFile + "
does not exist");
}
- try {
- return mapper.readValue(configFileObj, ExpansionServiceConfig.class);
+ try (InputStream stream = new FileInputStream(configFileObj)) {
Review Comment:
Done.
##########
sdks/java/expansion-service/src/main/java/org/apache/beam/sdk/expansion/service/JavaClassLookupTransformProvider.java:
##########
@@ -488,8 +489,49 @@ public static AllowList everything() {
AllowedClass.create("*", AllowedClass.WILDCARD,
AllowedClass.WILDCARD)));
}
+ static AllowList parseFromYamlStream(InputStream inputStream) {
+ Yaml yaml = new Yaml();
+ Map<Object, Object> config = yaml.load(inputStream);
+
+ if (config != null) {
+ String version = config.get("version") != null ? (String)
config.get("version") : "";
+ List<AllowedClass> allowedClasses = new ArrayList<>();
+ if (config.get("allowedClasses") != null) {
+ allowedClasses =
+ ((List<Map<Object, Object>>) config.get("allowedClasses"))
+ .stream()
+ .map(
+ data -> {
+ String className = (String) data.get("className");
+ if (className == null) {
+ throw new IllegalArgumentException(
+ "Expected each entry in the allowlist to
include the 'className'");
+ }
+ List<String> allowedBuilderMethods =
+ (List<String>)
data.get("allowedBuilderMethods");
+ List<String> allowedConstructorMethods =
+ (List<String>)
data.get("allowedConstructorMethods");
+ if (allowedBuilderMethods == null) {
+ allowedBuilderMethods = new ArrayList<>();
+ }
+ if (allowedConstructorMethods == null) {
+ allowedConstructorMethods = new ArrayList<>();
+ }
+ return AllowedClass.create(
+ className, allowedBuilderMethods,
allowedConstructorMethods);
+ })
+ .collect(Collectors.toList());
+ }
+ return AllowList.create(version, allowedClasses);
+ } else {
+ throw new IllegalArgumentException(
+ "Could not parse the provided YAML stream into a non-trivial
AllowList");
+ }
+ }
+
public abstract String getVersion();
+ @SuppressWarnings("mutable")
Review Comment:
I added these since I got some random warnings when compiling but seems like
this didn't fully resolve the issue. So removing these suppressions for now.
##########
sdks/java/expansion-service/build.gradle:
##########
@@ -41,10 +41,8 @@ dependencies {
implementation project(path: ":sdks:java:core", configuration: "shadow")
implementation project(path: ":runners:java-fn-execution")
implementation project(path: ":sdks:java:harness")
+ implementation "org.yaml:snakeyaml:2.0"
Review Comment:
Done.
##########
sdks/java/expansion-service/src/main/java/org/apache/beam/sdk/expansion/service/JavaClassLookupTransformProvider.java:
##########
@@ -488,8 +489,49 @@ public static AllowList everything() {
AllowedClass.create("*", AllowedClass.WILDCARD,
AllowedClass.WILDCARD)));
}
+ static AllowList parseFromYamlStream(InputStream inputStream) {
+ Yaml yaml = new Yaml();
+ Map<Object, Object> config = yaml.load(inputStream);
+
+ if (config != null) {
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
