Breaking change - adds a name to a DataLoader #193
Conversation
…of factory methods
| @@ -70,331 +70,29 @@ | |||
| @NullMarked | |||
| public class DataLoader<K, V> { | |||
There was a problem hiding this comment.
should DataLoader be final btw?
…of factory methods - added support in DLR for namings DLs
…of factory methods - added support in DLR for namings DLs - added direct register
| private final DataLoaderOptions options; | ||
| private final Object batchLoadFunction; | ||
|
|
||
| /** |
There was a problem hiding this comment.
These have been deprecated for FOREVER and since this is a breaking change chance - I am taking it
| assertState(name != null, () -> "The DataLoader must have a non null name"); | ||
| dataLoaders.put(name, nameAndInstrumentDL(name, instrumentation, dataLoader)); | ||
| return this; | ||
| } |
There was a problem hiding this comment.
New - we can register "named" DL now
| assertState(key.equals(dataLoader.getName()), | ||
| () -> String.format("Data loader name '%s' is not the same as registered key '%s'", dataLoader.getName(), key)); | ||
| return dataLoader; | ||
| } |
There was a problem hiding this comment.
It validates that a DL named "Foo" has been registered as the key "Foo" because dlr.register("Bar", fooDL) makes not sense
| */ | ||
| public <K, V> DataLoader<K, V> registerAndGet(String key, DataLoader<?, ?> dataLoader) { | ||
| dataLoaders.put(key, nameAndInstrumentDL(key, instrumentation, dataLoader)); | ||
| return getDataLoader(key); |
There was a problem hiding this comment.
New - gives back the DL immediately - it may have changed
| * @param dataLoader the data loader to register | ||
| * @return this registry | ||
| */ | ||
| public DataLoaderRegistry register(String key, DataLoader<?, ?> dataLoader) { |
There was a problem hiding this comment.
Since i can give a name to a dataloader and register it with a different name, or register the same DL multiple times with a different name (different options attached), what about providing a pass through cache key into the instrumentation i.e.
DataLoader loader, String dataLoaderKey in the instrumentation rather than attaching an explicit name to the loader itself. This way register is the SoT. I feel if we allow both we might introduce some slight oddities of dataLoaderKey != dataLoaderName else you fail at runtime with Data loader name '%s' is not the same as registered key '%s' If I attempt to do Factory create with name then register with a different name later.
Don't allow creation with a name, only allow association at registry time and pass through to instrumentation since keys can be only a registry thing.
There was a problem hiding this comment.
I gave some thought to this but there is no real link between a DataLoaderRegistry and a DataLoader.
So the instrumentation for a DL can be run without it coming from a DLR
So while you can register them "wrong" - so be it - it validates
4 participants
This adds name to a DataLoader which we should have done from the start in retrospect
This helps Instrumentation so they can report stats in terms of "names".