-
Notifications
You must be signed in to change notification settings - Fork 107
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
Continuously backup changes and load them when GRIP is launched #822
base: master
Are you sure you want to change the base?
Changes from all commits
08f6007
e4464e4
18bb0f4
3fb9623
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,13 @@ | |
*/ | ||
public interface DirtiesSaveEvent { | ||
|
||
DirtiesSaveEvent DIRTIES_SAVE_EVENT = new DirtiesSaveEvent() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are fields on interfaces automatically final? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fields declared in interfaces are always |
||
@Override | ||
public boolean doesDirtySave() { | ||
return true; | ||
} | ||
}; | ||
|
||
/** | ||
* Some events may have more logic regarding whether they make the save dirty or not. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
package edu.wpi.grip.core.serialization; | ||
|
||
import edu.wpi.grip.core.GripFileManager; | ||
import edu.wpi.grip.core.Pipeline; | ||
import edu.wpi.grip.core.PipelineRunner; | ||
import edu.wpi.grip.core.events.DirtiesSaveEvent; | ||
|
@@ -8,6 +9,7 @@ | |
import com.google.common.annotations.VisibleForTesting; | ||
import com.google.common.eventbus.EventBus; | ||
import com.google.common.eventbus.Subscribe; | ||
import com.google.common.io.Files; | ||
import com.google.common.reflect.ClassPath; | ||
import com.thoughtworks.xstream.XStream; | ||
import com.thoughtworks.xstream.annotations.XStreamAlias; | ||
|
@@ -22,11 +24,15 @@ | |
import java.io.Reader; | ||
import java.io.StringReader; | ||
import java.io.Writer; | ||
import java.nio.charset.Charset; | ||
import java.nio.charset.StandardCharsets; | ||
import java.util.LinkedList; | ||
import java.util.List; | ||
import java.util.Optional; | ||
import java.util.function.Consumer; | ||
import java.util.logging.Level; | ||
import java.util.logging.Logger; | ||
|
||
import javax.inject.Inject; | ||
import javax.inject.Singleton; | ||
|
||
|
@@ -36,6 +42,8 @@ | |
@Singleton | ||
public class Project { | ||
|
||
private static final Logger logger = Logger.getLogger(Project.class.getName()); | ||
|
||
protected final XStream xstream = new XStream(new PureJavaReflectionProvider()); | ||
@Inject | ||
private EventBus eventBus; | ||
|
@@ -74,7 +82,6 @@ public void initialize(StepConverter stepConverter, | |
} catch (InternalError ex) { | ||
throw new AssertionError("Failed to load class: " + clazz.getName(), ex); | ||
} | ||
|
||
}); | ||
} catch (IOException ex) { | ||
throw new AssertionError("Could not load classes for XStream annotation processing", ex); | ||
|
@@ -88,8 +95,27 @@ public Optional<File> getFile() { | |
return file; | ||
} | ||
|
||
/** | ||
* Sets the current project file to the given optional one. This also updates a file on disk | ||
* so the app can "remember" the most recent save file when it's opened later. | ||
* | ||
* @param file the optional file that this project is associated with. | ||
*/ | ||
public void setFile(Optional<File> file) { | ||
file.ifPresent(f -> logger.info("Setting project file to: " + f.getAbsolutePath())); | ||
this.file = file; | ||
try { | ||
if (file.isPresent()) { | ||
Files.write(file.get().getAbsolutePath(), | ||
GripFileManager.LAST_SAVE_FILE, | ||
Charset.defaultCharset()); | ||
} else { | ||
// No project file, delete the last_save file | ||
GripFileManager.LAST_SAVE_FILE.delete(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic here doesn't quite make sense in my head. Can you explain this better? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Basically, the |
||
} | ||
} catch (IOException e) { | ||
logger.log(Level.WARNING, "Could not set last file", e); | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -100,7 +126,7 @@ public void open(File file) throws IOException { | |
StandardCharsets.UTF_8)) { | ||
this.open(reader); | ||
} | ||
this.file = Optional.of(file); | ||
setFile(Optional.of(file)); | ||
} | ||
|
||
/** | ||
|
@@ -158,11 +184,29 @@ public void save(File file) throws IOException { | |
this.file = Optional.of(file); | ||
} | ||
|
||
/** | ||
* Save the project using a writer to write the data. This will clear the dirty flag. | ||
* | ||
* @param writer the writer to use to save the project | ||
* | ||
* @see #saveRaw(Writer) | ||
*/ | ||
public void save(Writer writer) { | ||
this.xstream.toXML(this.pipeline, writer); | ||
saveIsDirty.set(false); | ||
} | ||
|
||
/** | ||
* Save the project using a writer to write the data. This has no other side effects. | ||
* | ||
* @param writer the writer to use to save the project | ||
* | ||
* @see #save(Writer) | ||
*/ | ||
public void saveRaw(Writer writer) { | ||
xstream.toXML(pipeline, writer); | ||
} | ||
|
||
public boolean isSaveDirty() { | ||
return saveIsDirty.get(); | ||
} | ||
|
@@ -173,9 +217,7 @@ public void addIsSaveDirtyConsumer(Consumer<Boolean> consumer) { | |
|
||
@Subscribe | ||
public void onDirtiesSaveEvent(DirtiesSaveEvent dirtySaveEvent) { | ||
// Only update the flag the save isn't already dirty | ||
// We don't need to be redundantly checking if the event dirties the save | ||
if (!saveIsDirty.get() && dirtySaveEvent.doesDirtySave()) { | ||
if (dirtySaveEvent.doesDirtySave()) { | ||
saveIsDirty.set(true); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
package edu.wpi.grip.ui; | ||
|
||
import edu.wpi.grip.core.GripFileManager; | ||
import edu.wpi.grip.core.events.DirtiesSaveEvent; | ||
import edu.wpi.grip.core.serialization.Project; | ||
|
||
import com.google.common.eventbus.EventBus; | ||
import com.google.inject.Inject; | ||
import com.google.inject.Singleton; | ||
import com.thoughtworks.xstream.XStreamException; | ||
|
||
import java.io.File; | ||
import java.io.IOException; | ||
import java.nio.file.Files; | ||
import java.util.List; | ||
import java.util.Optional; | ||
import java.util.logging.Level; | ||
import java.util.logging.Logger; | ||
|
||
/** | ||
* Class handling loading project backups when the app is launched. This improves the UX by letting | ||
* users resume where they left off. | ||
*/ | ||
@Singleton | ||
public class ProjectBackupLoader { | ||
|
||
private static final Logger logger = Logger.getLogger(ProjectBackupLoader.class.getName()); | ||
|
||
@Inject | ||
private EventBus eventBus; | ||
@Inject | ||
private Project project; | ||
|
||
/** | ||
* Loads the backup project, or the previous save if it's identical to the backup. If there was | ||
* no previous save, the project will have no file associated with it. Does nothing if the backup | ||
* file does not exist. | ||
*/ | ||
public void loadBackupOrPreviousSave() { | ||
if (GripFileManager.LAST_SAVE_FILE.exists()) { | ||
// Load whichever is readable and more recent, backup or previous save | ||
try { | ||
File lastSaveFile = lastSaveFile(); | ||
if (lastSaveFile != null) { | ||
// Compare last save to backup | ||
if (lastSaveFile.lastModified() >= GripFileManager.BACKUP_FILE.lastModified()) { | ||
// Last save is more recent, load that one | ||
logger.info("Loading the last save file"); | ||
project.open(lastSaveFile); | ||
} else if (GripFileManager.BACKUP_FILE.exists()) { | ||
// Load backup, set the file to the last save file (instead of the backup), | ||
// and post an event marking the save as dirty | ||
loadBackup(); | ||
project.setFile(Optional.of(lastSaveFile)); | ||
eventBus.post(DirtiesSaveEvent.DIRTIES_SAVE_EVENT); | ||
} | ||
} else if (GripFileManager.BACKUP_FILE.exists()) { | ||
// Couldn't read from the last save, just load the backup if possible | ||
loadBackup(); | ||
project.setFile(Optional.empty()); | ||
} | ||
} catch (XStreamException | IOException e) { | ||
logger.log(Level.WARNING, "Could not open the last project file", e); | ||
} | ||
} else if (GripFileManager.BACKUP_FILE.exists()) { | ||
// Load the backup, if possible | ||
loadBackup(); | ||
project.setFile(Optional.empty()); | ||
} | ||
} | ||
|
||
private File lastSaveFile() throws IOException { | ||
List<String> lines = Files.readAllLines(GripFileManager.LAST_SAVE_FILE.toPath()); | ||
if (lines.size() == 1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why if there is only one line in the file is it considered valid? I'm confused. |
||
return new File(lines.get(0)); | ||
} else { | ||
logger.warning("Unexpected data in last_save file: " + lines); | ||
return null; | ||
} | ||
} | ||
|
||
private void loadBackup() { | ||
try { | ||
logger.info("Loading backup file"); | ||
project.open(GripFileManager.BACKUP_FILE); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this fails won't the pipeline be in a bogus state and be unusable? this could even crash GRIP because the APP was in a bad state. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Welp... guess I'm going to rebase this onto #692, which fixes all of those problems There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} catch (XStreamException | IOException e) { | ||
logger.log(Level.WARNING, "Could not load backup file", e); | ||
} | ||
eventBus.post(DirtiesSaveEvent.DIRTIES_SAVE_EVENT); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this also fire this event if the deserialization fails? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does... That's why the event is after the whole try-catch block. |
||
} | ||
|
||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment explaining the specific use of
LinkedHashSet
here because clearly it matters now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm actually going to revert this. Just looking at the last modified time now.