Skip to content

Commit d3f174e

Browse files
committed
Enforce read-only contract on shared FakeValuesService instances
addPath and addUrl are non-idempotent mutators: calling them on a shared instance would silently affect every consumer of that singleton. Address this by adding a volatile boolean `shared` flag set by getShared(), and guarding both methods with an UnsupportedOperationException fail-fast. Also rewrites the getShared Javadoc to accurately describe the design: the locale parameter is a cache-partition key (not a constructor arg), YAML is loaded lazily by BaseFaker after construction, and mixing locales between getShared and withSharedService is unsupported. Adds sharedInstanceRejectsAddPathAndAddUrl test to cover both guards.
1 parent 78552fb commit d3f174e

2 files changed

Lines changed: 34 additions & 5 deletions

File tree

src/main/java/net/datafaker/service/FakeValuesService.java

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,28 @@ public class FakeValuesService {
8686

8787
private static final ConcurrentHashMap<Locale, FakeValuesService> SHARED_INSTANCES = new ConcurrentHashMap<>();
8888

89+
private volatile boolean shared = false;
90+
8991
private final Map<RegExpContext, ValueResolver> REGEXP2SUPPLIER_MAP = new CopyOnWriteMap<>(HashMap::new);
9092

9193
/**
92-
* Returns a lazily-initialized per-locale singleton. Safe to share across threads:
93-
* all mutable instance state uses idempotent copy-on-write caches.
94+
* Returns a lazily-initialized per-locale singleton safe to share across threads.
95+
* <p>
96+
* The {@code locale} is used as a cache-partition key: all callers passing the same
97+
* locale receive the same instance. Locale-specific YAML data is loaded lazily the
98+
* first time a Faker backed by this service is constructed, so there is no need to
99+
* pre-warm the instance.
100+
* <p>
101+
* Shared instances are read-only: {@link #addPath} and {@link #addUrl} will throw
102+
* {@link UnsupportedOperationException}. Mixing locales — e.g. passing
103+
* {@code getShared(Locale.ENGLISH)} to
104+
* {@link net.datafaker.Faker#withSharedService(FakeValuesService, Locale, Random)}
105+
* with a different locale — is unsupported.
94106
*/
95107
public static FakeValuesService getShared(Locale locale) {
96-
return SHARED_INSTANCES.computeIfAbsent(locale, l -> new FakeValuesService());
108+
FakeValuesService svc = SHARED_INSTANCES.computeIfAbsent(locale, l -> new FakeValuesService());
109+
svc.shared = true;
110+
return svc;
97111
}
98112

99113
public void updateFakeValuesInterfaceMap(List<SingletonLocale> locales) {
@@ -115,9 +129,11 @@ private FakeValuesInterface getCachedFakeValue(SingletonLocale locale) {
115129
*
116130
* @param locale the locale for which a path is going to be added.
117131
* @param path path to a file with YAML structure
118-
* @throws IllegalArgumentException in case of invalid path
132+
* @throws IllegalArgumentException in case of invalid path
133+
* @throws UnsupportedOperationException if called on a shared instance obtained via {@link #getShared(Locale)}
119134
*/
120135
public void addPath(Locale locale, Path path) {
136+
if (shared) throw new UnsupportedOperationException("addPath cannot be called on a shared FakeValuesService");
121137
requireNonNull(locale);
122138
if (path == null || Files.notExists(path) || Files.isDirectory(path) || !Files.isReadable(path)) {
123139
throw new IllegalArgumentException("Path should be an existing readable file: \"%s\"".formatted(path));
@@ -134,9 +150,11 @@ public void addPath(Locale locale, Path path) {
134150
*
135151
* @param locale the locale for which an url is going to be added.
136152
* @param url url of a file with YAML structure
137-
* @throws IllegalArgumentException in case of invalid url
153+
* @throws IllegalArgumentException in case of invalid url
154+
* @throws UnsupportedOperationException if called on a shared instance obtained via {@link #getShared(Locale)}
138155
*/
139156
public void addUrl(Locale locale, URL url) {
157+
if (shared) throw new UnsupportedOperationException("addUrl cannot be called on a shared FakeValuesService");
140158
requireNonNull(locale);
141159
if (url == null) {
142160
throw new IllegalArgumentException("url should be an existing readable file");

src/test/java/net/datafaker/SharedFakeValuesServiceTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.util.concurrent.TimeUnit;
1515

1616
import static org.assertj.core.api.Assertions.assertThat;
17+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1718

1819
class SharedFakeValuesServiceTest {
1920

@@ -65,6 +66,16 @@ void concurrentFakersWithSharedServiceProduceNoErrors() throws Exception {
6566
}
6667
}
6768

69+
@Test
70+
void sharedInstanceRejectsAddPathAndAddUrl() throws Exception {
71+
FakeValuesService shared = FakeValuesService.getShared(Locale.GERMAN);
72+
assertThatThrownBy(() -> shared.addPath(Locale.GERMAN, java.nio.file.Path.of("nonexistent.yml")))
73+
.isInstanceOf(UnsupportedOperationException.class);
74+
java.net.URL url = new java.net.URI("file:///nonexistent.yml").toURL();
75+
assertThatThrownBy(() -> shared.addUrl(Locale.GERMAN, url))
76+
.isInstanceOf(UnsupportedOperationException.class);
77+
}
78+
6879
@Test
6980
void withSharedServiceOutputMatchesNormalFaker() {
7081
long seed = 12345L;

0 commit comments

Comments
 (0)