Merge pull request #2042 from stanakaj/issue-2041
Fix column removal in reorder leaves undeleted hidden cells.
This commit is contained in:
commit
51a8cbf946
@ -0,0 +1,84 @@
|
||||
/*
|
||||
|
||||
Copyright 2010, Google Inc.
|
||||
All rights reserved.
|
||||
|
||||
Redistribution and use in source and binary forms, with or without
|
||||
modification, are permitted provided that the following conditions are
|
||||
met:
|
||||
|
||||
* Redistributions of source code must retain the above copyright
|
||||
notice, this list of conditions and the following disclaimer.
|
||||
* Redistributions in binary form must reproduce the above
|
||||
copyright notice, this list of conditions and the following disclaimer
|
||||
in the documentation and/or other materials provided with the
|
||||
distribution.
|
||||
* Neither the name of Google Inc. nor the names of its
|
||||
contributors may be used to endorse or promote products derived from
|
||||
this software without specific prior written permission.
|
||||
|
||||
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
|
||||
"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
|
||||
LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
|
||||
A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
|
||||
OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
|
||||
SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
|
||||
LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
|
||||
DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
|
||||
THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
|
||||
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
|
||||
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
|
||||
|
||||
*/
|
||||
|
||||
package com.google.refine.model.changes;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.Writer;
|
||||
import java.util.Properties;
|
||||
import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import com.google.refine.model.Cell;
|
||||
import com.google.refine.util.Pool;
|
||||
|
||||
public class CellAtRowCellIndex {
|
||||
|
||||
final public int row;
|
||||
final public int cellIndex;
|
||||
final public Cell cell;
|
||||
final static private Pattern semicolonPattern = Pattern.compile(";");
|
||||
|
||||
public CellAtRowCellIndex(int row, int cellIndex, Cell cell) {
|
||||
this.row = row;
|
||||
this.cell = cell;
|
||||
this.cellIndex = cellIndex;
|
||||
}
|
||||
|
||||
public void save(Writer writer, Properties options) throws IOException {
|
||||
writer.write(Integer.toString(row));
|
||||
writer.write(';');
|
||||
writer.write(Integer.toString(cellIndex));
|
||||
writer.write(';');
|
||||
if (cell != null) {
|
||||
cell.save(writer, options);
|
||||
}
|
||||
}
|
||||
|
||||
static public CellAtRowCellIndex load(String s, Pool pool) throws Exception {
|
||||
|
||||
Matcher m = semicolonPattern.matcher(s);
|
||||
|
||||
m.find();
|
||||
int semicolon = m.start();
|
||||
m.find();
|
||||
int nextSemicolon = m.start();
|
||||
|
||||
int row = Integer.parseInt(s.substring(0, semicolon));
|
||||
int cellIndex = Integer.parseInt(s.substring(semicolon + 1, nextSemicolon));
|
||||
Cell cell = nextSemicolon < s.length() - 1 ? Cell.loadStreaming(s.substring(nextSemicolon + 1), pool)
|
||||
: null;
|
||||
|
||||
return new CellAtRowCellIndex(row, cellIndex, cell);
|
||||
}
|
||||
}
|
@ -42,16 +42,20 @@ import java.util.List;
|
||||
import java.util.Properties;
|
||||
|
||||
import com.google.refine.history.Change;
|
||||
import com.google.refine.model.Cell;
|
||||
import com.google.refine.model.Column;
|
||||
import com.google.refine.model.ColumnGroup;
|
||||
import com.google.refine.model.Project;
|
||||
import com.google.refine.model.Row;
|
||||
import com.google.refine.util.Pool;
|
||||
|
||||
public class ColumnReorderChange extends ColumnChange {
|
||||
final protected List<String> _columnNames;
|
||||
protected List<Column> _oldColumns;
|
||||
protected List<Column> _newColumns;
|
||||
protected List<Column> _removedColumns;
|
||||
protected List<ColumnGroup> _oldColumnGroups;
|
||||
protected CellAtRowCellIndex[] _oldCells;
|
||||
|
||||
public ColumnReorderChange(List<String> columnNames) {
|
||||
_columnNames = columnNames;
|
||||
@ -74,6 +78,43 @@ public class ColumnReorderChange extends ColumnChange {
|
||||
_oldColumnGroups = new ArrayList<ColumnGroup>(project.columnModel.columnGroups);
|
||||
}
|
||||
|
||||
if (_removedColumns == null) {
|
||||
_removedColumns = new ArrayList<Column>();
|
||||
for (String n : project.columnModel.getColumnNames()) {
|
||||
Column oldColumn = project.columnModel.getColumnByName(n);
|
||||
if(!_newColumns.contains(oldColumn)) {
|
||||
_removedColumns.add(oldColumn);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (_oldCells == null) {
|
||||
_oldCells = new CellAtRowCellIndex[project.rows.size() * _removedColumns.size()];
|
||||
|
||||
int count = 0;
|
||||
for (int i = 0; i < project.rows.size(); i++) {
|
||||
for (int j = 0; j < _removedColumns.size(); j++) {
|
||||
int cellIndex = _removedColumns.get(j).getCellIndex();
|
||||
Row row = project.rows.get(i);
|
||||
|
||||
Cell oldCell = null;
|
||||
if (cellIndex < row.cells.size()) {
|
||||
oldCell = row.cells.get(cellIndex);
|
||||
}
|
||||
_oldCells[count++] = new CellAtRowCellIndex(i, cellIndex,oldCell);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
//Clear cells on removed columns.
|
||||
for (int i = 0; i < project.rows.size(); i++) {
|
||||
for (int j = 0; j < _removedColumns.size(); j++) {
|
||||
int cellIndex = _removedColumns.get(j).getCellIndex();
|
||||
Row row = project.rows.get(i);
|
||||
row.setCell(cellIndex, null);
|
||||
}
|
||||
}
|
||||
|
||||
project.columnModel.columns.clear();
|
||||
project.columnModel.columns.addAll(_newColumns);
|
||||
project.columnModel.columnGroups.clear();
|
||||
@ -91,6 +132,12 @@ public class ColumnReorderChange extends ColumnChange {
|
||||
project.columnModel.columnGroups.clear();
|
||||
project.columnModel.columnGroups.addAll(_oldColumnGroups);
|
||||
|
||||
|
||||
for (int i = 0; i < _oldCells.length; i++) {
|
||||
Row row = project.rows.get(_oldCells[i].row);
|
||||
row.setCell(_oldCells[i].cellIndex,_oldCells[i].cell);
|
||||
}
|
||||
|
||||
project.update();
|
||||
}
|
||||
}
|
||||
@ -112,6 +159,17 @@ public class ColumnReorderChange extends ColumnChange {
|
||||
c.save(writer);
|
||||
writer.write('\n');
|
||||
}
|
||||
writer.write("removedColumnCount="); writer.write(Integer.toString(_removedColumns.size())); writer.write('\n');
|
||||
for (Column c : _removedColumns) {
|
||||
c.save(writer);
|
||||
writer.write('\n');
|
||||
}
|
||||
writer.write("oldCellCount="); writer.write(Integer.toString(_oldCells.length)); writer.write('\n');
|
||||
for (CellAtRowCellIndex c : _oldCells) {
|
||||
c.save(writer, options);
|
||||
writer.write('\n');
|
||||
}
|
||||
|
||||
writeOldColumnGroups(writer, options, _oldColumnGroups);
|
||||
writer.write("/ec/\n"); // end of change marker
|
||||
}
|
||||
@ -120,6 +178,8 @@ public class ColumnReorderChange extends ColumnChange {
|
||||
List<String> columnNames = new ArrayList<String>();
|
||||
List<Column> oldColumns = new ArrayList<Column>();
|
||||
List<Column> newColumns = new ArrayList<Column>();
|
||||
List<Column> removedColumns = new ArrayList<Column>();
|
||||
CellAtRowCellIndex[] oldCells = new CellAtRowCellIndex[0];
|
||||
List<ColumnGroup> oldColumnGroups = null;
|
||||
|
||||
String line;
|
||||
@ -151,6 +211,24 @@ public class ColumnReorderChange extends ColumnChange {
|
||||
newColumns.add(Column.load(line));
|
||||
}
|
||||
}
|
||||
} else if ("removedColumnCount".equals(field)) {
|
||||
int count = Integer.parseInt(line.substring(equal + 1));
|
||||
for (int i = 0; i < count; i++) {
|
||||
line = reader.readLine();
|
||||
if (line != null) {
|
||||
removedColumns.add(Column.load(line));
|
||||
}
|
||||
}
|
||||
} else if ("oldCellCount".equals(field)) {
|
||||
int oldCellCount = Integer.parseInt(line.substring(equal + 1));
|
||||
|
||||
oldCells = new CellAtRowCellIndex[oldCellCount];
|
||||
for (int i = 0; i < oldCellCount; i++) {
|
||||
line = reader.readLine();
|
||||
if (line != null) {
|
||||
oldCells[i] = CellAtRowCellIndex.load(line, pool);
|
||||
}
|
||||
}
|
||||
} else if ("oldColumnGroupCount".equals(field)) {
|
||||
int oldColumnGroupCount = Integer.parseInt(line.substring(equal + 1));
|
||||
|
||||
@ -161,6 +239,8 @@ public class ColumnReorderChange extends ColumnChange {
|
||||
ColumnReorderChange change = new ColumnReorderChange(columnNames);
|
||||
change._oldColumns = oldColumns;
|
||||
change._newColumns = newColumns;
|
||||
change._removedColumns = removedColumns;
|
||||
change._oldCells = oldCells;
|
||||
change._oldColumnGroups = oldColumnGroups != null ?
|
||||
oldColumnGroups : new LinkedList<ColumnGroup>();
|
||||
|
||||
|
@ -1,17 +1,17 @@
|
||||
/*******************************************************************************
|
||||
* Copyright (C) 2018, OpenRefine contributors
|
||||
* All rights reserved.
|
||||
*
|
||||
*
|
||||
* Redistribution and use in source and binary forms, with or without
|
||||
* modification, are permitted provided that the following conditions are met:
|
||||
*
|
||||
*
|
||||
* 1. Redistributions of source code must retain the above copyright notice,
|
||||
* this list of conditions and the following disclaimer.
|
||||
*
|
||||
*
|
||||
* 2. Redistributions in binary form must reproduce the above copyright notice,
|
||||
* this list of conditions and the following disclaimer in the documentation
|
||||
* and/or other materials provided with the distribution.
|
||||
*
|
||||
*
|
||||
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
|
||||
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
|
||||
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
|
||||
@ -27,24 +27,39 @@
|
||||
package com.google.refine.tests.operations.column;
|
||||
|
||||
import java.util.Arrays;
|
||||
import java.util.Properties;
|
||||
|
||||
import org.testng.Assert;
|
||||
import org.testng.annotations.BeforeMethod;
|
||||
import org.testng.annotations.BeforeSuite;
|
||||
import org.testng.annotations.Test;
|
||||
|
||||
import com.google.refine.model.AbstractOperation;
|
||||
import com.google.refine.model.Project;
|
||||
import com.google.refine.operations.OperationRegistry;
|
||||
import com.google.refine.operations.column.ColumnReorderOperation;
|
||||
import com.google.refine.process.Process;
|
||||
import com.google.refine.tests.RefineTest;
|
||||
import com.google.refine.tests.util.TestUtils;
|
||||
|
||||
|
||||
public class ColumnReorderOperationTests extends RefineTest {
|
||||
|
||||
|
||||
Project project;
|
||||
|
||||
@BeforeSuite
|
||||
public void setUp() {
|
||||
OperationRegistry.registerOperation(getCoreModule(), "column-reorder", ColumnReorderOperation.class);
|
||||
}
|
||||
|
||||
|
||||
@BeforeMethod
|
||||
public void createProject() {
|
||||
project = createCSVProject(
|
||||
"a,b,c\n"+
|
||||
"1|2,d,e\n"+
|
||||
"3,f,g\n");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void serializeColumnReorderOperation() {
|
||||
AbstractOperation op = new ColumnReorderOperation(Arrays.asList("b","c","a"));
|
||||
@ -52,4 +67,32 @@ public class ColumnReorderOperationTests extends RefineTest {
|
||||
+ "\"description\":\"Reorder columns\","
|
||||
+ "\"columnNames\":[\"b\",\"c\",\"a\"]}");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testEraseCellsOnRemovedColumns() throws Exception {
|
||||
|
||||
int aCol = project.columnModel.getColumnByName("a").getCellIndex();
|
||||
int bCol = project.columnModel.getColumnByName("b").getCellIndex();
|
||||
int cCol = project.columnModel.getColumnByName("c").getCellIndex();
|
||||
|
||||
Assert.assertEquals(project.rows.get(0).getCellValue(aCol), "1|2");
|
||||
Assert.assertEquals(project.rows.get(0).getCellValue(bCol), "d");
|
||||
Assert.assertEquals(project.rows.get(0).getCellValue(cCol), "e");
|
||||
Assert.assertEquals(project.rows.get(1).getCellValue(aCol), "3");
|
||||
Assert.assertEquals(project.rows.get(1).getCellValue(bCol), "f");
|
||||
Assert.assertEquals(project.rows.get(1).getCellValue(cCol), "g");
|
||||
|
||||
|
||||
AbstractOperation op = new ColumnReorderOperation(Arrays.asList("a"));
|
||||
Process process = op.createProcess(project, new Properties());
|
||||
process.performImmediate();
|
||||
|
||||
Assert.assertEquals(project.rows.get(0).getCellValue(aCol), "1|2");
|
||||
Assert.assertEquals(project.rows.get(0).getCellValue(bCol), null);
|
||||
Assert.assertEquals(project.rows.get(0).getCellValue(cCol), null);
|
||||
Assert.assertEquals(project.rows.get(1).getCellValue(aCol), "3");
|
||||
Assert.assertEquals(project.rows.get(1).getCellValue(bCol), null);
|
||||
Assert.assertEquals(project.rows.get(1).getCellValue(cCol), null);
|
||||
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user