Fixed issue datacleaner#566 with "link to" not working due to VertexC…

…ontext cast. Also covered a bunch more scenarios that had to do with resolving the wrong AnalysisJobBuilder at the time of editing properties, building links etc.
robinroby · Aug 21, 2015 · e5dee4f · e5dee4f
1 parent d834396
commit e5dee4f
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 64 deletions.
diff --git a/api/src/main/java/org/datacleaner/job/output/OutputDataStreamBuilderImpl.java b/api/src/main/java/org/datacleaner/job/output/OutputDataStreamBuilderImpl.java
@@ -24,7 +24,6 @@
 import org.apache.metamodel.schema.MutableColumn;
 import org.apache.metamodel.schema.MutableSchema;
 import org.apache.metamodel.schema.MutableTable;
-import org.apache.metamodel.schema.Schema;
 import org.apache.metamodel.schema.Table;
 import org.datacleaner.api.OutputDataStream;
 

diff --git a/...main/java/org/datacleaner/widgets/properties/SingleInputColumnComboBoxPropertyWidget.java b/...main/java/org/datacleaner/widgets/properties/SingleInputColumnComboBoxPropertyWidget.java
@@ -30,7 +30,6 @@
 import org.datacleaner.api.InputColumn;
 import org.datacleaner.data.MutableInputColumn;
 import org.datacleaner.descriptors.ConfiguredPropertyDescriptor;
-import org.datacleaner.job.builder.AnalysisJobBuilder;
 import org.datacleaner.job.builder.ComponentBuilder;
 import org.datacleaner.job.builder.SourceColumnChangeListener;
 import org.datacleaner.job.builder.TransformerChangeListener;
