feat: STREAMP-12186: adding disable flags for entity status support #358
Conversation
| @@ -1,5 +1,5 @@ | |||
| /** | |||
There was a problem hiding this comment.
All entity service classes are updated to use separated saveSpecification and saveStatus repository methods.
| return consumerBindingService.updateStatus(consumerBinding, status.asStatus()).get(); | ||
| } else { | ||
| return consumerBinding; | ||
| } |
There was a problem hiding this comment.
All of the individual entity mutation implementation classes have this new Spring value toggle, set to true by default which is the current mode of operation.
When set to false an updateStatus mutation will NOT update the entity status via the entity service, it will just be a successful NOOP.
| public static StatusInput statusInput() { | ||
| return StatusInput.builder().agentStatus(mapper.createObjectNode()).build(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Just a convenience test class to generate a statusInput
| verify(consumerBindingService, never()).updateStatus(consumerBinding.get(), statusInput.asStatus()); | ||
| assertEquals(consumerBinding.get(), result); | ||
| } | ||
|
|
||
| private ConsumerBindingKeyInput getConsumerBindingInputKey() { |
There was a problem hiding this comment.
Each entity has these two additional tests to test that when entity status is enabled (default) the entity service is called to update the status. The second test is for when entity status is disabled, the entity service should NOT be called when an updateStatus mutation is received.
| this.key = key; | ||
| this.specification = specification; | ||
| this.status = new Status(); | ||
| } | ||
| } |
There was a problem hiding this comment.
For all Entity Model classes, adding this additional convenience constructor which allows for creating Entities without needed to specify a status value.
| T saveSpecification(T entity); | ||
|
|
||
| T saveStatus(T entity); | ||
|
|
||
| Optional<T> findById(ID id); | ||
|
|
||
| List<T> findAll(); |
There was a problem hiding this comment.
Finally handling a TODO from long ago, deprecating the save method in favor of two separate saveSpecification and saveStatus methods.
| this.specification = specification; | ||
| this.status = new DefaultStatus(); | ||
| } | ||
|
|
||
| public interface Key<S extends Specification> {} |
There was a problem hiding this comment.
New convenience constructor so that a status does not need to be passed in.
| public DefaultEntityView(EventReceiver receiver) { | ||
| this(receiver, new ConcurrentHashMap<>()); | ||
| } | ||
|
|
||
|
|
||
| @Override | ||
| public CompletableFuture<Void> load(@NonNull EntityViewListener listener) { | ||
| val future = new CompletableFuture<Void>(); |
There was a problem hiding this comment.
Adding an optional entityStatusEnabled flag to all constructor patterns will can be used to disable entity status persistence.
There was a problem hiding this comment.
Couple of questions:
Rather than having multiple instances of the entityStatusEnabled in all instances of the *MutationImpl classes, would it be simpler to just ignore these in the KafkaEventSender? enableStatuses could be added to Config there, the sending could then be changed to something like:
@Override
public <K extends Entity.Key<S>, S extends Specification> CompletableFuture<Void> send(@NonNull Event<K, S> event) {
if (!config.enableStatuses && (event instanceof StatusEvent<K, S> || event instanceof StatusDeletionEvent<K, S>)) {
return CompletableFuture.completedFuture(null);
}
val avroEvent = converter.toAvro(event);
return send(avroEvent.getKey(), avroEvent.getValue());
}We'd then remove the if/else in all *MutationImpl.
Second question, and maybe this is progress for a follow up. Would it be possible to change the Event class hierarchy in allow easier differentiation between entity/status events.
I'm thinking two new interfaces with SpecificationEvent/StatusEvent/SpecificationDeletionEvent/StatusDeletionEvent extending the correct interface.
public interface EntityEvent<K extends Entity.Key<S>, S extends Specification> extends Event<K, S> { }
public interface StatusEvent<K extends Entity.Key<S>, S extends Specification> extends Event<K, S> { }We could then have a new EventReceiverListener which deals only with the entity events (meaning agents really are shielded from Status events...
<K extends Entity.Key<S>, S extends Specification> void onEvent(EntityEvent<K, S> event);
.../java/com/expediagroup/streamplatform/streamregistry/repository/kafka/DefaultRepository.java
Outdated
Show resolved
Hide resolved
.../java/com/expediagroup/streamplatform/streamregistry/repository/kafka/DefaultRepository.java
Outdated
Show resolved
Hide resolved
|
@jamespfaulkner - OK summary of updates:
|
| import lombok.NonNull; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.Value; | ||
| import lombok.*; |
Adding flags to disable Entity Status.