Persistence extensions: allow null serviceId #4213
Conversation
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
| @@ -80,7 +80,7 @@ public PersistenceExtensions(@Reference PersistenceServiceRegistry registry, | |||
| * @param item the item to store | |||
| */ | |||
| public static void persist(Item item) { | |||
| internalPersist(item); | |||
| internalPersist(item, (String) null); | |||
There was a problem hiding this comment.
Why do you need a cast to String here?
There was a problem hiding this comment.
Was needed when I only half changed the file (version with signature internalPersist(Item, TimeSeries)), but no longer needed, so removed.
| String serviceId = getDefaultServiceId(); | ||
| if (serviceId == null) { | ||
| private static void internalPersist(Item item, @Nullable String serviceId) { | ||
| String sId = serviceId == null ? getDefaultServiceId() : serviceId; |
There was a problem hiding this comment.
| String sId = serviceId == null ? getDefaultServiceId() : serviceId; | |
| String effectiveServiceId = Objects.requireNonNullElse(serviceId, getDefaultServiceId()); |
Also below
|
Perhaps label this as regression? |
|
Also to ensure that this is honored in the future, could we please add some tests to check? |
|
I don't think it's a regression. It didn't work before, right? |
It worked like this before #3736: #3736 removed this and instead, it actually looks up |
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
True, it would, but relying on it was dangerous. For some actions it did not work, and for others it gave wrong result as it was never meant to be used like that. For some, you got lucky and it actually did end up giving the right result. |
See #3736 (comment)