|
| 1 | +# OpenFeature Java API Improvements |
| 2 | + |
| 3 | +This document outlines improvement opportunities for the OpenFeature Java API based on analysis of the current codebase structure. |
| 4 | + |
| 5 | +## Status |
| 6 | + |
| 7 | +✅ **Completed**: Builder class names have been standardized to use `Builder` instead of class-specific names (e.g., `ImmutableMetadataBuilder` → `Builder`) |
| 8 | + |
| 9 | +## 1. POJO Structure Consistency |
| 10 | + |
| 11 | +### Current Issues |
| 12 | +- **Mixed Mutability Patterns**: `ProviderEvaluation` is mutable with both builder pattern AND public setters, while event details are immutable with builders only |
| 13 | +- **Constructor Overloading vs Builders**: Some classes provide multiple constructors alongside builders |
| 14 | + |
| 15 | +### Improvement Prompts |
| 16 | +``` |
| 17 | +Make ProviderEvaluation immutable by: |
| 18 | +1. Making all fields final |
| 19 | +2. Removing public setters (setValue, setVariant, etc.) |
| 20 | +3. Adding private constructor that takes all parameters |
| 21 | +4. Ensuring builder is the only way to create instances |
| 22 | +
|
| 23 | +Remove constructor overloads from POJOs and standardize on builder pattern only for: |
| 24 | +- FlagEvaluationDetails |
| 25 | +- ProviderEvaluation |
| 26 | +- EventDetails |
| 27 | +- ProviderEventDetails |
| 28 | +``` |
| 29 | + |
| 30 | +## 2. Builder Pattern Standardization |
| 31 | + |
| 32 | +### Current Issues |
| 33 | +- **Inconsistent Method Names**: Some builders use `addX()` (ImmutableMetadata), others use `x()` |
| 34 | +- **Validation Inconsistency**: Some builders validate in `build()`, others in setter methods |
| 35 | + |
| 36 | +### Improvement Prompts |
| 37 | +``` |
| 38 | +Update ImmutableMetadata.Builder to use consistent naming: |
| 39 | +- Change addString() → string() |
| 40 | +- Change addInteger() → integer() |
| 41 | +- Change addLong() → longValue() |
| 42 | +- Change addFloat() → floatValue() |
| 43 | +- Change addDouble() → doubleValue() |
| 44 | +- Change addBoolean() → boolean() |
| 45 | +
|
| 46 | +Move all validation logic to build() methods instead of setter methods for: |
| 47 | +- All builder classes that currently validate in setters |
| 48 | +- Add comprehensive validation in build() with clear error messages |
| 49 | +
|
| 50 | +Ensure all builder methods return 'this' for fluent interface consistency |
| 51 | +``` |
| 52 | + |
| 53 | +## 3. Structure Handling Unification |
| 54 | + |
| 55 | +### Current Issues |
| 56 | +- **Inconsistent Context Constructors**: `ImmutableContext` and `MutableContext` have different constructor patterns |
| 57 | +- **Value Conversion Duplication**: `convertValue()` method exists in multiple places |
| 58 | + |
| 59 | +### Improvement Prompts |
| 60 | +``` |
| 61 | +Create unified context creation patterns: |
| 62 | +1. Add static factory methods: Context.of(), Context.empty(), Context.withTargeting(key) |
| 63 | +2. Standardize constructor patterns between ImmutableContext and MutableContext |
| 64 | +3. Add builder() static methods to both context types for consistency |
| 65 | +
|
| 66 | +Extract value conversion logic: |
| 67 | +1. Create ValueConverter utility class |
| 68 | +2. Move convertValue() from Structure interface to utility |
| 69 | +3. Update all implementations to use centralized conversion logic |
| 70 | +
|
| 71 | +Add convenience methods for common structure operations: |
| 72 | +- Structure.empty() |
| 73 | +- Structure.of(Map<String, Object>) |
| 74 | +- Structure.withAttribute(key, value) |
| 75 | +``` |
| 76 | + |
| 77 | +## 4. Metadata Handling Improvements |
| 78 | + |
| 79 | +### Current Issues |
| 80 | +- **Interface Hierarchy**: `ClientMetadata` has deprecated `getName()` method |
| 81 | +- **Builder Inconsistency**: `ImmutableMetadata` builder uses `addType()` methods vs standard patterns |
| 82 | + |
| 83 | +### Improvement Prompts |
| 84 | +``` |
| 85 | +Clean up metadata interfaces: |
| 86 | +1. Remove deprecated getName() method from ClientMetadata after checking usage |
| 87 | +2. Create clear separation between ClientMetadata and generic Metadata |
| 88 | +3. Consider if both interfaces are needed or can be unified |
| 89 | +
|
| 90 | +Add metadata factory methods: |
| 91 | +- Metadata.empty() |
| 92 | +- Metadata.of(String name) |
| 93 | +- Metadata.builder() shortcuts for common cases |
| 94 | +
|
| 95 | +Improve metadata builder ergonomics: |
| 96 | +- Add putAll(Map<String, Object>) method |
| 97 | +- Add convenience methods for common metadata patterns |
| 98 | +``` |
| 99 | + |
| 100 | +## 5. Event Details Architecture Refinement |
| 101 | + |
| 102 | +### Current Issues |
| 103 | +- **Complex Composition**: `EventDetails` composes `ProviderEventDetails` but both implement same interface |
| 104 | + |
| 105 | +### Improvement Prompts |
| 106 | +``` |
| 107 | +Evaluate event details architecture: |
| 108 | +1. Consider if separate ProviderEventDetails and EventDetails are necessary |
| 109 | +2. Document the relationship and usage patterns clearly |
| 110 | +3. If keeping both, ensure clear distinction in naming and purpose |
| 111 | +
|
| 112 | +Add event details convenience methods: |
| 113 | +- EventDetails.forProviderError(String providerName, ErrorCode code, String message) |
| 114 | +- EventDetails.forProviderReady(String providerName) |
| 115 | +- EventDetails.forConfigurationChange(String providerName, List<String> flagsChanged) |
| 116 | +
|
| 117 | +Improve event details validation: |
| 118 | +- Ensure providerName is always required per OpenFeature spec |
| 119 | +- Add validation in builder.build() methods |
| 120 | +- Provide clear error messages for invalid states |
| 121 | +``` |
| 122 | + |
| 123 | +## 6. API Ergonomics and Developer Experience |
| 124 | + |
| 125 | +### High Priority Improvements |
| 126 | +``` |
| 127 | +Add static import friendly factory methods: |
| 128 | +- Value.of(Object) for common value creation |
| 129 | +- Context.of(String targetingKey) for simple contexts |
| 130 | +- Context.of(String targetingKey, Map<String, Object> attributes) |
| 131 | +
|
| 132 | +Add null safety annotations: |
| 133 | +- @Nullable for optional parameters and return values |
| 134 | +- @NonNull for required parameters |
| 135 | +- Import javax.annotation or create custom annotations |
| 136 | +
|
| 137 | +Create fluent shortcuts for common patterns: |
| 138 | +- EvaluationContext.withTargeting(String key) |
| 139 | +- FlagEvaluationDetails.success(String flagKey, T value) |
| 140 | +- FlagEvaluationDetails.error(String flagKey, T defaultValue, ErrorCode code, String message) |
| 141 | +``` |
| 142 | + |
| 143 | +### Medium Priority Improvements |
| 144 | +``` |
| 145 | +Reduce method overloading where builders can be used: |
| 146 | +- Evaluate if multiple overloaded constructors are needed |
| 147 | +- Prefer builder pattern over method overloads for complex objects |
| 148 | +
|
| 149 | +Improve error messages and validation: |
| 150 | +- Add descriptive error messages in builder validation |
| 151 | +- Include parameter names and expected values in exceptions |
| 152 | +- Add @throws documentation for checked exceptions |
| 153 | +
|
| 154 | +Consider Optional usage for nullable returns: |
| 155 | +- Evaluate using Optional<T> instead of null returns |
| 156 | +- Focus on public API methods that commonly return null |
| 157 | +- Document null-safety contracts clearly |
| 158 | +``` |
| 159 | + |
| 160 | +### Low Priority Improvements |
| 161 | +``` |
| 162 | +Package structure optimization: |
| 163 | +- Consider if all API classes need to be in single package |
| 164 | +- Evaluate creating sub-packages for: contexts, events, metadata, evaluation |
| 165 | +- Maintain backward compatibility during any restructuring |
| 166 | +
|
| 167 | +Documentation improvements: |
| 168 | +- Add usage examples in class-level Javadocs |
| 169 | +- Include common patterns and anti-patterns |
| 170 | +- Add code examples for complex builder usage |
| 171 | +
|
| 172 | +Performance optimizations: |
| 173 | +- Reduce object allocation in hot paths (evaluation) |
| 174 | +- Consider object pooling for frequently created objects |
| 175 | +- Optimize map operations in context merging |
| 176 | +``` |
| 177 | + |
| 178 | +## 7. Checkstyle Fixes Needed |
| 179 | + |
| 180 | +Based on current checkstyle errors: |
| 181 | + |
| 182 | +``` |
| 183 | +Fix checkstyle issues in: |
| 184 | +1. ProviderEventDetails.java:19 - Add empty line before @deprecated tag |
| 185 | +2. FlagEvaluationDetails.java:4 - Remove unused Optional import |
| 186 | +3. EventDetails.java - Add Javadoc comments for missing builder methods: |
| 187 | + - flagsChanged() method |
| 188 | + - message() method |
| 189 | + - eventMetadata() method |
| 190 | + - errorCode() method |
| 191 | +``` |
| 192 | + |
| 193 | +## Implementation Guidelines |
| 194 | + |
| 195 | +### Breaking Changes |
| 196 | +- Document all breaking changes with migration guides |
| 197 | +- Consider deprecation periods for public API changes |
| 198 | +- Provide automated migration tools where possible |
| 199 | + |
| 200 | +### Backward Compatibility |
| 201 | +- Maintain existing public API surface where possible |
| 202 | +- Use @Deprecated annotations with clear migration paths |
| 203 | +- Version new features appropriately |
| 204 | + |
| 205 | +### Testing Strategy |
| 206 | +- Add comprehensive tests for all builder patterns |
| 207 | +- Test null safety and validation thoroughly |
| 208 | +- Include integration tests for common usage patterns |
| 209 | +- Maintain test coverage above 80% for all changes |
| 210 | + |
| 211 | +## Next Steps |
| 212 | + |
| 213 | +1. **Fix Checkstyle Issues** - Address the 6 current checkstyle violations |
| 214 | +2. **Prioritize High-Value Changes** - Start with POJO consistency and builder standardization |
| 215 | +3. **Create Migration Guide** - Document any breaking changes for users |
| 216 | +4. **Update Documentation** - Refresh examples and usage patterns |
| 217 | +5. **Performance Testing** - Ensure changes don't negatively impact performance |
| 218 | + |
| 219 | +This document serves as a roadmap for incrementally improving the API while maintaining stability and backward compatibility. |
0 commit comments