Skip to content

AB#238930 #19

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.

Sign up for GitHub

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
maryan-opti wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

AB#238930 #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
35fb354
handle Engagement payload as JSON String
maryan-opti Jan 13, 2025
c27173c
update pom dependency
maryan-opti Jan 13, 2025
d16e438
fix pom file
maryan-opti Jan 14, 2025
95786bc
handle unauth blob get
maryan-opti Jan 15, 2025
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
14 changes: 12 additions & 2 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,22 @@
<url>http://github.com/optimove/Optimove-SDK-Engagement-java/tree/master</url>
</scm>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>libraries-bom</artifactId>
<version>26.52.0</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>

<dependencies>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-storage</artifactId>
<version>2.34.0</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.dataformat</groupId>
Expand All @@ -62,7 +72,7 @@
<dependency>
<groupId>org.apache.avro</groupId>
<artifactId>avro</artifactId>
<version>1.11.3</version>
<version>1.11.4</version>
<exclusions>
<exclusion>
<groupId>org.apache.commons</groupId>
Expand Down
18 changes: 9 additions & 9 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ public class Main {
public static void main(String[] args) {
Logger logger = LoggerFactory.getLogger(Main.class);

Engagement engagement = new Engagement(null, 1, "bucket-name", "customers-folder/customers", "metadata-path/metadata_287934", logger);
Engagement engagement = new Engagement("{\r\n \"EventTypeID\": 14,\r\n \"TimeStamp\": \"2025-01-08 08:58\",\r\n \"CampaignID\": 1111,\r\n \"EngagementID\": 1111,\r\n \"TenantID\": 1111,\r\n \"BucketName\": \"bucketName\",\r\n \"CustomersFolderPath\": \"path\",\r\n \"MetadataFilePath\": \"pathCustomers\",\r\n \"DecryptionKey\": \"some-secret-key\",\r\n \"ChannelID\": 1111\r\n}", logger);

Metadata metadata = engagement.getMetadata();
System.out.println(metadata.getNumberOfFiles());
System.out.println(metadata.getNumberOfBatches());

for (int fileNumber = 1; fileNumber <= metadata.getNumberOfFiles(); fileNumber++) {
System.out.println("file number: " + fileNumber);
for (int batchNumber = 1; batchNumber <= metadata.getNumberOfBatches(); batchNumber++) {
System.out.println("batch number: " + batchNumber);

try (DataFileStream<GenericRecord> dataFileReader = engagement.getCustomerBatchById(fileNumber)) {
try (DataFileStream<GenericRecord> dataFileReader = engagement.getCustomerBatchById(batchNumber)) {
GenericRecord record = null;
while (dataFileReader.hasNext()) {
record = dataFileReader.next(record);
Expand Down Expand Up @@ -86,27 +86,27 @@ logger.error("Failed to retrieve metadata: " + e.getMessage(), e);
}
```
### Customer Data Retrieval
When retrieving customer data for an engagement, the SDK may encounter errors such as a missing or malformed customer data file, or an invalid file ID.
When retrieving customer data for an engagement, the SDK may encounter errors such as a missing or malformed customer data file, or an invalid batch ID.

In the case of a missing or malformed customer data file, the getCustomerBatchById() method will throw a RuntimeException with a message indicating the cause of the error. For example:


```java
try (DataFileStream<GenericRecord> dataFileReader = engagement.getCustomerBatchById(fileNumber)) {
try (DataFileStream<GenericRecord> dataFileReader = engagement.getCustomerBatchById(batchNumber)) {
// use dataFileReader
} catch (RuntimeException e) {
logger.error("Failed to get customer batch by id: " + e.getMessage(), e);
// handle error
}
```
If an invalid file ID is provided, the getCustomerBatchById() method will throw an IllegalArgumentException with a message indicating the cause of the error. For example:
If an invalid batch ID is provided, the getCustomerBatchById() method will throw an IllegalArgumentException with a message indicating the cause of the error. For example:


```java
try (DataFileStream<GenericRecord> dataFileReader = engagement.getCustomerBatchById(-1)) {
// use dataFileReader
} catch (IllegalArgumentException e) {
logger.error("Invalid file id: " + e.getMessage(), e);
logger.error("Invalid batch id: " + e.getMessage(), e);
// handle error
}
```
Expand Down
50 changes: 49 additions & 1 deletion src/main/java/optimove/sdk/engagement/Engagement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package optimove.sdk.engagement;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.cloud.storage.Blob;
import com.google.cloud.ReadChannel;

Expand All @@ -25,9 +26,56 @@ public Engagement(String decryptionKey, int tenantID, String bucketName, String
this.logger = logger;
}

// Constructor that accepts JSON string and logger
public Engagement(String jsonConfig, Logger logger) {
if (jsonConfig == null || logger == null) {
throw new IllegalArgumentException("Neither jsonConfig nor logger can be null");
}

try {
ObjectMapper mapper = new ObjectMapper();
EngagementConfig config = mapper.readValue(jsonConfig, EngagementConfig.class);
this.settings = createSettingsFromConfig(config);
this.logger = logger;
} catch (IOException e) {
throw new RuntimeException("Failed to parse engagement configuration: " + e.getMessage(), e);
}
}

// Constructor that accepts any object and logger
public Engagement(Object configObject, Logger logger) {
if (configObject == null || logger == null) {
throw new IllegalArgumentException("Neither configObject nor logger can be null");
}

try {
ObjectMapper mapper = new ObjectMapper();
// First convert the object to a JSON string
String jsonString = mapper.writeValueAsString(configObject);
// Then convert it to our EngagementConfig class
EngagementConfig config = mapper.readValue(jsonString, EngagementConfig.class);

this.settings = createSettingsFromConfig(config);
this.logger = logger;
} catch (IOException e) {
throw new RuntimeException("Failed to convert object to engagement configuration: " + e.getMessage(), e);
}
}
Comment on lines +30 to +63
Copy link

@baz-reviewer baz-reviewer bot Aug 27, 2025

Choose a reason for hiding this comment

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

Both constructors for Engagement (one taking a JSON string, one taking an object) repeat the same logic for mapping to EngagementConfig, creating settings, and assigning logger. Would it be more concise to extract the shared logic into a private method, reducing duplication and making future changes easier?

Suggested change
public Engagement(String jsonConfig, Logger logger) {
if (jsonConfig == null || logger == null) {
throw new IllegalArgumentException("Neither jsonConfig nor logger can be null");
}
try {
ObjectMapper mapper = new ObjectMapper();
EngagementConfig config = mapper.readValue(jsonConfig, EngagementConfig.class);
this.settings = createSettingsFromConfig(config);
this.logger = logger;
} catch (IOException e) {
throw new RuntimeException("Failed to parse engagement configuration: " + e.getMessage(), e);
}
}
// Constructor that accepts any object and logger
public Engagement(Object configObject, Logger logger) {
if (configObject == null || logger == null) {
throw new IllegalArgumentException("Neither configObject nor logger can be null");
}
try {
ObjectMapper mapper = new ObjectMapper();
// First convert the object to a JSON string
String jsonString = mapper.writeValueAsString(configObject);
// Then convert it to our EngagementConfig class
EngagementConfig config = mapper.readValue(jsonString, EngagementConfig.class);
this.settings = createSettingsFromConfig(config);
this.logger = logger;
} catch (IOException e) {
throw new RuntimeException("Failed to convert object to engagement configuration: " + e.getMessage(), e);
}
}
// Constructor that accepts JSON string and logger
public Engagement(String jsonConfig, Logger logger) {
if (jsonConfig == null || logger == null) {
throw new IllegalArgumentException("Neither jsonConfig nor logger can be null");
}
try {
ObjectMapper mapper = new ObjectMapper();
EngagementConfig config = mapper.readValue(jsonConfig, EngagementConfig.class);
initializeFromConfig(config, logger);
} catch (IOException e) {
throw new RuntimeException("Failed to parse engagement configuration: " + e.getMessage(), e);
}
}
// Constructor that accepts any object and logger
public Engagement(Object configObject, Logger logger) {
if (configObject == null || logger == null) {
throw new IllegalArgumentException("Neither configObject nor logger can be null");
}
try {
ObjectMapper mapper = new ObjectMapper();
// First convert the object to a JSON string
String jsonString = mapper.writeValueAsString(configObject);
// Then convert it to our EngagementConfig class
EngagementConfig config = mapper.readValue(jsonString, EngagementConfig.class);
initializeFromConfig(config, logger);
} catch (IOException e) {
throw new RuntimeException("Failed to convert object to engagement configuration: " + e.getMessage(), e);
}
}
// Helper method to initialize from config
private void initializeFromConfig(EngagementConfig config, Logger logger) {
this.settings = createSettingsFromConfig(config);
this.logger = logger;
}

Finding type: Conciseness


// Helper method to create settings from config
private EngagementSettings createSettingsFromConfig(EngagementConfig config) {
return new EngagementSettings(
config.getDecryptionKey(),
config.getTenantID(),
config.getBucketName(),
config.getCustomersFolderPath(),
config.getMetadataFilePath()
);
}

public Metadata getMetadata() {
try {
Blob fileBlob = StorageSingleton.getBlob(this.settings.getBucketName(), this.settings.getMetadataFilePath());
Blob fileBlob = StorageSingleton.getBlob(this.settings.getBucketName(), this.settings.getMetadataFilePath(), this.settings.getDecryptionKey());
String metadataString = StorageSingleton.blobToString(fileBlob, this.settings.getDecryptionKey());

return MetadataService.jsonStringToMetadata(metadataString);
Expand Down
49 changes: 49 additions & 0 deletions src/main/java/optimove/sdk/engagement/EngagementConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package optimove.sdk.engagement;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;

@JsonIgnoreProperties(ignoreUnknown = true)
class EngagementConfig {
@JsonProperty("DecryptionKey")
private String decryptionKey;

@JsonProperty("TenantID")
private int tenantID;

@JsonProperty("BucketName")
private String bucketName;

@JsonProperty("CustomersFolderPath")
private String customersFolderPath;

@JsonProperty("MetadataFilePath")
private String metadataFilePath;

@JsonProperty("EventTypeID")
private int eventTypeId;

@JsonProperty("TimeStamp")
private String timestamp;

@JsonProperty("CampaignID")
private int campaignId;

@JsonProperty("EngagementID")
private int engagementId;

@JsonProperty("ChannelID")
private int channelId;

// Getters
public String getDecryptionKey() { return decryptionKey; }
public int getTenantID() { return tenantID; }
public String getBucketName() { return bucketName; }
public String getCustomersFolderPath() { return customersFolderPath; }
public String getMetadataFilePath() { return metadataFilePath; }
public int getEventTypeId() { return eventTypeId; }
public String getTimestamp() { return timestamp; }
public int getCampaignId() { return campaignId; }
public int getEngagementId() { return engagementId; }
public int getChannelId() { return channelId; }
}
4 changes: 4 additions & 0 deletions src/main/java/optimove/sdk/engagement/Metadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ public long getNumberOfFiles() {
return NumberOfFiles;
}

public long getNumberOfBatches() {
return NumberOfFiles;
}

public long getPlanDetailChannelID() {
return PlanDetailChannelID;
}
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/optimove/sdk/engagement/StorageSingleton.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,22 @@
public class StorageSingleton {

private static Storage instance = null;
private static final String projectId = "optimoveSDK";

private StorageSingleton() { }

public static Storage getInstance() {
if(instance == null) {
instance = StorageOptions.newBuilder()
.setProjectId(projectId)
.build()
.getService();
}
return instance;
}

public static Blob getBlob(String bucketName, String srcFileName) {
return getInstance().get(bucketName, srcFileName);
return getInstance().get(BlobId.of(bucketName, srcFileName));
}
public static Blob getBlob(String bucketName, String srcFileName, String decryptionKey) {
return getInstance().get(BlobId.of(bucketName, srcFileName), Storage.BlobGetOption.decryptionKey(decryptionKey));
}
public static ReadChannel getReadChanel(String bucketName, String srcFileName) {
return getInstance().reader(BlobId.of(bucketName, srcFileName));
Expand Down
15 changes: 10 additions & 5 deletions src/main/java/org/example/Main.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,31 @@
import org.apache.avro.file.DataFileStream;
import org.apache.avro.generic.GenericRecord;
import java.io.IOException;
import java.time.Duration;
import java.time.Instant;

import optimove.sdk.engagement.Engagement;
import optimove.sdk.engagement.Metadata;

import org.apache.commons.lang3.time.StopWatch;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class Main {
public static void main(String[] args) {
Logger logger = LoggerFactory.getLogger(Main.class);

Engagement engagement = new Engagement(null, 1, "bucket-name", "customers-folder/customers", "metadata-path/metadata_287934", logger);
Engagement engagement = new Engagement("{\r\n \"EventTypeID\": 14,\r\n \"TimeStamp\": \"2025-01-08 08:58\",\r\n \"CampaignID\": 1111,\r\n \"EngagementID\": 1111,\r\n \"TenantID\": 1111,\r\n \"BucketName\": \"bucketName\",\r\n \"CustomersFolderPath\": \"path\",\r\n \"MetadataFilePath\": \"pathCustomers\",\r\n \"DecryptionKey\": \"some-secret-key\",\r\n \"ChannelID\": 1111\r\n}", logger);

Metadata metadata = engagement.getMetadata();
System.out.println(metadata.getNumberOfFiles());
System.out.println(metadata.getNumberOfBatches());

for (int fileNumber = 1; fileNumber <= metadata.getNumberOfFiles(); fileNumber++) {
System.out.println("file number: " + fileNumber);
for (int batchNumber = 1; batchNumber <= metadata.getNumberOfBatches(); batchNumber++) {
System.out.println("batch number: " + batchNumber);

try (DataFileStream<GenericRecord> dataFileReader = engagement.getCustomerBatchById(fileNumber)) {
DataFileStream<GenericRecord> dataFileReader = null;
try {
dataFileReader = engagement.getCustomerBatchById(batchNumber);
Comment on lines +28 to +30
Copy link

@baz-reviewer baz-reviewer bot Aug 27, 2025

Choose a reason for hiding this comment

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

The DataFileStream is no longer in a try-with-resources block. This could lead to resource leaks if exceptions occur before the stream is properly closed.

Suggested change
DataFileStream<GenericRecord> dataFileReader = null;
try {
dataFileReader = engagement.getCustomerBatchById(batchNumber);
try (DataFileStream<GenericRecord> dataFileReader = engagement.getCustomerBatchById(batchNumber)) {

Finding type: Logical Bugs

GenericRecord record = null;
while (dataFileReader.hasNext()) {
record = dataFileReader.next(record);
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/EngagementTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void test_retrieving_metadata_from_valid_bucket_and_file_path() {

try (MockedStatic<StorageSingleton> classMock = mockStatic(StorageSingleton.class)) {

classMock.when(() -> StorageSingleton.getBlob(anyString(), anyString())).thenReturn(fileBlob);
classMock.when(() -> StorageSingleton.getBlob(anyString(), anyString(), anyString())).thenReturn(fileBlob);
classMock.when(() -> StorageSingleton.blobToString(eq(fileBlob), any(String.class))).thenReturn(metadataString);
classMock.when(() -> StorageSingleton.getReadChanel(anyString(), anyString(), anyString())).thenReturn(readChannel);

Expand All @@ -68,7 +68,7 @@ public void test_retrieving_metadata_from_valid_bucket_and_file_path() {

// Verify the expected behavior
assertNotNull(metadata);
assumeTrue(metadata.getNumberOfFiles() == 3);
assumeTrue(metadata.getNumberOfBatches() == 3);
assumeTrue(metadata.getNumberOfCustomers() == 300);
assumeTrue(metadata.getCampaignPlanID() == 10);
assumeTrue(metadata.getCampaignID() == 1);
Expand Down
Loading