Skip to content

Commit 473a981

Browse files
aepfliclaude
andcommitted
feat: Clean up builder patterns and remove unnecessary convenience methods
Remove problematic convenience methods and enforce consistent builder usage: - Remove EventDetails.fromProviderEventDetails() static methods - Remove HookContext.from() static method - Remove FlagEvaluationDetails.from() static method - Update all usages to use .builder() pattern consistently - Add required imports for Optional and ImmutableMetadata Benefits: ✅ Consistent API - single builder() method per class ✅ No confusion between convenience methods and builders ✅ More explicit and discoverable API surface ✅ Better IDE autocompletion and IntelliSense ✅ Easier to maintain and understand Breaking change: Convenience methods removed in favor of builder pattern 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Simon Schrottner <[email protected]>
1 parent d4b7dbe commit 473a981

File tree

6 files changed

+50
-91
lines changed

6 files changed

+50
-91
lines changed

openfeature-api/src/main/java/dev/openfeature/api/EventDetails.java

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -209,32 +209,4 @@ public EventDetails build() {
209209
}
210210
}
211211

212-
/**
213-
* Create EventDetails from ProviderEventDetails with provider name.
214-
*
215-
* @param providerEventDetails the provider event details
216-
* @param providerName the name of the provider
217-
* @return EventDetails instance
218-
*/
219-
public static EventDetails fromProviderEventDetails(
220-
ProviderEventDetails providerEventDetails, String providerName) {
221-
return fromProviderEventDetails(providerEventDetails, providerName, null);
222-
}
223-
224-
/**
225-
* Create EventDetails from ProviderEventDetails with provider name and domain.
226-
*
227-
* @param providerEventDetails the provider event details
228-
* @param providerName the name of the provider
229-
* @param domain the domain associated with the event
230-
* @return EventDetails instance
231-
*/
232-
public static EventDetails fromProviderEventDetails(
233-
ProviderEventDetails providerEventDetails, String providerName, String domain) {
234-
return builder()
235-
.providerName(providerName)
236-
.domain(domain)
237-
.providerEventDetails(providerEventDetails)
238-
.build();
239-
}
240212
}

openfeature-api/src/main/java/dev/openfeature/api/FlagEvaluationDetails.java

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -202,24 +202,4 @@ public FlagEvaluationDetails<T> build() {
202202
}
203203
}
204204

205-
/**
206-
* Generate detail payload from the provider response.
207-
*
208-
* @param providerEval provider response
209-
* @param flagKey key for the flag being evaluated
210-
* @param <T> type of flag being returned
211-
* @return detail payload
212-
*/
213-
public static <T> FlagEvaluationDetails<T> from(ProviderEvaluation<T> providerEval, String flagKey) {
214-
return FlagEvaluationDetails.<T>builder()
215-
.flagKey(flagKey)
216-
.value(providerEval.getValue())
217-
.variant(providerEval.getVariant())
218-
.reason(providerEval.getReason())
219-
.errorMessage(providerEval.getErrorMessage())
220-
.errorCode(providerEval.getErrorCode())
221-
.flagMetadata(Optional.ofNullable(providerEval.getFlagMetadata())
222-
.orElse(ImmutableMetadata.builder().build()))
223-
.build();
224-
}
225205
}

openfeature-api/src/main/java/dev/openfeature/api/HookContext.java

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -52,34 +52,6 @@ public static <T> Builder<T> builder() {
5252
return new Builder<>();
5353
}
5454

55-
/**
56-
* Builds a {@link HookContext} instances from request data.
57-
*
58-
* @param key feature flag key
59-
* @param type flag value type
60-
* @param clientMetadata info on which client is calling
61-
* @param providerMetadata info on the provider
62-
* @param ctx Evaluation Context for the request
63-
* @param defaultValue Fallback value
64-
* @param <T> type that the flag is evaluating against
65-
* @return resulting context for hook
66-
*/
67-
public static <T> HookContext<T> from(
68-
String key,
69-
FlagValueType type,
70-
ClientMetadata clientMetadata,
71-
Metadata providerMetadata,
72-
EvaluationContext ctx,
73-
T defaultValue) {
74-
return HookContext.<T>builder()
75-
.flagKey(key)
76-
.type(type)
77-
.clientMetadata(clientMetadata)
78-
.providerMetadata(providerMetadata)
79-
.ctx(ctx)
80-
.defaultValue(defaultValue)
81-
.build();
82-
}
8355

8456
@Override
8557
public boolean equals(Object o) {

openfeature-sdk/src/main/java/dev/openfeature/sdk/DefaultOpenFeatureAPI.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -473,19 +473,30 @@ private void runHandlersForProvider(FeatureProvider provider, ProviderEvent even
473473
.orElse("unknown");
474474

475475
// run the global handlers
476-
eventSupport.runGlobalHandlers(event, EventDetails.fromProviderEventDetails(details, providerName));
476+
eventSupport.runGlobalHandlers(event, EventDetails.builder()
477+
.providerName(providerName)
478+
.providerEventDetails(details)
479+
.build());
477480

478481
// run the handlers associated with domains for this provider
479482
domainsForProvider.forEach(domain -> eventSupport.runClientHandlers(
480-
domain, event, EventDetails.fromProviderEventDetails(details, providerName, domain)));
483+
domain, event, EventDetails.builder()
484+
.providerName(providerName)
485+
.domain(domain)
486+
.providerEventDetails(details)
487+
.build()));
481488

