Refactor index workflow to prepare for the tree view

filters/build.gradle.kts

@@ -24,6 +24,10 @@ dependencies {
     api(project(":logmodel")) {
         because("Exposes LogRecord as a generic argument")
     }
+    api(project(":base")) {
+        because("Exposes Observable in API")
+    }
 
-    implementation(project(":base"))
+    testFixturesApi(platform(libs.test.assertj.bom))
+    testFixturesApi(libs.test.assertj.core)
 }

filters/src/main/java/name/mlopatkin/andlogview/filters/FilterChain.java

@@ -16,6 +16,8 @@
 package name.mlopatkin.andlogview.filters;
 
 import name.mlopatkin.andlogview.logmodel.LogRecord;
+import name.mlopatkin.andlogview.utils.events.Observable;
+import name.mlopatkin.andlogview.utils.events.Subject;
 
 import com.google.common.collect.MultimapBuilder;
 import com.google.common.collect.SetMultimap;
@@ -30,8 +32,20 @@
  * The order in which filters are added to/removed from FilterChain doesn't matter.
  */
 public class FilterChain implements FilterCollection<Filter> {
+    /**
+     * An observer to be notified when the set of filters in this chain changes.
+     */
+    @FunctionalInterface
+    public interface Observer {
+        /**
+         * Called when the set of filters in the {@link FilterChain} changes
+         */
+        void onFiltersChanged();
+    }
+
     private final SetMultimap<FilteringMode, Filter> filters =
             MultimapBuilder.enumKeys(FilteringMode.class).hashSetValues().build();
+    private final Subject<Observer> observers = new Subject<>();
 
     private boolean include(FilteringMode mode, LogRecord record) {
         var filtersForMode = filters.get(mode);
@@ -59,6 +73,7 @@ private boolean include(FilteringMode mode, LogRecord record) {
     public void addFilter(Filter filter) {
         if (filter.isEnabled()) {
             filters.put(filter.getMode(), filter);
+            notifyObservers();
         }
     }
 
@@ -68,7 +83,18 @@ public boolean shouldShow(LogRecord record) {
 
     @Override
     public void removeFilter(Filter filter) {
-        filters.remove(filter.getMode(), filter);
+        if (filters.remove(filter.getMode(), filter)) {
+            notifyObservers();
+        }
     }
 
+    private void notifyObservers() {
+        for (Observer observer : observers) {
+            observer.onFiltersChanged();
+        }
+    }
+
+    public Observable<Observer> asObservable() {
+        return observers.asObservable();
+    }
 }

filters/src/main/java/name/mlopatkin/andlogview/filters/FilterCollection.java

@@ -16,6 +16,8 @@
 
 package name.mlopatkin.andlogview.filters;
 
+import name.mlopatkin.andlogview.utils.events.ScopedObserver;
+
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
@@ -74,7 +76,7 @@ default void replaceFilter(T oldFilter, T newFilter) {
      * @param model the model to connect this collection to
      * @return the observer that is subscribed to the provided model. It can be used to unsubscribe.
      */
-    default FilterModel.Observer setModel(FilterModel model) {
+    default ScopedObserver setModel(FilterModel model) {
         for (var filter : model.getFilters()) {
             var transformed = transformFilter(filter);
             if (transformed != null) {
@@ -99,7 +101,6 @@ protected void onMyFilterReplaced(FilterModel model, T oldFilter, T newFilter) {
             }
         };
 
-        model.asObservable().addObserver(observer);
-        return observer;
+        return model.asObservable().addScopedObserver(observer);
     }
 }

filters/src/main/java/name/mlopatkin/andlogview/filters/FilterModel.java

@@ -101,4 +101,13 @@ interface Observer {
     static FilterModel create() {
         return new FilterModelImpl();
     }
+
+    /**
+     * Creates a new FilterModel with provided filters as its content.
+     *
+     * @return the new FilterModel
+     */
+    static FilterModel create(Collection<? extends Filter> filters) {
+        return new FilterModelImpl(filters);
+    }
 }

filters/src/main/java/name/mlopatkin/andlogview/filters/FilterModelImpl.java

@@ -19,25 +19,31 @@
 import name.mlopatkin.andlogview.utils.events.Observable;
 import name.mlopatkin.andlogview.utils.events.Subject;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Objects;
 
 class FilterModelImpl implements FilterModel {
-    private final Subject<Observer> observers = new Subject<>();
-    private final Map<Filter, Integer> filterPositions = new HashMap<>();
+    @VisibleForTesting
+    protected final Subject<Observer> observers = new Subject<>();
+
     private final List<Filter> filters = new ArrayList<>();
 
+    public FilterModelImpl() {}
+
+    public FilterModelImpl(Collection<? extends Filter> filters) {
+        this.filters.addAll(ImmutableSet.copyOf(filters));
+    }
+
     @Override
     public void addFilter(Filter filter) {
-        var prevPosition = filterPositions.putIfAbsent(filter, filters.size());
-        if (prevPosition == null) {
+        if (!filters.contains(filter)) {
             filters.add(filter);
             for (var observer : observers) {
                 observer.onFilterAdded(this, filter);
@@ -47,9 +53,7 @@ public void addFilter(Filter filter) {
 
     @Override
     public void removeFilter(Filter filter) {
-        var position = filterPositions.remove(filter);
-        if (position != null) {
-            filters.remove(position.intValue());
+        if (filters.remove(filter)) {
             for (var observer : observers) {
                 observer.onFilterRemoved(this, filter);
             }
@@ -58,17 +62,16 @@ public void removeFilter(Filter filter) {
 
     @Override
     public void replaceFilter(Filter toReplace, Filter newFilter) {
-        Preconditions.checkArgument(filterPositions.containsKey(toReplace),
+        var position = filters.indexOf(toReplace);
+        Preconditions.checkArgument(position >= 0,
                 String.format("Filter %s is not in the model", toReplace));
         if (Objects.equals(toReplace, newFilter)) {
             // Replacing the filter with itself, do nothing.
             return;
         }
-        var position = filterPositions.get(toReplace);
-        if (filterPositions.putIfAbsent(newFilter, position) != null) {
+        if (filters.contains(newFilter)) {
             throw new IllegalArgumentException(String.format("Filter %s is already in the model", newFilter));
         }
-        filterPositions.remove(toReplace);
         filters.set(position, newFilter);
 
         for (var observer : observers) {

filters/src/test/java/name/mlopatkin/andlogview/filters/FilterModelTest.java

@@ -16,22 +16,23 @@
 
 package name.mlopatkin.andlogview.filters;
 
+import static name.mlopatkin.andlogview.filters.FilterModelAssert.assertThatFilters;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
 
-import org.assertj.core.api.AbstractCollectionAssert;
+import com.google.common.collect.ImmutableList;
+
 import org.assertj.core.api.ListAssert;
-import org.assertj.core.api.ObjectAssert;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Spy;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 
 @ExtendWith(MockitoExtension.class)
@@ -66,11 +67,6 @@ private ListAssert<Filter> assertThatObserverSawFilters() {
         return assertThat(observedModel);
     }
 
-    private AbstractCollectionAssert<?, Collection<? extends Filter>, Filter, ObjectAssert<Filter>> assertThatFilters(
-            FilterModel model) {
-        return assertThat(model.getFilters());
-    }
-
     @Test
     void addingFilterNotifiesObservers() {
         var model = createModel();
@@ -195,6 +191,21 @@ void replacementFilterKeepsPositionOfReplaced() {
         assertThatFilters(model).containsExactly(filter1, filter2Replacement, filter3);
     }
 
+    @Test
+    void canReplaceFilterAfterRemovingFilterInFront() {
+        var model = createModel();
+        var filter1 = createFilter("filter1");
+        var filter2 = createFilter("filter2");
+        model.addFilter(filter1);
+        model.addFilter(filter2);
+
+        var replacement = createFilter("replacement");
+        model.removeFilter(filter1);
+        model.replaceFilter(filter2, replacement);
+
+        assertThatFilters(model).containsExactly(replacement);
+    }
+
     @Test
     void removingAndAddingFilterMovesItToBack() {
         var model = createModel();
@@ -209,6 +220,16 @@ void removingAndAddingFilterMovesItToBack() {
         assertThatFilters(model).containsExactly(filter2, filter1);
     }
 
+    @Test
+    void canAddMultipleFiltersAtOnce() {
+        var filter1 = createFilter("filter1");
+        var filter2 = createFilter("filter2");
+
+        var model = new FilterModelImpl(ImmutableList.of(filter1, filter2, filter1));
+
+        assertThatFilters(model).containsExactly(filter1, filter2);
+    }
+
     FilterModel createModel() {
         return new FilterModelImpl();
     }

filters/src/testFixtures/java/name/mlopatkin/andlogview/filters/FilterModelAssert.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright 2024 the Andlogview authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package name.mlopatkin.andlogview.filters;
+
+import org.assertj.core.api.AbstractAssert;
+import org.assertj.core.api.AbstractCollectionAssert;
+import org.assertj.core.api.Assertions;
+import org.assertj.core.api.ObjectAssert;
+import org.assertj.core.util.CheckReturnValue;
+
+import java.util.Collection;
+
+public class FilterModelAssert extends AbstractAssert<FilterModelAssert, TestFilterModel> {
+    protected FilterModelAssert(TestFilterModel testFilterModel) {
+        super(testFilterModel, FilterModelAssert.class);
+    }
+
+    public FilterModelAssert hasNoObservers() {
+        if (actual.hasObservers()) {
+            throw failure("still has observers");
+        }
+        return this;
+    }
+
+    @CheckReturnValue
+    public static FilterModelAssert assertThat(TestFilterModel filterModel) {
+        return new FilterModelAssert(filterModel);
+    }
+
+    @CheckReturnValue
+    public static AbstractCollectionAssert<
+            ?, Collection<? extends Filter>, Filter, ObjectAssert<Filter>
+            > assertThatFilters(FilterModel model) {
+        return Assertions.assertThat(model.getFilters());
+    }
+}

filters/src/testFixtures/java/name/mlopatkin/andlogview/filters/TestFilterModel.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2024 the Andlogview authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package name.mlopatkin.andlogview.filters;
+
+/**
+ * Test implementation of {@link FilterModel} to verify that all observers are unsubscribed.
+ */
+public class TestFilterModel extends FilterModelImpl {
+    public boolean hasObservers() {
+        return !observers.isEmpty();
+    }
+}

src/name/mlopatkin/andlogview/ui/bookmarks/BookmarkController.java

@@ -19,6 +19,8 @@
 import name.mlopatkin.andlogview.ui.indexframe.AbstractIndexController;
 import name.mlopatkin.andlogview.ui.indexframe.IndexController;
 import name.mlopatkin.andlogview.ui.indexframe.IndexFrame;
+import name.mlopatkin.andlogview.ui.logtable.LogRecordTableModel;
+import name.mlopatkin.andlogview.ui.mainframe.DialogFactory;
 import name.mlopatkin.andlogview.ui.mainframe.MainFrameDependencies;
 import name.mlopatkin.andlogview.ui.mainframe.MainFrameScoped;
 import name.mlopatkin.andlogview.widgets.UiHelper;
@@ -36,8 +38,12 @@ public class BookmarkController extends AbstractIndexController implements Index
     private final IndexFrame indexFrame;
 
     @Inject
-    public BookmarkController(MainFrameDependencies mainFrameDependencies, BookmarkModel bookmarksModel,
-            BookmarksLogModelFilter logModelFilter, @Named(MainFrameDependencies.FOR_MAIN_FRAME) JTable mainLogTable) {
+    public BookmarkController(
+            BookmarkModel bookmarksModel,
+            LogRecordTableModel logModel,
+            BookmarksLogModelFilter logModelFilter,
+            @Named(MainFrameDependencies.FOR_MAIN_FRAME) JTable mainLogTable,
+            DialogFactory dialogFactory) {
         super(mainLogTable);
         this.mainLogTable = mainLogTable;
 
@@ -57,12 +63,13 @@ public void onBookmarkRemoved() {
         };
         bookmarksModel.asObservable().addObserver(bookmarkChangeObserver);
 
-        BookmarksDi.BookmarksFrameComponent.Builder builder = DaggerBookmarksDi_BookmarksFrameComponent.builder();
-        builder.mainFrameDependencies(mainFrameDependencies);
-        builder.setIndexController(this);
-        builder.setIndexFilter(logModelFilter);
-
-        BookmarksDi.BookmarksFrameComponent indexFrameComponent = builder.build();
+        var builder = DaggerBookmarksDi_BookmarksFrameComponent.builder()
+                .bookmarkModel(bookmarksModel)
+                .logRecordTableModel(logModel)
+                .dialogFactory(dialogFactory)
+                .setIndexController(this)
+                .setIndexFilter(logModelFilter);
+        var indexFrameComponent = (BookmarksDi.BookmarksFrameComponent) builder.build();
 
         indexFrame = indexFrameComponent.createFrame();
         indexFrame.setTitle("Bookmarks");