[FFM-5471] - Target identifier set in the target builder function in …

…the Java SDK is not used (#125)

What
Targets reported by Java SDK are incorrectly showing in the UI Target Management -> Targets as the global target

Why
We're sending the global target identifier in the targetData when we shouldn't be. Also global target is not correctly applied to metricsData when isGlobalTargetEnabled is set

Testing
Tested manually with sample app to confirm changes are reflected in the UI.
Also added a new unit test to capture and assert on the posted MetricsData object.

src/main/java/io/harness/cf/client/api/MetricsProcessor.java

@@ -31,6 +31,9 @@ class MetricsProcessor extends AbstractScheduledService {
   private static final String SDK_LANGUAGE = "SDK_LANGUAGE";
   private static final String SDK_VERSION = "SDK_VERSION";
 
+  private static final Target globalTarget =
+      Target.builder().name(GLOBAL_TARGET_NAME).identifier(GLOBAL_TARGET).build();
+
   private final Connector connector;
   private final BaseConfig config;
   private final AtomicLongMap<MetricEvent> frequencyMap;
@@ -70,11 +73,12 @@ public synchronized void pushToQueue(Target target, String featureName, Variatio
 
     uniqueTargetSet.add(target);
 
-    if (config.isGlobalTargetEnabled()) {
-      target.setIdentifier(GLOBAL_TARGET);
+    Target metricTarget = globalTarget;
+    if (!config.isGlobalTargetEnabled()) {
+      metricTarget = target;
     }
 
-    frequencyMap.incrementAndGet(new MetricEvent(featureName, target, variation));
+    frequencyMap.incrementAndGet(new MetricEvent(featureName, metricTarget, variation));
   }
 
   /** This method sends the metrics data to the analytics server and resets the cache */
@@ -117,10 +121,8 @@ protected Metrics prepareSummaryMetricsBody(Map<MetricEvent, Long> data, Set<Tar
     final Metrics metrics = new Metrics(new ArrayList<>(), new ArrayList<>());
     final Map<SummaryMetrics, Long> summaryMetricsData = new HashMap<>();
 
-    addTargetData(
-        metrics, Target.builder().name(GLOBAL_TARGET_NAME).identifier(GLOBAL_TARGET).build());
-
     targets.forEach(target -> addTargetData(metrics, target));
+
     data.forEach(
         (target, count) ->
             summaryMetricsData.put(

src/test/java/io/harness/cf/client/api/MetricsProcessorTest.java

@@ -1,22 +1,22 @@
 package io.harness.cf.client.api;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.fail;
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.when;
 
 import com.google.common.collect.Maps;
 import io.harness.cf.client.connector.Connector;
+import io.harness.cf.client.connector.ConnectorException;
 import io.harness.cf.client.dto.Target;
-import io.harness.cf.model.FeatureConfig;
-import io.harness.cf.model.Metrics;
-import io.harness.cf.model.Variation;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
+import io.harness.cf.model.*;
+import java.util.*;
 import java.util.concurrent.*;
 import lombok.NonNull;
 import lombok.extern.slf4j.Slf4j;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.*;
 
 @Slf4j
@@ -141,7 +141,7 @@ public void onMetricsFailure() {
   }
 
   @Test
-  public void testPrepareSummaryMetricsBody() {
+  void testPrepareSummaryMetricsBody() {
     Target target = Target.builder().identifier("harness").build();
     FeatureConfig feature = FeatureConfig.builder().feature("bool-flag").build();
     Variation variation = Variation.builder().identifier("true").value("true").build();
@@ -150,12 +150,69 @@ public void testPrepareSummaryMetricsBody() {
     Set<Target> uniqueTargets = new HashSet<>();
     uniqueTargets.add(target);
 
-    Map<MetricEvent, Long> map = Maps.newHashMap();
-    map.put(event, 6L);
+    Map<MetricEvent, Long> freqMap = Maps.newHashMap();
+    freqMap.put(event, 6L);
 
-    Metrics metrics = metricsProcessor.prepareSummaryMetricsBody(map, uniqueTargets);
+    Metrics metrics = metricsProcessor.prepareSummaryMetricsBody(freqMap, uniqueTargets);
 
-    assert metrics.getTargetData() != null;
-    assert metrics.getTargetData().get(1).getIdentifier().equals(target.getIdentifier());
+    assertNotNull(metrics);
+    assertNotNull(metrics.getTargetData());
+    assertNotNull(metrics.getMetricsData());
+
+    assertEquals(target.getIdentifier(), metrics.getTargetData().get(0).getIdentifier());
+    assertEquals(6, metrics.getMetricsData().get(0).getCount());
+  }
+
+  @ParameterizedTest
+  @ValueSource(booleans = {true, false})
+  void shouldPostCorrectMetrics_WhenGlobalTargetEnabledOrDisabled(boolean globalTargetEnabled)
+      throws ConnectorException {
+    final Connector mockConnector = Mockito.mock(Connector.class);
+    final BaseConfig mockConfig = Mockito.mock(BaseConfig.class);
+    final ArgumentCaptor<Metrics> metricsArgumentCaptor = ArgumentCaptor.forClass(Metrics.class);
+
+    when(mockConfig.isGlobalTargetEnabled()).thenReturn(globalTargetEnabled);
+    when(mockConfig.getBufferSize()).thenReturn(10);
+    doNothing().when(mockConnector).postMetrics(metricsArgumentCaptor.capture());
+
+    final MetricsProcessor processor =
+        new MetricsProcessor(mockConnector, mockConfig, Mockito.mock(MetricsCallback.class));
+
+    final Target target = Target.builder().identifier("target123").build();
+    final Variation variation = Variation.builder().identifier("true").value("true").build();
+    processor.pushToQueue(target, "feature1", variation);
+    processor.pushToQueue(target, "feature1", variation);
+    processor.pushToQueue(target, "feature2", variation);
+    processor.pushToQueue(target, "feature2", variation);
+    processor.pushToQueue(target, "feature3", variation);
+    processor.pushToQueue(target, "feature3", variation);
+    processor.runOneIteration(); // Mimic scheduled job
+
+    final Metrics sentMetrics = metricsArgumentCaptor.getValue();
+
+    assertNotNull(sentMetrics.getMetricsData());
+    assertNotNull(sentMetrics.getTargetData());
+
+    assertEquals(1, sentMetrics.getTargetData().size());
+    assertEquals("target123", sentMetrics.getTargetData().get(0).getIdentifier());
+
+    assertEquals(3, sentMetrics.getMetricsData().size());
+    for (MetricsData md : sentMetrics.getMetricsData()) {
+      final Map<String, String> map = keyValueArrayToMap(md.getAttributes());
+      assertEquals(2, md.getCount());
+      if (globalTargetEnabled) {
+        assertEquals("__global__cf_target", map.get("target"));
+      } else {
+        assertEquals("target123", map.get("target"));
+      }
+    }
+  }
+
+  private Map<String, String> keyValueArrayToMap(List<KeyValue> keyValueList) {
+    final Map<String, String> map = new HashMap<>();
+    for (KeyValue kv : keyValueList) {
+      map.put(kv.getKey(), kv.getValue());
+    }
+    return map;
   }
 }