Github user StefanRRichter commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5947#discussion_r185840213
  
    --- Diff: 
flink-end-to-end-tests/flink-datastream-allround-test/src/main/java/org/apache/flink/streaming/tests/artificialstate/eventpayload/ArtificialValueStateBuilder.java
 ---
    @@ -33,21 +33,28 @@
        private static final long serialVersionUID = -1205814329756790916L;
     
        private transient ValueState<STATE> valueState;
    +   private transient boolean afterRestoration;
        private final TypeSerializer<STATE> typeSerializer;
        private final JoinFunction<IN, STATE, STATE> stateValueGenerator;
    +   private final RestoredStateVerifier<STATE> restoredStateVerifier;
     
        public ArtificialValueStateBuilder(
                String stateName,
                JoinFunction<IN, STATE, STATE> stateValueGenerator,
    -           TypeSerializer<STATE> typeSerializer) {
    -
    +           TypeSerializer<STATE> typeSerializer,
    +           RestoredStateVerifier<STATE> restoredStateVerifier) {
                super(stateName);
                this.typeSerializer = typeSerializer;
                this.stateValueGenerator = stateValueGenerator;
    +           this.restoredStateVerifier = restoredStateVerifier;
        }
     
        @Override
        public void artificialStateForElement(IN event) throws Exception {
    +           if (afterRestoration) {
    --- End diff --
    
    I find this way of checking the state rather invasive not completely 
thorough. There is now a pretty tight coupling between creating artificial 
state and checking something about it on restore. In particular, there is a 
hardcoded way now when to check. This makes it harder to reuse the classes in 
further test jobs that we might want to build with them. Can't we use a way 
that is more based on composition? For example, wrap the state builder in a 
state checker? This is also only doing just one check, so if the input element 
has a key that we never encountered, the state is `null` and there might be no 
check.


---

Reply via email to