From b8b9feac0c4d272e0fde747a4dcd8215f0ea8ba0 Mon Sep 17 00:00:00 2001 From: s_tanaka Date: Wed, 15 May 2019 11:26:17 +0900 Subject: [PATCH] Fix column removal in reorder leaves undeleted hidden cells. --- .../model/changes/CellAtRowCellIndex.java | 84 +++++++++++++++++++ .../model/changes/ColumnReorderChange.java | 80 ++++++++++++++++++ .../column/ColumnReorderOperationTests.java | 55 ++++++++++-- 3 files changed, 213 insertions(+), 6 deletions(-) create mode 100644 main/src/com/google/refine/model/changes/CellAtRowCellIndex.java diff --git a/main/src/com/google/refine/model/changes/CellAtRowCellIndex.java b/main/src/com/google/refine/model/changes/CellAtRowCellIndex.java new file mode 100644 index 000000000..90c645dac --- /dev/null +++ b/main/src/com/google/refine/model/changes/CellAtRowCellIndex.java @@ -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); + } +} diff --git a/main/src/com/google/refine/model/changes/ColumnReorderChange.java b/main/src/com/google/refine/model/changes/ColumnReorderChange.java index bdf714998..a0bd45494 100644 --- a/main/src/com/google/refine/model/changes/ColumnReorderChange.java +++ b/main/src/com/google/refine/model/changes/ColumnReorderChange.java @@ -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 _columnNames; protected List _oldColumns; protected List _newColumns; + protected List _removedColumns; protected List _oldColumnGroups; + protected CellAtRowCellIndex[] _oldCells; public ColumnReorderChange(List columnNames) { _columnNames = columnNames; @@ -74,6 +78,43 @@ public class ColumnReorderChange extends ColumnChange { _oldColumnGroups = new ArrayList(project.columnModel.columnGroups); } + if (_removedColumns == null) { + _removedColumns = new ArrayList(); + 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 columnNames = new ArrayList(); List oldColumns = new ArrayList(); List newColumns = new ArrayList(); + List removedColumns = new ArrayList(); + CellAtRowCellIndex[] oldCells = new CellAtRowCellIndex[0]; List 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(); diff --git a/main/tests/server/src/com/google/refine/tests/operations/column/ColumnReorderOperationTests.java b/main/tests/server/src/com/google/refine/tests/operations/column/ColumnReorderOperationTests.java index 7e1ca7746..97c1afeaf 100644 --- a/main/tests/server/src/com/google/refine/tests/operations/column/ColumnReorderOperationTests.java +++ b/main/tests/server/src/com/google/refine/tests/operations/column/ColumnReorderOperationTests.java @@ -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); + + } }