@@ -50,12 +49,11 @@ public class SingleInputColumnComboBoxPropertyWidget extends AbstractPropertyWid
         SourceColumnChangeListener, TransformerChangeListener, MutableInputColumn.Listener {
 
     private final DCComboBox<InputColumn<?>> _comboBox;
-    private final AnalysisJobBuilder _analysisJobBuilder;
     private final Class<?> _dataType;
     private volatile List<InputColumn<?>> _inputColumns;
 
     @Inject
-    public SingleInputColumnComboBoxPropertyWidget(AnalysisJobBuilder analysisJobBuilder,
+    public SingleInputColumnComboBoxPropertyWidget(
             ComponentBuilder componentBuilder, ConfiguredPropertyDescriptor propertyDescriptor) {
         super(componentBuilder, propertyDescriptor);
         _comboBox = new DCComboBox<InputColumn<?>>();
@@ -66,9 +64,8 @@ public void onItemSelected(InputColumn<?> item) {
                 fireValueChanged();
             }
         });
-        _analysisJobBuilder = analysisJobBuilder;
-        _analysisJobBuilder.getSourceColumnListeners().add(this);
-        _analysisJobBuilder.getTransformerChangeListeners().add(this);
+        getAnalysisJobBuilder().getSourceColumnListeners().add(this);
+        getAnalysisJobBuilder().getTransformerChangeListeners().add(this);
         _dataType = propertyDescriptor.getTypeArgument(0);
 
         updateComponents();
@@ -82,7 +79,7 @@ private void updateComponents() {
     }
 
     private void updateComponents(InputColumn<?> currentValue) {
-        _inputColumns = _analysisJobBuilder.getAvailableInputColumns(getComponentBuilder(), _dataType);
+        _inputColumns = getAnalysisJobBuilder().getAvailableInputColumns(getComponentBuilder(), _dataType);
 
         if (currentValue != null) {
             if (!_inputColumns.contains(currentValue)) {
@@ -170,8 +167,8 @@ public void onRemove(TransformerComponentBuilder<?> transformerJobBuilder) {
     @Override
     public void onPanelRemove() {
         super.onPanelRemove();
-        _analysisJobBuilder.getSourceColumnListeners().remove(this);
-        _analysisJobBuilder.getTransformerChangeListeners().remove(this);
+        getAnalysisJobBuilder().getSourceColumnListeners().remove(this);
+        getAnalysisJobBuilder().getTransformerChangeListeners().remove(this);
 
         for (InputColumn<?> column : _inputColumns) {
             if (column instanceof MutableInputColumn) {

diff --git a/...n/java/org/datacleaner/widgets/properties/SingleInputColumnRadioButtonPropertyWidget.java b/...n/java/org/datacleaner/widgets/properties/SingleInputColumnRadioButtonPropertyWidget.java
@@ -30,21 +30,20 @@
 import javax.swing.JRadioButton;
 import javax.swing.event.DocumentEvent;
 
+import org.datacleaner.actions.AddExpressionBasedColumnActionListener;
+import org.datacleaner.api.InputColumn;
 import org.datacleaner.data.MutableInputColumn;
 import org.datacleaner.descriptors.ConfiguredPropertyDescriptor;
 import org.datacleaner.job.builder.ComponentBuilder;
-import org.datacleaner.job.builder.AnalysisJobBuilder;
 import org.datacleaner.job.builder.SourceColumnChangeListener;
 import org.datacleaner.job.builder.TransformerChangeListener;
 import org.datacleaner.job.builder.TransformerComponentBuilder;
-import org.datacleaner.util.LabelUtils;
-import org.datacleaner.util.ReflectionUtils;
-import org.datacleaner.util.StringUtils;
-import org.datacleaner.actions.AddExpressionBasedColumnActionListener;
-import org.datacleaner.api.InputColumn;
 import org.datacleaner.panels.DCPanel;
 import org.datacleaner.util.DCDocumentListener;
 import org.datacleaner.util.IconUtils;
+import org.datacleaner.util.LabelUtils;
+import org.datacleaner.util.ReflectionUtils;
+import org.datacleaner.util.StringUtils;
 import org.datacleaner.util.WidgetFactory;
 import org.datacleaner.util.WidgetUtils;
 import org.jdesktop.swingx.JXRadioGroup;
@@ -59,7 +58,6 @@ public class SingleInputColumnRadioButtonPropertyWidget extends AbstractProperty
         SourceColumnChangeListener, TransformerChangeListener, MutableInputColumn.Listener {
 
     private final JXRadioGroup<JRadioButton> _radioGroup = new JXRadioGroup<JRadioButton>();
-    private final AnalysisJobBuilder _analysisJobBuilder;
     private final Class<?> _dataType;
     private final ConfiguredPropertyDescriptor _propertyDescriptor;
     private final DCPanel _buttonPanel;
@@ -68,15 +66,14 @@ public class SingleInputColumnRadioButtonPropertyWidget extends AbstractProperty
     private final JXTextField _searchDatastoreTextField;
 
     @Inject
-    public SingleInputColumnRadioButtonPropertyWidget(AnalysisJobBuilder analysisJobBuilder,
-            ComponentBuilder componentBuilder, ConfiguredPropertyDescriptor propertyDescriptor) {
+    public SingleInputColumnRadioButtonPropertyWidget(ComponentBuilder componentBuilder,
+            ConfiguredPropertyDescriptor propertyDescriptor) {
         super(componentBuilder, propertyDescriptor);
         _radioGroup.setLayoutAxis(BoxLayout.Y_AXIS);
         _radioGroup.setOpaque(false);
 
-        _analysisJobBuilder = analysisJobBuilder;
-        _analysisJobBuilder.getSourceColumnListeners().add(this);
-        _analysisJobBuilder.getTransformerChangeListeners().add(this);
+        getAnalysisJobBuilder().getSourceColumnListeners().add(this);
+        getAnalysisJobBuilder().getTransformerChangeListeners().add(this);
         _propertyDescriptor = propertyDescriptor;
         _dataType = propertyDescriptor.getTypeArgument(0);
 
@@ -106,8 +103,7 @@ protected void onChange(DocumentEvent event) {
         _buttonPanel.setLayout(new VerticalLayout(2));
 
         if (_dataType == String.class || _dataType == Object.class) {
-            final JButton expressionColumnButton = WidgetFactory
-                    .createSmallButton(IconUtils.MODEL_COLUMN_EXPRESSION);
+            final JButton expressionColumnButton = WidgetFactory.createSmallButton(IconUtils.MODEL_COLUMN_EXPRESSION);
             expressionColumnButton.setToolTipText("Create expression/value based column");
             expressionColumnButton.addActionListener(AddExpressionBasedColumnActionListener.forSingleColumn(this));
             expressionColumnButton.setFocusable(false);
@@ -130,7 +126,7 @@ private void updateComponents() {
     }
 
     private void updateComponents(InputColumn<?> value) {
-        _inputColumns = _analysisJobBuilder.getAvailableInputColumns(getComponentBuilder(), _dataType);
+        _inputColumns = getAnalysisJobBuilder().getAvailableInputColumns(getComponentBuilder(), _dataType);
 
         if (value != null) {
             if (!_inputColumns.contains(value)) {
@@ -225,7 +221,8 @@ public void onRemove(InputColumn<?> sourceColumn) {
     private void handleRemovedColumn(InputColumn<?> column) {
         if (isColumnApplicable(column)) {
             final ComponentBuilder componentBuilder = getComponentBuilder();
-            final InputColumn<?> currentValue = (InputColumn<?>) componentBuilder.getConfiguredProperty(_propertyDescriptor);
+            final InputColumn<?> currentValue = (InputColumn<?>) componentBuilder
+                    .getConfiguredProperty(_propertyDescriptor);
             if (currentValue != null) {
                 if (currentValue.equals(column)) {
                     componentBuilder.setConfiguredProperty(_propertyDescriptor, null);
@@ -262,8 +259,8 @@ public void onRemove(TransformerComponentBuilder<?> transformerJobBuilder) {
     @Override
     public void onPanelRemove() {
         super.onPanelRemove();
-        _analysisJobBuilder.getSourceColumnListeners().remove(this);
-        _analysisJobBuilder.getTransformerChangeListeners().remove(this);
+        getAnalysisJobBuilder().getSourceColumnListeners().remove(this);
+        getAnalysisJobBuilder().getTransformerChangeListeners().remove(this);
 
         for (InputColumn<?> column : _inputColumns) {
             if (column instanceof MutableInputColumn) {

diff --git a/desktop/ui/src/main/java/org/datacleaner/widgets/visualization/JobGraphLinkPainter.java b/desktop/ui/src/main/java/org/datacleaner/widgets/visualization/JobGraphLinkPainter.java
@@ -65,7 +65,7 @@ public static class VertexContext {
         final private OutputDataStream _outputDataStream;
         final private AnalysisJobBuilder _analysisJobBuilder;
 
-        VertexContext(Object vertex, AnalysisJobBuilder analysisJobBuilder, OutputDataStream outputDataStream){
+        public VertexContext(Object vertex, AnalysisJobBuilder analysisJobBuilder, OutputDataStream outputDataStream) {
             _vertex = vertex;
             _outputDataStream = outputDataStream;
             _analysisJobBuilder = analysisJobBuilder;
@@ -103,7 +103,6 @@ public JobGraphLinkPainter(JobGraphContext graphContext, JobGraphActions actions
         _arrowPaintable = new ArrowPaintable();
     }
 
-
     /**
      * Called when the drawing of a new link/edge is started
      * 
@@ -197,16 +196,12 @@ private boolean createLink(final VertexContext fromVertex, final Object toVertex
 
         final AnalysisJobBuilder sourceAnalysisJobBuilder = fromVertex.getAnalysisJobBuilder();
 
-        if (fromVertex.getVertex() instanceof Table || fromVertex.getOutputDataStream() != null) {
-            final Table table;
-
-            if(fromVertex.getOutputDataStream() != null){
-                sourceColumns = sourceAnalysisJobBuilder.getSourceColumns();
-            } else {
-                table = (Table) fromVertex;
-                sourceColumns = sourceAnalysisJobBuilder.getSourceColumnsOfTable(table);
-            }
-
+        if (fromVertex.getOutputDataStream() != null) {
+            sourceColumns = sourceAnalysisJobBuilder.getSourceColumns();
+            filterOutcomes = null;
+        } else if (fromVertex.getVertex() instanceof Table) {
+            final Table table = (Table) fromVertex.getVertex();
+            sourceColumns = sourceAnalysisJobBuilder.getSourceColumnsOfTable(table);
             filterOutcomes = null;
         } else if (fromVertex.getVertex() instanceof InputColumnSourceJob) {
             InputColumn<?>[] outputColumns;
@@ -231,11 +226,7 @@ private boolean createLink(final VertexContext fromVertex, final Object toVertex
             if (sourceColumns != null && !sourceColumns.isEmpty()) {
 
                 try {
-                    if (sourceAnalysisJobBuilder != componentBuilder.getAnalysisJobBuilder()){
-                        if(componentBuilder.getInput().length > 0) {
-                            logger.debug("createLink(...) returning false - cannot change analysisJobBuilder for a component with configured inputColumns");
-                            return false;
-                        }
+                    if (sourceAnalysisJobBuilder != componentBuilder.getAnalysisJobBuilder()) {
                         sourceAnalysisJobBuilder.moveComponent(componentBuilder);
                     }
 

diff --git a/...i/src/main/java/org/datacleaner/widgets/visualization/JobGraphLinkPainterMousePlugin.java b/...i/src/main/java/org/datacleaner/widgets/visualization/JobGraphLinkPainterMousePlugin.java
@@ -80,7 +80,7 @@ public void mousePressed(MouseEvent me) {
             if (vertex != null) {
                 me.consume();
                 _linkPainter.startLink(new JobGraphLinkPainter.VertexContext(vertex,
-                        _graphContext.getMainAnalysisJobBuilder(), null));
+                        _graphContext.getAnalysisJobBuilder(vertex), null));
             }
         }
     }

diff --git a/desktop/ui/src/main/java/org/datacleaner/widgets/visualization/JobGraphMouseListener.java b/desktop/ui/src/main/java/org/datacleaner/widgets/visualization/JobGraphMouseListener.java
@@ -195,7 +195,7 @@ protected void onNameChanged() {
 
     private JMenuItem createLinkMenuItem(final Object from) {
         return createLinkMenuItem(new JobGraphLinkPainter.VertexContext(from,
-                _graphContext.getMainAnalysisJobBuilder(), null));
+                _graphContext.getAnalysisJobBuilder(from), null));
     }
 
     private JMenuItem createLinkMenuItem(final JobGraphLinkPainter.VertexContext from) {

diff --git a/desktop/ui/src/main/java/org/datacleaner/widgets/visualization/JobGraphNodeBuilder.java b/desktop/ui/src/main/java/org/datacleaner/widgets/visualization/JobGraphNodeBuilder.java
@@ -29,15 +29,12 @@
 import org.apache.metamodel.schema.Table;
 import org.datacleaner.api.InputColumn;
 import org.datacleaner.api.OutputDataStream;
-import org.datacleaner.job.AnalysisJob;
 import org.datacleaner.job.ComponentRequirement;
 import org.datacleaner.job.FilterOutcome;
 import org.datacleaner.job.HasComponentRequirement;
 import org.datacleaner.job.HasFilterOutcomes;
 import org.datacleaner.job.InputColumnSinkJob;
 import org.datacleaner.job.InputColumnSourceJob;
-import org.datacleaner.job.OutputDataStreamJob;
-import org.datacleaner.job.OutputDataStreamJobSource;
 import org.datacleaner.job.builder.AnalysisJobBuilder;
 import org.datacleaner.job.builder.AnalyzerComponentBuilder;
 import org.datacleaner.job.builder.ComponentBuilder;
@@ -70,15 +67,20 @@ public DirectedGraph<Object, JobGraphLink> buildGraph() {
         final DirectedGraph<Object, JobGraphLink> graph = new DirectedSparseGraph<Object, JobGraphLink>();
         final List<Table> sourceTables = _analysisJobBuilder.getSourceTables();
 
-        final SourceColumnFinder sourceColumnFinder = new SourceColumnFinder();
-        sourceColumnFinder.addSources(_analysisJobBuilder);
-
-        buildGraphInternal(graph, _analysisJobBuilder, sourceColumnFinder, sourceTables);
+        buildGraphInternal(graph, _analysisJobBuilder, sourceTables);
         return graph;
     }
 
     private void buildGraphInternal(final DirectedGraph<Object, JobGraphLink> graph,
-            final AnalysisJobBuilder analysisJobBuilder, SourceColumnFinder sourceColumnFinder, List<Table> sourceTables) {
+            final AnalysisJobBuilder analysisJobBuilder, List<Table> sourceTables) {
+
+        // note: currently SourceColumnFinder cannot cross links from
+        // OutputDataStreams to the main/parent AnalysisJobBuilder, so we create
+        // a new SourceColumnFinder for each AnalysisJobBuilder instead of
+        // reusing the instance.
+        final SourceColumnFinder sourceColumnFinder = new SourceColumnFinder();
+        sourceColumnFinder.addSources(analysisJobBuilder);
+
         for (Table table : sourceTables) {
             addNodes(graph, sourceColumnFinder, table, -1);
         }
@@ -244,7 +246,6 @@ private void addNodes(DirectedGraph<Object, JobGraphLink> graph, SourceColumnFin
             // decrement recurseCount
             recurseCount--;
 
-
             if (item instanceof InputColumnSinkJob) {
                 InputColumn<?>[] inputColumns = ((InputColumnSinkJob) item).getInput();
                 for (InputColumn<?> inputColumn : inputColumns) {
@@ -310,11 +311,12 @@ private void addNodes(DirectedGraph<Object, JobGraphLink> graph, SourceColumnFin
             if (item instanceof ComponentBuilder) {
                 ComponentBuilder componentBuilder = (ComponentBuilder) item;
 
-                for(OutputDataStream outputDataStream : componentBuilder.getOutputDataStreams()){
-                    if(componentBuilder.isOutputDataStreamConsumed(outputDataStream)) {
-                        final AnalysisJobBuilder outputDataStreamJobBuilder = componentBuilder.getOutputDataStreamJobBuilder(outputDataStream);
+                for (OutputDataStream outputDataStream : componentBuilder.getOutputDataStreams()) {
+                    if (componentBuilder.isOutputDataStreamConsumed(outputDataStream)) {
+                        final AnalysisJobBuilder outputDataStreamJobBuilder = componentBuilder
+                                .getOutputDataStreamJobBuilder(outputDataStream);
                         List<Table> sourceTables = outputDataStreamJobBuilder.getSourceTables();
-                        buildGraphInternal(graph, outputDataStreamJobBuilder, scf, sourceTables);
+                        buildGraphInternal(graph, outputDataStreamJobBuilder, sourceTables);
                         addEdge(graph, item, sourceTables.get(0), null, null);
                     }
                 }

diff --git a/engine/core/src/main/java/org/datacleaner/job/builder/AnalysisJobBuilder.java b/engine/core/src/main/java/org/datacleaner/job/builder/AnalysisJobBuilder.java
@@ -416,7 +416,7 @@ public ComponentBuilder addComponent(ComponentDescriptor<?> descriptor) {
 
     public ComponentBuilder addComponent(ComponentBuilder builder) {
         if (builder instanceof FilterComponentBuilder) {
-            return  addFilter((FilterComponentBuilder<?, ?>) builder);
+            return addFilter((FilterComponentBuilder<?, ?>) builder);
         } else if (builder instanceof TransformerComponentBuilder) {
             return addTransformer((TransformerComponentBuilder<?>) builder);
         } else if (builder instanceof AnalyzerComponentBuilder) {
@@ -428,11 +428,18 @@ public ComponentBuilder addComponent(ComponentBuilder builder) {
 
     /**
      * Adds a {@link ComponentBuilder} and removes it from its previous scope.
-     * @param builder The builder to add
+     * 
+     * @param builder
+     *            The builder to add
      * @return The same builder
      */
     public ComponentBuilder moveComponent(ComponentBuilder builder) {
         builder.getAnalysisJobBuilder().removeComponent(builder);
+
+        // when moving the component to a different scope we need to first reset
+        // the prior input
+        builder.clearInputColumns();
+
         addComponent(builder);
         builder.setAnalysisJobBuilder(this);
         return builder;
@@ -473,7 +480,7 @@ protected Object addComponent(ComponentJob componentJob) {
         return builder;
     }
 
-    public AnalysisJobBuilder removeComponent(ComponentBuilder builder){
+    public AnalysisJobBuilder removeComponent(ComponentBuilder builder) {
         if (builder instanceof FilterComponentBuilder) {
             return removeFilter((FilterComponentBuilder<?, ?>) builder);
         } else if (builder instanceof TransformerComponentBuilder) {
@@ -484,7 +491,6 @@ public AnalysisJobBuilder removeComponent(ComponentBuilder builder){
             throw new UnsupportedOperationException("Unknown component type: " + builder);
         }
 
-
     }
 
     public <F extends Filter<C>, C extends Enum<C>> FilterComponentBuilder<F, C> addFilter(Class<F> filterClass) {