Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue#935 #1024

Merged
merged 1 commit into from
Jul 15, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions catroid/src/org/catrobat/catroid/common/Constants.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,21 @@
* Catroid: An on-device visual programming system for Android devices
* Copyright (C) 2010-2013 The Catrobat Team
* (<http://developer.catrobat.org/credits>)
*
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
*
* An additional term exception under section 7 of the GNU Affero
* General Public License, version 3, is available at
* http://developer.catrobat.org/license_additional_term
*
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
Expand All @@ -34,6 +34,7 @@ public final class Constants {
public static final int APPLICATION_BUILD_NUMBER = 0; // updated from jenkins nightly/release build
public static final String APPLICATION_BUILD_NAME = ""; // updated from jenkins nightly/release build
public static final String PROJECTCODE_NAME = "code.xml";
public static final String PROJECTCODE_NAME_TMP = "tmp_" + PROJECTCODE_NAME;

public static final String CATROBAT_EXTENSION = ".catrobat";
public static final String IMAGE_STANDARD_EXTENTION = ".png";
Expand Down Expand Up @@ -84,11 +85,10 @@ public final class Constants {
public static final String EXTRA_X_VALUE_POCKET_PAINT = "org.catrobat.extra.PAINTROID_X";
public static final String EXTRA_Y_VALUE_POCKET_PAINT = "org.catrobat.extra.PAINTROID_Y";
public static final String POCKET_PAINT_PACKAGE_NAME = "org.catrobat.paintroid";
public static final String POCKET_PAINT_DOWNLOAD_LINK = "market://details?id=" + POCKET_PAINT_PACKAGE_NAME;
public static final String POCKET_PAINT_INTENT_ACTIVITY_NAME = "org.catrobat.paintroid.MainActivity";

//Various:
public static final int BUFFER_8K = 8 * 1024;
public static final String POCKET_PAINT_DOWNLOAD_LINK = "market://details?id=" + POCKET_PAINT_PACKAGE_NAME;
public static final String PREF_PROJECTNAME_KEY = "projectName";
public static final String PROJECTNAME_TO_LOAD = "projectNameToLoad";

Expand Down
112 changes: 98 additions & 14 deletions catroid/src/org/catrobat/catroid/io/StorageHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import android.graphics.Bitmap.CompressFormat;
import android.util.Log;

import com.google.common.base.Charsets;
import com.google.common.io.Files;
import com.thoughtworks.xstream.XStream;
import com.thoughtworks.xstream.converters.reflection.FieldDictionary;
import com.thoughtworks.xstream.converters.reflection.PureJavaReflectionProvider;
Expand Down Expand Up @@ -130,6 +132,7 @@
import static org.catrobat.catroid.common.Constants.IMAGE_DIRECTORY;
import static org.catrobat.catroid.common.Constants.NO_MEDIA_FILE;
import static org.catrobat.catroid.common.Constants.PROJECTCODE_NAME;
import static org.catrobat.catroid.common.Constants.PROJECTCODE_NAME_TMP;
import static org.catrobat.catroid.common.Constants.SOUND_DIRECTORY;
import static org.catrobat.catroid.utils.Utils.buildPath;
import static org.catrobat.catroid.utils.Utils.buildProjectPath;
Expand Down Expand Up @@ -279,6 +282,10 @@ public File getBackPackSoundDirectory() {
}

public Project loadProject(String projectName) {
codeFileSanityCheck(projectName);

Log.d(TAG, "loadProject " + projectName);

loadSaveLock.lock();
try {
File projectCodeFile = new File(buildProjectPath(projectName), PROJECTCODE_NAME);
Expand Down Expand Up @@ -311,22 +318,51 @@ public boolean cancelLoadProject() {
return false;
}


public boolean saveProject(Project project) {
BufferedWriter writer = null;

if (project == null) {
return false;
}

Log.d(TAG, "saveProject " + project.getName());

codeFileSanityCheck(project.getName());

loadSaveLock.lock();

String projectXml;
File tmpCodeFile = null;
File currentCodeFile = null;

try {

if (project == null) {
return false;
projectXml = XML_HEADER.concat(xstream.toXML(project));
tmpCodeFile = new File(buildProjectPath(project.getName()), PROJECTCODE_NAME_TMP);
currentCodeFile = new File(buildProjectPath(project.getName()), PROJECTCODE_NAME);

if (currentCodeFile.exists()) {
try {
String oldProjectXml = Files.toString(currentCodeFile, Charsets.UTF_8);

if (oldProjectXml.equals(projectXml)) {
Log.d(TAG, "Project version is the same. Do not update " + currentCodeFile.getName());
return false;
}
Log.d(TAG, "Project version differ <" + oldProjectXml.length() + "> <"
+ projectXml.length() + ">. update " + currentCodeFile.getName());

} catch (Exception exception) {
Log.e(TAG, "Opening old project " + currentCodeFile.getName() + " failed.", exception);
return false;
}
}

File projectDirectory = new File(buildProjectPath(project.getName()));
createProjectDataStructure(projectDirectory);

writer = new BufferedWriter(new FileWriter(new File(projectDirectory, PROJECTCODE_NAME)),
Constants.BUFFER_8K);
String projectXml = XML_HEADER.concat(xstream.toXML(project));
writer = new BufferedWriter(new FileWriter(tmpCodeFile), Constants.BUFFER_8K);
writer.write(projectXml);
writer.flush();
return true;
Expand All @@ -337,10 +373,53 @@ public boolean saveProject(Project project) {
if (writer != null) {
try {
writer.close();

if (currentCodeFile.exists() && !currentCodeFile.delete()) {
Log.e(TAG, "Could not delete " + currentCodeFile.getName());
}

if (!tmpCodeFile.renameTo(currentCodeFile)) {
Log.e(TAG, "Could not rename " + currentCodeFile.getName());
}

} catch (IOException ioException) {
Log.e(TAG, "Failed closing the buffered writer", ioException);
}
}

loadSaveLock.unlock();
}
}

public void codeFileSanityCheck(String projectName) {
loadSaveLock.lock();

try {
File tmpCodeFile = new File(buildProjectPath(projectName), PROJECTCODE_NAME_TMP);

if (tmpCodeFile.exists()) {
File currentCodeFile = new File(buildProjectPath(projectName), PROJECTCODE_NAME);
if (currentCodeFile.exists()) {
Log.w(TAG, "TMP File probably corrupted. Both files exist. Discard " + tmpCodeFile.getName());

if (!tmpCodeFile.delete()) {
Log.e(TAG, "Could not delete " + tmpCodeFile.getName());
}

return;
}

Log.w(TAG, "Process interrupted before renaming. Rename " + PROJECTCODE_NAME_TMP +
" to " + PROJECTCODE_NAME);

if (!tmpCodeFile.renameTo(currentCodeFile)) {
Log.e(TAG, "Could not rename " + tmpCodeFile.getName());
}

}
} catch (Exception exception) {
Log.e(TAG, "Exception " + exception);
} finally {
loadSaveLock.unlock();
}
}
Expand Down Expand Up @@ -432,15 +511,13 @@ public File copySoundFile(String path) throws IOException, IllegalArgumentExcept
File outputFile = new File(buildPath(soundDirectory.getAbsolutePath(),
inputFileChecksum + "_" + inputFile.getName()));

return copyFileAddCheckSum(outputFile, inputFile, soundDirectory);
return copyFileAddCheckSum(outputFile, inputFile);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also at copyFile, 3rd Parameter was never used (or implemented)


public File copySoundFileBackPack(SoundInfo selectedSoundInfo) throws IOException, IllegalArgumentException {

String path = selectedSoundInfo.getAbsolutePath();

File backPackDirectory = new File(buildPath(DEFAULT_ROOT, BACKPACK_DIRECTORY, BACKPACK_SOUND_DIRECTORY));

File inputFile = new File(path);
if (!inputFile.exists() || !inputFile.canRead()) {
throw new IllegalArgumentException("file " + path + " doesn`t exist or can`t be read");
Expand All @@ -452,7 +529,7 @@ public File copySoundFileBackPack(SoundInfo selectedSoundInfo) throws IOExceptio
File outputFile = new File(buildPath(DEFAULT_ROOT, BACKPACK_DIRECTORY, BACKPACK_SOUND_DIRECTORY, currentProject
+ "_" + selectedSoundInfo.getTitle() + "_" + inputFileChecksum));

return copyFileAddCheckSum(outputFile, inputFile, backPackDirectory);
return copyFileAddCheckSum(outputFile, inputFile);
}

public File copyImage(String currentProjectName, String inputFilePath, String newName) throws IOException {
Expand Down Expand Up @@ -493,7 +570,7 @@ public File copyImage(String currentProjectName, String inputFilePath, String ne
}

File outputFile = new File(newFilePath);
return copyFileAddCheckSum(outputFile, inputFile, imageDirectory);
return copyFileAddCheckSum(outputFile, inputFile);
}
}

Expand All @@ -512,7 +589,7 @@ public File makeTempImageCopy(String inputFilePath) throws IOException {

File outputFile = new File(Constants.TMP_IMAGE_PATH);

File copiedFile = UtilFile.copyFile(outputFile, inputFile, tempDirectory);
File copiedFile = UtilFile.copyFile(outputFile, inputFile);

return copiedFile;
}
Expand Down Expand Up @@ -586,11 +663,18 @@ public void fillChecksumContainer() {
}

public String getXMLStringOfAProject(Project project) {
return xstream.toXML(project);
loadSaveLock.lock();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the lock does not only protect the codefiles, it is also for the xstream object (references are checked, it is only used within this class)

This solved a testfailure with concurrentmodificationexception.

String xmlProject = "";
try {
xmlProject = xstream.toXML(project);
} finally {
loadSaveLock.unlock();
}
return xmlProject;
}

private File copyFileAddCheckSum(File destinationFile, File sourceFile, File directory) throws IOException {
File copiedFile = UtilFile.copyFile(destinationFile, sourceFile, directory);
private File copyFileAddCheckSum(File destinationFile, File sourceFile) throws IOException {
File copiedFile = UtilFile.copyFile(destinationFile, sourceFile);
addChecksum(destinationFile, sourceFile);

return copiedFile;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ private void copyDirectory(File destinationFile, File sourceFile) throws IOExcep
copyDirectory(new File(destinationFile, subDirectoryName), new File(sourceFile, subDirectoryName));
}
} else {
UtilFile.copyFile(destinationFile, sourceFile, null);
UtilFile.copyFile(destinationFile, sourceFile);
}
}
}
17 changes: 8 additions & 9 deletions catroid/src/org/catrobat/catroid/utils/UtilFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,21 @@
* Catroid: An on-device visual programming system for Android devices
* Copyright (C) 2010-2013 The Catrobat Team
* (<http://developer.catrobat.org/credits>)
*
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
*
* An additional term exception under section 7 of the GNU Affero
* General Public License, version 3, is available at
* http://developer.catrobat.org/license_additional_term
*
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
Expand Down Expand Up @@ -46,10 +46,6 @@
import java.util.Locale;

public final class UtilFile {
public enum FileType {
TYPE_IMAGE_FILE, TYPE_SOUND_FILE
}

private static final String TAG = UtilFile.class.getSimpleName();

// Suppress default constructor for noninstantiability
Expand Down Expand Up @@ -209,7 +205,7 @@ public boolean accept(File dir, String filename) {
return projectList;
}

public static File copyFile(File destinationFile, File sourceFile, File directory) throws IOException {
public static File copyFile(File destinationFile, File sourceFile) throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, not really part of this PR, but while we're at it, remove this abomination and replace with Guava's [copy()](http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/io/Files.html#copy%28java.io.File, java.io.File%29) function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or never mind, I'll do it myself and look for other places where we could use Guava. But the new fileAsString() method should never make it to the master either way.

FileInputStream inputStream = null;
FileChannel inputChannel = null;
FileOutputStream outputStream = null;
Expand Down Expand Up @@ -342,4 +338,7 @@ public static String decodeSpecialCharsForFileSystem(String projectName) {
return projectName;
}

public enum FileType {
TYPE_IMAGE_FILE, TYPE_SOUND_FILE
}
}
Loading