482489
if (providerRepository.isDefaultProvider(provider)) {
483490
// run handlers for clients that have no bound providers (since this is the default)
484491
Set<String> allDomainNames = eventSupport.getAllDomainNames();
485492
Set<String> boundDomains = providerRepository.getAllBoundDomains();
486493
allDomainNames.removeAll(boundDomains);
487494
allDomainNames.forEach(domain -> eventSupport.runClientHandlers(
488-
domain, event, EventDetails.fromProviderEventDetails(details, providerName, domain)));
495+
domain, event, EventDetails.builder()
496+
.providerName(providerName)
497+
.domain(domain)
498+
.providerEventDetails(details)
499+
.build()));
489500
}
490501
}
491502
}

openfeature-sdk/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import dev.openfeature.api.Hook;
1313
import dev.openfeature.api.HookContext;
1414
import dev.openfeature.api.ImmutableContext;
15+
import dev.openfeature.api.ImmutableMetadata;
1516
import dev.openfeature.api.ImmutableStructure;
1617
import dev.openfeature.api.ProviderEvaluation;
1718
import dev.openfeature.api.ProviderEvent;
@@ -33,6 +34,7 @@
3334
import java.util.List;
3435
import java.util.Map;
3536
import java.util.Objects;
37+
import java.util.Optional;
3638
import java.util.concurrent.ConcurrentLinkedQueue;
3739
import java.util.concurrent.atomic.AtomicReference;
3840
import java.util.function.Consumer;
@@ -201,18 +203,25 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
201203

202204
var mergedCtx = hookSupport.beforeHooks(
203205
type,
204-
HookContext.from(
205-
key,
206-
type,
207-
this.getMetadata(),
208-
provider.getMetadata(),
209-
mergeEvaluationContext(ctx),
210-
defaultValue),
206+
HookContext.<T>builder()
207+
.flagKey(key)
208+
.type(type)
209+
.clientMetadata(this.getMetadata())
210+
.providerMetadata(provider.getMetadata())
211+
.ctx(mergeEvaluationContext(ctx))
212+
.defaultValue(defaultValue)
213+
.build(),
211214
mergedHooks,
212215
hints);
213216

214-
afterHookContext =
215-
HookContext.from(key, type, this.getMetadata(), provider.getMetadata(), mergedCtx, defaultValue);
217+
afterHookContext = HookContext.<T>builder()
218+
.flagKey(key)
219+
.type(type)
220+
.clientMetadata(this.getMetadata())
221+
.providerMetadata(provider.getMetadata())
222+
.ctx(mergedCtx)
223+
.defaultValue(defaultValue)
224+
.build();
216225

217226
// "short circuit" if the provider is in NOT_READY or FATAL state
218227
if (ProviderState.NOT_READY.equals(state)) {
@@ -225,7 +234,16 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
225234
var providerEval =
226235
(ProviderEvaluation<T>) createProviderEvaluation(type, key, defaultValue, provider, mergedCtx);
227236

228-
details = FlagEvaluationDetails.from(providerEval, key);
237+
details = FlagEvaluationDetails.<T>builder()
238+
.flagKey(key)
239+
.value(providerEval.getValue())
240+
.variant(providerEval.getVariant())
241+
.reason(providerEval.getReason())
242+
.errorMessage(providerEval.getErrorMessage())
243+
.errorCode(providerEval.getErrorCode())
244+
.flagMetadata(Optional.ofNullable(providerEval.getFlagMetadata())
245+
.orElse(ImmutableMetadata.builder().build()))
246+
.build();
229247
if (details.getErrorCode() != null) {
230248
var error =
231249
ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage());

openfeature-sdk/src/test/java/dev/openfeature/sdk/HookContextTest.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,14 @@ class HookContextTest {
2121
void metadata_field_is_type_metadata() {
2222
ClientMetadata clientMetadata = mock(ClientMetadata.class);
2323
Metadata meta = mock(Metadata.class);
24-
HookContext<Object> hc =
25-
HookContext.from("key", FlagValueType.BOOLEAN, clientMetadata, meta, new ImmutableContext(), false);
24+
HookContext<Boolean> hc = HookContext.<Boolean>builder()
25+
.flagKey("key")
26+
.type(FlagValueType.BOOLEAN)
27+
.clientMetadata(clientMetadata)
28+
.providerMetadata(meta)
29+
.ctx(new ImmutableContext())
30+
.defaultValue(false)
31+
.build();
2632

2733
assertTrue(ClientMetadata.class.isAssignableFrom(hc.getClientMetadata().getClass()));
2834
assertTrue(Metadata.class.isAssignableFrom(hc.getProviderMetadata().getClass()));

0 commit comments

Comments
 (0)