Repository navigation
Support percentage-weighted choices in custom YAML providers #1962
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -142,6 +142,9 @@ public void addUrl(Locale locale, URL url) { | |
| public Object fetch(String key, FakerContext context) { | ||
| List<?> valuesArray = null; | ||
| final Object o = fetchObject(key, context); | ||
| if (o instanceof WeightedYamlValues weighted) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Also, branch strings skip the |
||
| return weighted.select(context.getRandomService()); | ||
| } | ||
| if (o instanceof List<?> list) { | ||
| valuesArray = list; | ||
| final int size = valuesArray.size(); | ||
|
|
@@ -196,6 +199,9 @@ public String safeFetch(String key, FakerContext context, @Nullable String defau | |
| Object o = fetchObject(key, context); | ||
| String str; | ||
| if (o == null) return defaultIfNull; | ||
| if (o instanceof WeightedYamlValues weighted) { | ||
| return weighted.select(context.getRandomService()); | ||
| } | ||
| if (o instanceof List<?> list) { | ||
| final List<String> values = (List<String>) list; | ||
| final int size = values.size(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| package net.datafaker.service; | ||
|
|
||
| import org.jspecify.annotations.Nullable; | ||
|
|
||
| import java.math.BigDecimal; | ||
| import java.util.ArrayList; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Random; | ||
| import java.util.function.UnaryOperator; | ||
|
|
||
| /** A custom YAML choice, parsed once but sampled using the current Faker's random service. */ | ||
| final class WeightedYamlValues { | ||
| private final List<Map<String, Object>> choices; | ||
|
|
||
| private WeightedYamlValues(List<Map<String, Object>> choices) { | ||
| this.choices = List.copyOf(choices); | ||
| } | ||
|
|
||
| @Nullable | ||
| static WeightedYamlValues from(List<?> values, UnaryOperator<String> qualify) { | ||
| // Ordinary lists (including lists of maps) retain their existing meaning. | ||
| boolean weighted = values.stream().anyMatch(value -> value instanceof Map<?, ?> map | ||
| && map.keySet().stream().anyMatch(key -> key instanceof String s && s.endsWith("%"))); | ||
| if (!weighted) return null; | ||
|
|
||
| List<Map<String, Object>> choices = new ArrayList<>(values.size()); | ||
| var uniqueValues = new HashSet<Object>(); | ||
| BigDecimal total = BigDecimal.ZERO; | ||
| for (Object value : values) { | ||
| if (!(value instanceof Map<?, ?> map) || map.size() != 1) { | ||
| throw invalid("each entry must be a single percentage-to-value mapping"); | ||
| } | ||
| var entry = map.entrySet().iterator().next(); | ||
| if (!(entry.getKey() instanceof String key) || !key.matches("[0-9]+(?:\\.[0-9]+)?%")) { | ||
| throw invalid("percentage keys must be decimal strings such as '60%'"); | ||
| } | ||
| BigDecimal percentage = new BigDecimal(key.substring(0, key.length() - 1)); | ||
| double weight = percentage.doubleValue(); | ||
| if (percentage.signum() <= 0 || !Double.isFinite(weight) || weight <= 0) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: the key regex |
||
| throw invalid("percentages must be finite and positive"); | ||
| } | ||
| total = total.add(percentage); | ||
| Object branch = entry.getValue(); | ||
| Object qualified; | ||
| if (branch instanceof String string) { | ||
| qualified = qualify.apply(string); | ||
| } else if (branch instanceof List<?> list && !list.isEmpty()) { | ||
| List<String> strings = new ArrayList<>(list.size()); | ||
| for (Object item : list) { | ||
| if (!(item instanceof String string)) { | ||
| throw invalid("list branches must contain only string templates"); | ||
| } | ||
| strings.add(qualify.apply(string)); | ||
| } | ||
| qualified = List.copyOf(strings); | ||
| } else { | ||
| throw invalid("each branch must be a string template or a non-empty list of string templates"); | ||
| } | ||
| if (!uniqueValues.add(qualified)) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This uniqueness rule exists only because |
||
| throw invalid("branch values must be unique, as required by WeightedRandomSelector"); | ||
| } | ||
| choices.add(Map.of("weight", weight, "value", qualified)); | ||
| } | ||
| if (total.compareTo(BigDecimal.valueOf(100)) != 0) { | ||
| throw invalid("percentages must total 100"); | ||
| } | ||
| return new WeightedYamlValues(choices); | ||
| } | ||
|
|
||
| String select(RandomService random) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This subclasses Suggestion: compute Object selected = WeightedRandomSelector.selectWeightedElement(random.nextDouble() * total, cumulative, values);( |
||
| // The Java provider API accepts Random, while RandomService also supports other | ||
| // RandomGenerators. Forward its draw without creating an independent random stream. | ||
| Random delegate = new Random(0) { | ||
| @Override | ||
| public double nextDouble() { | ||
| return random.nextDouble(); | ||
| } | ||
| }; | ||
| Object selected = new WeightedRandomSelector(delegate).select(choices); | ||
| if (selected instanceof List<?> list) { | ||
| return (String) list.get(list.size() == 1 ? 0 : random.nextInt(list.size())); | ||
| } | ||
| return (String) selected; | ||
| } | ||
|
|
||
| private static IllegalArgumentException invalid(String reason) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error doesn't say which key or file is broken. Values are loaded lazily, one whole file at a time, so |
||
| return new IllegalArgumentException("Invalid weighted YAML choices: " + reason); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| package net.datafaker.service; | ||
|
|
||
| import net.datafaker.providers.base.BaseFaker; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.io.TempDir; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.util.Locale; | ||
| import java.util.Random; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
|
||
| class WeightedYamlProviderTest { | ||
| @TempDir | ||
| Path directory; | ||
|
|
||
| @Test | ||
| void resolvesCustomWeightedYamlAndPreservesOrdinaryValues() { | ||
| BaseFaker faker = custom(new Random(1)); | ||
| faker.addUrl(Locale.ENGLISH, getClass().getResource("/weighted-custom.yml")); | ||
| for (int i = 0; i < 20; i++) { | ||
| assertThat(faker.resolve("weighted_custom.strings")).isIn("first", "second"); | ||
| assertThat(faker.resolve("weighted_custom.domains")).isIn(".it", ".ee", ".lv", ".lt"); | ||
| assertThat(faker.resolve("weighted_custom.local_template")).isEqualTo("chosen!"); | ||
| assertThat(faker.resolve("weighted_custom.reference")).isNotEmpty(); | ||
| assertThat(faker.resolve("weighted_custom.ordinary")).isIn("a", "b"); | ||
| assertThat(faker.resolve("weighted_custom.scalar")).isEqualTo("plain"); | ||
| assertThat(faker.resolve("weighted_custom.nested.value")).isEqualTo("nested"); | ||
| } | ||
| assertThat(faker.fakeValuesService().fetchString("weighted_custom.strings", faker.getContext())).isIn("first", "second"); | ||
| Object ordinary = faker.fakeValuesService().fetchObject("weighted_custom.ordinary", faker.getContext()); | ||
| assertThat(ordinary).isEqualTo(java.util.List.of("a", "b")); | ||
| assertThat(faker.fakeValuesService().safeFetch("weighted_custom.empty", faker.getContext(), "fallback")).isEqualTo("fallback"); | ||
| // Custom data is merged with the ordinary locale fallback, not substituted for it. | ||
| assertThat(faker.resolve("name.first_name")).isNotEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void sameSeedAndSharedYamlDoNotShareSamples() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name says "do not share samples", but the test asserts the two outputs are equal. Maybe |
||
| BaseFaker first = custom(new Random(123)); | ||
| BaseFaker second = custom(new Random(123)); | ||
| for (BaseFaker faker : java.util.List.of(first, second)) { | ||
| faker.addUrl(Locale.ENGLISH, getClass().getResource("/weighted-custom.yml")); | ||
| } | ||
| for (int i = 0; i < 100; i++) { | ||
| assertThat(first.resolve("weighted_custom.domains")).isEqualTo(second.resolve("weighted_custom.domains")); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| void customSourcesAndRandomServicesAreIsolated() throws IOException { | ||
| Path a = directory.resolve("a.yml"); | ||
| Path b = directory.resolve("b.yml"); | ||
| Files.writeString(a, "en:\n faker:\n weighted_custom:\n value:\n - '100%': 'source-a'\n"); | ||
| Files.writeString(b, "en:\n faker:\n weighted_custom:\n value:\n - '100%': 'source-b'\n"); | ||
| BaseFaker first = custom(new Random(1)); | ||
| BaseFaker second = custom(new Random(1)); | ||
| first.addPath(Locale.ENGLISH, a); | ||
| second.addPath(Locale.ENGLISH, b); | ||
| assertThat(first.resolve("weighted_custom.value")).isEqualTo("source-a"); | ||
| assertThat(second.resolve("weighted_custom.value")).isEqualTo("source-b"); | ||
| first.getContext().setRandomService(new RandomService(new Random(2))); | ||
| assertThat(first.resolve("weighted_custom.value")).isEqualTo("source-a"); | ||
| } | ||
|
|
||
| @Test | ||
| void preservesIntegerKeysInOrdinaryNestedMaps() throws IOException { | ||
| Path file = directory.resolve("integer-keys.yml"); | ||
| Files.writeString(file, """ | ||
| en: | ||
| faker: | ||
| weighted_custom: | ||
| sibling: 'ordinary sibling' | ||
| original: | ||
| 1: 'one' | ||
| 2: ['two', 'deux'] | ||
| """); | ||
| BaseFaker faker = custom(new Random(1)); | ||
| faker.addPath(Locale.ENGLISH, file); | ||
| assertThat(faker.resolve("weighted_custom.sibling")).isEqualTo("ordinary sibling"); | ||
| Object original = faker.fakeValuesService().fetchObject("weighted_custom.original", faker.getContext()); | ||
| assertThat(original).isEqualTo(java.util.Map.of(1, "one", 2, java.util.List.of("two", "deux"))); | ||
| } | ||
|
|
||
| @Test | ||
| void malformedCustomYamlFailsClearly() throws IOException { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once the error message includes the key, it would be good to assert that here as well (e.g. |
||
| Path bad = directory.resolve("bad.yml"); | ||
| Files.writeString(bad, "en:\n faker:\n weighted_custom:\n value:\n - '60%': 'a'\n - '30%': 'b'\n"); | ||
| BaseFaker faker = custom(new Random(1)); | ||
| assertThatThrownBy(() -> { | ||
| faker.addPath(Locale.ENGLISH, bad); | ||
| faker.resolve("weighted_custom.value"); | ||
| }) | ||
| .isInstanceOf(IllegalArgumentException.class).hasMessageContaining("percentages must total 100"); | ||
| } | ||
|
|
||
| private static BaseFaker custom(Random random) { | ||
| return new BaseFaker(Locale.ENGLISH, random); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quoting isn't required: in YAML,
- 60%: xalready reads the key as a string. This section is also quite long; the last two paragraphs could be one sentence each.