From 490d20427a7cb190231a38302435ed2159f49609 Mon Sep 17 00:00:00 2001 From: Brian Xiao Date: Tue, 11 May 2021 16:09:38 -0400 Subject: [PATCH 1/3] merge now updates provider in bulletconfig --- .../java/com/yahoo/bullet/common/BulletConfig.java | 1 + .../com/yahoo/bullet/common/BulletConfigTest.java | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/main/java/com/yahoo/bullet/common/BulletConfig.java b/src/main/java/com/yahoo/bullet/common/BulletConfig.java index 73dc40d0..44af6fd0 100644 --- a/src/main/java/com/yahoo/bullet/common/BulletConfig.java +++ b/src/main/java/com/yahoo/bullet/common/BulletConfig.java @@ -398,6 +398,7 @@ public static Validator getValidator() { public void merge(Config other) { super.merge(other); validate(); + provider = BulletRecordProvider.from(getAs(RECORD_PROVIDER_CLASS_NAME, String.class)); } /** diff --git a/src/test/java/com/yahoo/bullet/common/BulletConfigTest.java b/src/test/java/com/yahoo/bullet/common/BulletConfigTest.java index 9b4f752d..800ed293 100644 --- a/src/test/java/com/yahoo/bullet/common/BulletConfigTest.java +++ b/src/test/java/com/yahoo/bullet/common/BulletConfigTest.java @@ -8,6 +8,8 @@ import com.yahoo.bullet.querying.partitioning.MockPartitioner; import com.yahoo.bullet.record.BulletRecord; import com.yahoo.bullet.record.BulletRecordProvider; +import com.yahoo.bullet.record.avro.TypedAvroBulletRecordProvider; +import com.yahoo.bullet.record.simple.TypedSimpleBulletRecordProvider; import com.yahoo.bullet.result.Meta; import com.yahoo.bullet.result.Meta.Concept; import com.yahoo.bullet.typesystem.Type; @@ -171,6 +173,17 @@ public void testMerging() { Assert.assertEquals(config.get("pi"), 3.14); } + @Test + public void testMergingDifferentBulletRecordProvider() { + BulletConfig config = new BulletConfig(); + Assert.assertTrue(config.getBulletRecordProvider() instanceof TypedAvroBulletRecordProvider); + + Config another = new BulletConfig(null); + another.set(BulletConfig.RECORD_PROVIDER_CLASS_NAME, "com.yahoo.bullet.record.simple.TypedSimpleBulletRecordProvider"); + config.merge(another); + Assert.assertTrue(config.getBulletRecordProvider() instanceof TypedSimpleBulletRecordProvider); + } + @Test public void testPropertiesWithPrefix() { BulletConfig config = new BulletConfig("src/test/resources/test_config.yaml"); From 1afdfd9f31b9936ba781bd91ca523b5618bc2e1f Mon Sep 17 00:00:00 2001 From: Brian Xiao Date: Tue, 11 May 2021 17:58:38 -0400 Subject: [PATCH 2/3] getcachedbulletrecordprovider --- .../com/yahoo/bullet/common/BulletConfig.java | 25 ++++++++--- .../yahoo/bullet/common/BulletConfigTest.java | 42 +++++++++++++------ 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/yahoo/bullet/common/BulletConfig.java b/src/main/java/com/yahoo/bullet/common/BulletConfig.java index 44af6fd0..4fee7395 100644 --- a/src/main/java/com/yahoo/bullet/common/BulletConfig.java +++ b/src/main/java/com/yahoo/bullet/common/BulletConfig.java @@ -337,7 +337,6 @@ public class BulletConfig extends Config { public BulletConfig(String file) { super(file, DEFAULT_CONFIGURATION_NAME); VALIDATOR.validate(this); - provider = BulletRecordProvider.from(getAs(RECORD_PROVIDER_CLASS_NAME, String.class)); } /** @@ -346,19 +345,36 @@ public BulletConfig(String file) { public BulletConfig() { super(DEFAULT_CONFIGURATION_NAME); VALIDATOR.validate(this); - provider = BulletRecordProvider.from(getAs(RECORD_PROVIDER_CLASS_NAME, String.class)); } /** - * Get the {@link BulletRecordProvider} stored in this BulletConfig instance. This BulletRecordProvider is - * created when this BulletConfig is constructed. + * Construct a {@link BulletRecordProvider} and store it in this BulletConfig instance. * * @return The BulletRecordProvider instance. */ public BulletRecordProvider getBulletRecordProvider() { + provider = BulletRecordProvider.from(getAs(RECORD_PROVIDER_CLASS_NAME, String.class)); return provider; } + /** + * Get the {@link BulletRecordProvider} stored in this BulletConfig instance, or construct and store one first if + * there is none. + * + * @return The BulletRecordProvider instance. + */ + public BulletRecordProvider getCachedBulletRecordProvider() { + if (provider == null) { + return getBulletRecordProvider(); + } + return provider; + } + + /** + * Construct a {@link Schema} if configured. + * + * @return The Schema instance if configured; otherwise, null. + */ public Schema getSchema() { String schemaFile = getAs(RECORD_SCHEMA_FILE_NAME, String.class); if (schemaFile == null) { @@ -398,7 +414,6 @@ public static Validator getValidator() { public void merge(Config other) { super.merge(other); validate(); - provider = BulletRecordProvider.from(getAs(RECORD_PROVIDER_CLASS_NAME, String.class)); } /** diff --git a/src/test/java/com/yahoo/bullet/common/BulletConfigTest.java b/src/test/java/com/yahoo/bullet/common/BulletConfigTest.java index 800ed293..b2ae5e5f 100644 --- a/src/test/java/com/yahoo/bullet/common/BulletConfigTest.java +++ b/src/test/java/com/yahoo/bullet/common/BulletConfigTest.java @@ -173,17 +173,6 @@ public void testMerging() { Assert.assertEquals(config.get("pi"), 3.14); } - @Test - public void testMergingDifferentBulletRecordProvider() { - BulletConfig config = new BulletConfig(); - Assert.assertTrue(config.getBulletRecordProvider() instanceof TypedAvroBulletRecordProvider); - - Config another = new BulletConfig(null); - another.set(BulletConfig.RECORD_PROVIDER_CLASS_NAME, "com.yahoo.bullet.record.simple.TypedSimpleBulletRecordProvider"); - config.merge(another); - Assert.assertTrue(config.getBulletRecordProvider() instanceof TypedSimpleBulletRecordProvider); - } - @Test public void testPropertiesWithPrefix() { BulletConfig config = new BulletConfig("src/test/resources/test_config.yaml"); @@ -474,11 +463,13 @@ public void testGetBulletRecordProvider() { BulletConfig config = new BulletConfig(); BulletRecordProvider providerA = config.getBulletRecordProvider(); BulletRecordProvider providerB = config.getBulletRecordProvider(); - Assert.assertEquals(providerA, providerB); + + // Creates a new provider each time + Assert.assertNotEquals(providerA, providerB); // Ensure the provider generates new records each time BulletRecord recordA = providerA.getInstance(); - BulletRecord recordB = providerB.getInstance(); + BulletRecord recordB = providerA.getInstance(); Assert.assertNotNull(recordA); Assert.assertNotNull(recordB); @@ -488,6 +479,31 @@ public void testGetBulletRecordProvider() { Assert.assertTrue(recordA.typedGet("someField").isNull()); } + @Test + public void testGetCachedBulletRecordProvider() { + BulletConfig config = new BulletConfig(); + BulletRecordProvider providerA = config.getCachedBulletRecordProvider(); + BulletRecordProvider providerB = config.getCachedBulletRecordProvider(); + + // Uses the same provider + Assert.assertEquals(providerA, providerB); + } + + @Test + public void testSettingDifferentBulletRecordProvider() { + BulletConfig config = new BulletConfig(); + + // Default record provider is TypedAvroBulletRecordProvider + Assert.assertTrue(config.getBulletRecordProvider() instanceof TypedAvroBulletRecordProvider); + + config.set(BulletConfig.RECORD_PROVIDER_CLASS_NAME, "com.yahoo.bullet.record.simple.TypedSimpleBulletRecordProvider"); + + // Cached record provider doesn't change with new setting + Assert.assertTrue(config.getCachedBulletRecordProvider() instanceof TypedAvroBulletRecordProvider); + + Assert.assertTrue(config.getBulletRecordProvider() instanceof TypedSimpleBulletRecordProvider); + } + @Test(expectedExceptions = IllegalStateException.class) public void testEqualityPartitioningWithNoFieldsValidation() { BulletConfig config = new BulletConfig(); From a467e3a02f651d8ae22278b748bee48ac1655a92 Mon Sep 17 00:00:00 2001 From: Brian Xiao Date: Wed, 12 May 2021 15:11:42 -0400 Subject: [PATCH 3/3] pr comments --- src/test/java/com/yahoo/bullet/common/BulletConfigTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/yahoo/bullet/common/BulletConfigTest.java b/src/test/java/com/yahoo/bullet/common/BulletConfigTest.java index b2ae5e5f..3de40861 100644 --- a/src/test/java/com/yahoo/bullet/common/BulletConfigTest.java +++ b/src/test/java/com/yahoo/bullet/common/BulletConfigTest.java @@ -486,7 +486,7 @@ public void testGetCachedBulletRecordProvider() { BulletRecordProvider providerB = config.getCachedBulletRecordProvider(); // Uses the same provider - Assert.assertEquals(providerA, providerB); + Assert.assertSame(providerA, providerB); } @Test @@ -496,7 +496,7 @@ public void testSettingDifferentBulletRecordProvider() { // Default record provider is TypedAvroBulletRecordProvider Assert.assertTrue(config.getBulletRecordProvider() instanceof TypedAvroBulletRecordProvider); - config.set(BulletConfig.RECORD_PROVIDER_CLASS_NAME, "com.yahoo.bullet.record.simple.TypedSimpleBulletRecordProvider"); + config.set(BulletConfig.RECORD_PROVIDER_CLASS_NAME, TypedSimpleBulletRecordProvider.class.getName()); // Cached record provider doesn't change with new setting Assert.assertTrue(config.getCachedBulletRecordProvider() instanceof TypedAvroBulletRecordProvider);