khandelwal-prateek commented on code in PR #4052:
URL: https://github.com/apache/gobblin/pull/4052#discussion_r1755105657
##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/util/TemporalWorkFlowUtils.java:
##########
@@ -0,0 +1,52 @@
+package org.apache.gobblin.temporal.ddm.util;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import lombok.NonNull;
+import lombok.experimental.UtilityClass;
+import org.apache.gobblin.configuration.ConfigurationKeys;
+import org.apache.gobblin.temporal.ddm.work.assistance.Help;
+
+
+/**
+ * Utility class for handling Temporal workflow-related operations.
+ */
+@UtilityClass
+public class TemporalWorkFlowUtils {
+
+ /**
+ * Generates search attributes for a WorkFlow based on the provided GAAS
job properties.
+ *
+ * @param jobProps the properties of the job, must not be null.
+ * @return a map containing the generated search attributes.
+ */
+ public static Map<String, Object> generateGaasSearchAttributes(@NonNull
Properties jobProps) {
+ Map<String, Object> attributes = new HashMap<>();
+ attributes.put(Help.GAAS_FLOW_KEY, String.format("%s.%s",
jobProps.getProperty(ConfigurationKeys.FLOW_GROUP_KEY),
+ jobProps.getProperty(ConfigurationKeys.FLOW_NAME_KEY)));
+ attributes.put(Help.USER_TO_PROXY_SEARCH_KEY,
jobProps.getProperty(Help.USER_TO_PROXY_KEY));
+ return attributes;
+ }
+
+ /**
+ * Converts search attribute values from a map of lists to a map of objects.
+ *
+ * @param searchAttributes a map where the keys are attribute names and the
values are lists of attribute values.
+ * Can be null.
+ * @return a map where the keys are attribute names and the values are the
corresponding attribute values.
+ * If the input map is null, an empty map is returned.
+ */
+ public static Map<String, Object>
convertSearchAttributesValuesFromListToObject(
+ Map<String, List<?>> searchAttributes) {
+ if (searchAttributes == null) {
+ return null;
Review Comment:
javadoc mentions that the method should return an empty map if
searchAttributes is null, however, this returns null. We can use
`Collections.emptyMap()` here instead of returning null
##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/util/TemporalWorkFlowUtils.java:
##########
@@ -0,0 +1,52 @@
+package org.apache.gobblin.temporal.ddm.util;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import lombok.NonNull;
+import lombok.experimental.UtilityClass;
+import org.apache.gobblin.configuration.ConfigurationKeys;
+import org.apache.gobblin.temporal.ddm.work.assistance.Help;
+
+
+/**
+ * Utility class for handling Temporal workflow-related operations.
+ */
+@UtilityClass
+public class TemporalWorkFlowUtils {
+
+ /**
+ * Generates search attributes for a WorkFlow based on the provided GAAS
job properties.
+ *
+ * @param jobProps the properties of the job, must not be null.
+ * @return a map containing the generated search attributes.
+ */
+ public static Map<String, Object> generateGaasSearchAttributes(@NonNull
Properties jobProps) {
+ Map<String, Object> attributes = new HashMap<>();
+ attributes.put(Help.GAAS_FLOW_KEY, String.format("%s.%s",
jobProps.getProperty(ConfigurationKeys.FLOW_GROUP_KEY),
+ jobProps.getProperty(ConfigurationKeys.FLOW_NAME_KEY)));
+ attributes.put(Help.USER_TO_PROXY_SEARCH_KEY,
jobProps.getProperty(Help.USER_TO_PROXY_KEY));
Review Comment:
let's add null checks for required keys in jobProps. If `FLOW_GROUP_KEY`,
`FLOW_NAME_KEY`, or `USER_TO_PROXY_KEY` are missing, the method will currently
throw a NullPointerException. It would be better to handle this gracefully by
throwing an IllegalArgumentException with an error message.
##########
gobblin-temporal/src/test/java/org/apache/gobblin/temporal/ddm/util/TemporalWorkFlowUtilsTest.java:
##########
@@ -0,0 +1,78 @@
+package org.apache.gobblin.temporal.ddm.util;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import org.apache.gobblin.configuration.ConfigurationKeys;
+import org.apache.gobblin.temporal.ddm.work.assistance.Help;
+import org.mockito.Mockito;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+
+public class TemporalWorkFlowUtilsTest {
+
+ private Properties jobProps = Mockito.mock(Properties.class);
+
+ @Test
+ public void testGenerateGaasSearchAttributes() {
+ // Mocking the properties
+
Mockito.when(jobProps.getProperty(ConfigurationKeys.FLOW_GROUP_KEY)).thenReturn("group");
+
Mockito.when(jobProps.getProperty(ConfigurationKeys.FLOW_NAME_KEY)).thenReturn("name");
+
Mockito.when(jobProps.getProperty(Help.USER_TO_PROXY_KEY)).thenReturn("userProxy");
Review Comment:
the test assumes that the required properties(`FLOW_GROUP_KEY`,
`FLOW_NAME_KEY` and `USER_TO_PROXY_KEY`) are always present. It would be useful
to add a test where some of these properties are missing or null and test the
behaviour in such cases
--
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: dev-unsubscr...@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
