AB#238930

[maryan-opti]

Generated description

Below is a concise technical summary of the changes proposed in this PR:

graph LR
main_("main"):::modified
Engagement_Engagement_("Engagement.Engagement"):::added
Engagement_getMetadata_("Engagement.getMetadata"):::modified
StorageSingleton_getBlob_("StorageSingleton.getBlob"):::modified
StorageSingleton_getInstance_("StorageSingleton.getInstance"):::modified
getNumberOfBatches_("getNumberOfBatches"):::added
main_ -- "Replaced constructor with JSON config for flexible initialization" --> Engagement_Engagement_
main_ -- "Fetches metadata blob using decryption key for security" --> Engagement_getMetadata_
Engagement_getMetadata_ -- "Added decryption key parameter to securely fetch blob" --> StorageSingleton_getBlob_
StorageSingleton_getBlob_ -- "Removed project ID setting in storage instance creation" --> StorageSingleton_getInstance_
main_ -- "Renamed method to reflect batch processing semantics" --> getNumberOfBatches_
classDef added stroke:#15AA7A
classDef removed stroke:#CD5270
classDef modified stroke:#EDAC4C
linkStyle default stroke:#CBD5E1,font-size:13px

Loading

Introduce a new JSON-based configuration mechanism for the Engagement SDK, allowing for more flexible initialization of the Engagement component by parsing configuration details from a JSON string or object. Integrate the decryption key directly into Google Cloud Storage blob retrieval operations, enhancing secure data access, and update internal terminology from "files" to "batches" for clarity.

Topic	Details
Update Terminology	Update the SDK's public API and examples to use "batches" instead of "files" for clarity, including changes in Metadata methods and example code. Modified files (4) src/test/java/EngagementTest.java readme.md src/main/java/org/example/Main.java src/main/java/optimove/sdk/engagement/Metadata.java Latest Contributors(0) User Commit Date
Configure SDK	Introduce new constructors for the Engagement class that accept a JSON string or an object to configure the SDK, leveraging the new EngagementConfig class for structured configuration, and pass the decryption key directly to StorageSingleton for secure Google Cloud Storage blob retrieval. Modified files (7) src/test/java/EngagementTest.java src/main/java/optimove/sdk/engagement/Engagement.java readme.md src/main/java/org/example/Main.java pom.xml src/main/java/optimove/sdk/engagement/EngagementConfig.java src/main/java/optimove/sdk/engagement/StorageSingleton.java Latest Contributors(0) User Commit Date

This pull request is reviewed by Baz. Review like a pro on (Baz).

[baz-reviewer]

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

[baz-reviewer]

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