From 27a6627664357a604333a22c080ad8b56e09244a Mon Sep 17 00:00:00 2001 From: Mateusz Rogalski Date: Wed, 25 Jul 2018 15:54:52 +0200 Subject: [PATCH 1/3] Avoid wiping out comments and metadata files in case of IOException FIX #2 --- .../rawgist/model/GistCommentResponse.java | 61 ++++++++++ .../repository/git/GistCommentStore.java | 12 +- .../rawgist/repository/git/GistMetadata.java | 70 +++++++++++ .../repository/git/GistMetadataStore.java | 12 +- .../repository/GistCommentStoreTest.java | 109 ++++++++++++++++++ .../repository/GistMetadataStoreTest.java | 99 ++++++++++++++++ 6 files changed, 361 insertions(+), 2 deletions(-) create mode 100644 rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistCommentStoreTest.java create mode 100644 rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistMetadataStoreTest.java diff --git a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/model/GistCommentResponse.java b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/model/GistCommentResponse.java index 4b29c83..b19114f 100644 --- a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/model/GistCommentResponse.java +++ b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/model/GistCommentResponse.java @@ -118,4 +118,65 @@ public void addAdditionalProperties(Map properties) { this.additionalProperties.putAll(properties); } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((additionalProperties == null) ? 0 : additionalProperties.hashCode()); + result = prime * result + ((body == null) ? 0 : body.hashCode()); + result = prime * result + ((createdAt == null) ? 0 : createdAt.hashCode()); + result = prime * result + ((id == null) ? 0 : id.hashCode()); + result = prime * result + ((updatedAt == null) ? 0 : updatedAt.hashCode()); + result = prime * result + ((url == null) ? 0 : url.hashCode()); + result = prime * result + ((user == null) ? 0 : user.hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + GistCommentResponse other = (GistCommentResponse) obj; + if (additionalProperties == null) { + if (other.additionalProperties != null) + return false; + } else if (!additionalProperties.equals(other.additionalProperties)) + return false; + if (body == null) { + if (other.body != null) + return false; + } else if (!body.equals(other.body)) + return false; + if (createdAt == null) { + if (other.createdAt != null) + return false; + } else if (!createdAt.equals(other.createdAt)) + return false; + if (id == null) { + if (other.id != null) + return false; + } else if (!id.equals(other.id)) + return false; + if (updatedAt == null) { + if (other.updatedAt != null) + return false; + } else if (!updatedAt.equals(other.updatedAt)) + return false; + if (url == null) { + if (other.url != null) + return false; + } else if (!url.equals(other.url)) + return false; + if (user == null) { + if (other.user != null) + return false; + } else if (!user.equals(other.user)) + return false; + return true; + } + } diff --git a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistCommentStore.java b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistCommentStore.java index 92bee49..03f6b61 100644 --- a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistCommentStore.java +++ b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistCommentStore.java @@ -14,12 +14,14 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; import org.springframework.cache.annotation.CachePut; import org.springframework.cache.annotation.Cacheable; import org.springframework.stereotype.Component; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.io.Files; import com.mangosolutions.rcloud.rawgist.model.GistCommentResponse; import com.mangosolutions.rcloud.rawgist.repository.GistError; import com.mangosolutions.rcloud.rawgist.repository.GistErrorCode; @@ -33,6 +35,9 @@ public class GistCommentStore implements CommentStore { @Autowired private ObjectMapper objectMapper; + @Value("${gists.commentstore.workingCopySuffix:.tmp}") + private String workingCopySuffix = ".tmp"; + public GistCommentStore() { this.objectMapper = new ObjectMapper(); } @@ -72,7 +77,12 @@ public List load(File store) { public List save(File store, List comments) { if(comments != null) { try { - objectMapper.writeValue(store, comments); + File tmpStore = new File(store.getParent(), store.getName() + workingCopySuffix); + if(tmpStore.exists()) { + logger.warn("{} already exists, previous Gist comments update seems to have failed.", tmpStore); + } + objectMapper.writeValue(tmpStore, comments); + Files.move(tmpStore, store); } catch (IOException e) { GistError error = new GistError(GistErrorCode.ERR_COMMENTS_NOT_WRITEABLE, "Could not save comments"); logger.error(error.getFormattedMessage() + " with path {}", store); diff --git a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadata.java b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadata.java index db892d1..dd97de6 100644 --- a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadata.java +++ b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadata.java @@ -152,4 +152,74 @@ public Fork getForkOf() { return this.forkOf; } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + (_public ? 1231 : 1237); + result = prime * result + ((additionalProperties == null) ? 0 : additionalProperties.hashCode()); + result = prime * result + ((createdAt == null) ? 0 : createdAt.hashCode()); + result = prime * result + ((description == null) ? 0 : description.hashCode()); + result = prime * result + ((forkOf == null) ? 0 : forkOf.hashCode()); + result = prime * result + ((forks == null) ? 0 : forks.hashCode()); + result = prime * result + ((id == null) ? 0 : id.hashCode()); + result = prime * result + ((owner == null) ? 0 : owner.hashCode()); + result = prime * result + ((updatedAt == null) ? 0 : updatedAt.hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + GistMetadata other = (GistMetadata) obj; + if (_public != other._public) + return false; + if (additionalProperties == null) { + if (other.additionalProperties != null) + return false; + } else if (!additionalProperties.equals(other.additionalProperties)) + return false; + if (createdAt == null) { + if (other.createdAt != null) + return false; + } else if (!createdAt.equals(other.createdAt)) + return false; + if (description == null) { + if (other.description != null) + return false; + } else if (!description.equals(other.description)) + return false; + if (forkOf == null) { + if (other.forkOf != null) + return false; + } else if (!forkOf.equals(other.forkOf)) + return false; + if (forks == null) { + if (other.forks != null) + return false; + } else if (!forks.equals(other.forks)) + return false; + if (id == null) { + if (other.id != null) + return false; + } else if (!id.equals(other.id)) + return false; + if (owner == null) { + if (other.owner != null) + return false; + } else if (!owner.equals(other.owner)) + return false; + if (updatedAt == null) { + if (other.updatedAt != null) + return false; + } else if (!updatedAt.equals(other.updatedAt)) + return false; + return true; + } + } diff --git a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadataStore.java b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadataStore.java index 314f7f6..dd78777 100644 --- a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadataStore.java +++ b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadataStore.java @@ -12,11 +12,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; import org.springframework.cache.annotation.CachePut; import org.springframework.cache.annotation.Cacheable; import org.springframework.stereotype.Component; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.io.Files; import com.mangosolutions.rcloud.rawgist.repository.GistError; import com.mangosolutions.rcloud.rawgist.repository.GistErrorCode; import com.mangosolutions.rcloud.rawgist.repository.GistRepositoryError; @@ -29,6 +31,9 @@ public class GistMetadataStore implements MetadataStore { @Autowired private ObjectMapper objectMapper; + @Value("${gists.metadatastore.workingCopySuffix:.tmp}") + private String workingCopySuffix = ".tmp"; + public GistMetadataStore() { this.objectMapper = new ObjectMapper(); } @@ -69,7 +74,12 @@ public GistMetadata load(File store) { @CachePut(cacheNames = "metadatastore", key = "#store.getAbsolutePath()") public GistMetadata save(File store, GistMetadata metadata) { try { - objectMapper.writeValue(store, metadata); + File tmpStore = new File(store.getParent(), store.getName() + workingCopySuffix); + if(tmpStore.exists()) { + logger.warn("{} already exists, previous Gist metadata update seems to have failed.", tmpStore); + } + objectMapper.writeValue(tmpStore, metadata); + Files.move(tmpStore, store); } catch (IOException e) { GistError error = new GistError(GistErrorCode.ERR_METADATA_NOT_WRITEABLE, "Could not update metadata for gist {}", metadata.getId()); logger.error(error.getFormattedMessage() + " with path {}", store); diff --git a/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistCommentStoreTest.java b/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistCommentStoreTest.java new file mode 100644 index 0000000..ebc0416 --- /dev/null +++ b/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistCommentStoreTest.java @@ -0,0 +1,109 @@ +package com.mangosolutions.rcloud.rawgist.repository; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.same; +import static org.mockito.Mockito.doThrow; + +import java.io.File; +import java.io.IOException; +import java.util.List; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.Lists; +import com.mangosolutions.rcloud.rawgist.model.GistCommentResponse; +import com.mangosolutions.rcloud.rawgist.repository.git.GistCommentStore; + +@RunWith(MockitoJUnitRunner.class) +public class GistCommentStoreTest { + + private GistCommentStore instance = new GistCommentStore(); + + @Rule + public TemporaryFolder tempFolder = new TemporaryFolder(); + + @Mock + private ObjectMapper mockObjectMapper; + + private static final String MOCK_BODY = "Some description"; + private static final long MOCK_ID = 1234l; + + @Test + public void shouldCreateNewCommentsFile() throws IOException { + File testDir = tempFolder.newFolder(); + File outputFile = new File(testDir, "comments.json"); + + GistCommentResponse comment = new GistCommentResponse(); + comment.setBody(MOCK_BODY); + comment.setId(MOCK_ID); + List comments = Lists.newArrayList(comment); + + instance.save(outputFile, comments); + + List result = instance.load(outputFile); + assertEquals(comments, result); + } + + @Test + public void shouldUpdateExistingCommentsFile() throws IOException { + File testDir = tempFolder.newFolder(); + File outputFile = new File(testDir, "comments.json"); + + outputFile.createNewFile(); + + GistCommentResponse comment = new GistCommentResponse(); + comment.setBody(MOCK_BODY); + comment.setId(MOCK_ID); + List comments = Lists.newArrayList(comment); + + instance.save(outputFile, comments); + + List result = instance.load(outputFile); + assertEquals(comments, result); + } + + @Test + public void shouldNotUpdateExistingCommentsFileInCaseOfError() throws IOException { + File testDir = tempFolder.newFolder(); + File outputFile = new File(testDir, "comments.json"); + + GistCommentResponse comment = new GistCommentResponse(); + comment.setBody(MOCK_BODY); + comment.setId(MOCK_ID); + List comments = Lists.newArrayList(comment); + + instance.save(outputFile, comments); + + GistCommentResponse comment2 = new GistCommentResponse(); + comment2.setBody("Second comment"); + comment2.setId(2345l); + List comments2 = Lists.newArrayList(comments); + comments2.add(comment2); + + doThrow(IOException.class).when(mockObjectMapper).writeValue(any(File.class), same(comments2)); + ObjectMapper functionalObjectMapper = instance.getObjectMapper(); + + instance.setObjectMapper(mockObjectMapper); + + try { + instance.save(outputFile, comments2); + fail("Expected gist error provoked by IOException thrown by ObjectMapper"); + } catch (GistRepositoryError error) { + // not interested + } + + instance.setObjectMapper(functionalObjectMapper); + List result = instance.load(outputFile); + + assertEquals("Initial comments should be loaded from the comments file", comments, result); + + } +} diff --git a/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistMetadataStoreTest.java b/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistMetadataStoreTest.java new file mode 100644 index 0000000..b5ba411 --- /dev/null +++ b/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistMetadataStoreTest.java @@ -0,0 +1,99 @@ +package com.mangosolutions.rcloud.rawgist.repository; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.same; +import static org.mockito.Mockito.doThrow; + +import java.io.File; +import java.io.IOException; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.mangosolutions.rcloud.rawgist.repository.git.GistMetadata; +import com.mangosolutions.rcloud.rawgist.repository.git.GistMetadataStore; + +@RunWith(MockitoJUnitRunner.class) +public class GistMetadataStoreTest { + + private GistMetadataStore instance = new GistMetadataStore(); + + @Rule + public TemporaryFolder tempFolder = new TemporaryFolder(); + + @Mock + private ObjectMapper mockObjectMapper; + + private static final String MOCK_DESCRIPTION = "Some description"; + private static final String MOCK_ID = "Some id"; + + @Test + public void shouldCreateNewMetadataFile() throws IOException { + File testDir = tempFolder.newFolder(); + File outputFile = new File(testDir, "gist.json"); + + GistMetadata gistMetadata = new GistMetadata(); + gistMetadata.setDescription(MOCK_DESCRIPTION); + gistMetadata.setId(MOCK_ID); + instance.save(outputFile, gistMetadata); + + GistMetadata result = instance.load(outputFile); + assertEquals(gistMetadata, result); + } + + @Test + public void shouldUpdateExistingMetadataFile() throws IOException { + File testDir = tempFolder.newFolder(); + File outputFile = new File(testDir, "gist.json"); + + outputFile.createNewFile(); + + GistMetadata gistMetadata = new GistMetadata(); + gistMetadata.setDescription(MOCK_DESCRIPTION); + gistMetadata.setId(MOCK_ID); + instance.save(outputFile, gistMetadata); + + GistMetadata result = instance.load(outputFile); + assertEquals(gistMetadata, result); + } + + @Test + public void shouldNotUpdateExistingMetadataFileInCaseOfError() throws IOException { + File testDir = tempFolder.newFolder(); + File outputFile = new File(testDir, "gist.json"); + + GistMetadata gistMetadata = new GistMetadata(); + gistMetadata.setDescription(MOCK_DESCRIPTION); + gistMetadata.setId(MOCK_ID); + instance.save(outputFile, gistMetadata); + + GistMetadata gistMetadataNew = new GistMetadata(); + gistMetadataNew.setDescription(MOCK_DESCRIPTION); + gistMetadataNew.setId(MOCK_ID); + gistMetadataNew.setOwner("Someone"); + + doThrow(IOException.class).when(mockObjectMapper).writeValue(any(File.class), same(gistMetadataNew)); + ObjectMapper functionalObjectMapper = instance.getObjectMapper(); + + instance.setObjectMapper(mockObjectMapper); + + try { + instance.save(outputFile, gistMetadata); + fail("Expected gist error provoked by IOException thrown by ObjectMapper"); + } catch (GistRepositoryError error) { + // not interested + } + + instance.setObjectMapper(functionalObjectMapper); + GistMetadata result = instance.load(outputFile); + + assertEquals("Initial Gist metadata should be loaded from metadata file", gistMetadata, result); + } +} From 0f7ce615ea5000fd85bac09c1ce0406ffbc04a39 Mon Sep 17 00:00:00 2001 From: Mateusz Rogalski Date: Wed, 25 Jul 2018 18:08:09 +0200 Subject: [PATCH 2/3] Ensure atomicity of file operations --- .../repository/git/GistCommentStore.java | 51 ++++++++++++++--- .../repository/git/GistMetadataStore.java | 56 ++++++++++++++++--- .../repository/GistCommentStoreTest.java | 29 +++++++++- .../repository/GistMetadataStoreTest.java | 28 +++++++++- 4 files changed, 145 insertions(+), 19 deletions(-) diff --git a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistCommentStore.java b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistCommentStore.java index 03f6b61..18a193e 100644 --- a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistCommentStore.java +++ b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistCommentStore.java @@ -8,6 +8,8 @@ import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.List; @@ -21,7 +23,7 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.io.Files; +import com.google.common.base.Preconditions; import com.mangosolutions.rcloud.rawgist.model.GistCommentResponse; import com.mangosolutions.rcloud.rawgist.repository.GistError; import com.mangosolutions.rcloud.rawgist.repository.GistErrorCode; @@ -59,7 +61,8 @@ public void setObjectMapper(ObjectMapper objectMapper) { @Cacheable(value = "commentstore", key = "#store.getAbsolutePath()") public List load(File store) { List comments = new ArrayList<>(); - if (store.exists()) { + File workingCopy = workingCopyFor(store); + if (store.exists() || loadState(workingCopy, store)) { try { comments = objectMapper.readValue(store, new TypeReference>() { }); @@ -77,12 +80,15 @@ public List load(File store) { public List save(File store, List comments) { if(comments != null) { try { - File tmpStore = new File(store.getParent(), store.getName() + workingCopySuffix); - if(tmpStore.exists()) { - logger.warn("{} already exists, previous Gist comments update seems to have failed.", tmpStore); + File workingCopy = new File(store.getParent(), store.getName() + workingCopySuffix); + if(!store.exists()) { + loadState(workingCopy, store); } - objectMapper.writeValue(tmpStore, comments); - Files.move(tmpStore, store); + write(workingCopy, comments); + if(store.exists()) { + Files.delete(store.toPath()); + } + Files.move(workingCopy.toPath(), store.toPath(), StandardCopyOption.ATOMIC_MOVE); } catch (IOException e) { GistError error = new GistError(GistErrorCode.ERR_COMMENTS_NOT_WRITEABLE, "Could not save comments"); logger.error(error.getFormattedMessage() + " with path {}", store); @@ -92,6 +98,37 @@ public List save(File store, List comm return comments; } + private void write(File output, List comments) throws IOException { + try { + objectMapper.writeValue(output, comments); + } catch(IOException e) { + if(output.exists()) { + Files.delete(output.toPath()); + } + GistError error = new GistError(GistErrorCode.ERR_COMMENTS_NOT_WRITEABLE, "Could not save comments"); + logger.error(error.getFormattedMessage() + " with path {}", output); + throw new GistRepositoryError(error, e); + } + } + private File workingCopyFor(File store) { + return new File(store.getParent(), store.getName() + workingCopySuffix); + } + + private boolean loadState(File from, File to) { + Preconditions.checkState(!to.exists(), "Target file '" + to.getAbsolutePath() + "' must not exist"); + if(from.exists()) { + try { + Files.move(from.toPath(), to.toPath(), StandardCopyOption.ATOMIC_MOVE); + logger.warn("{} recreated state from working copy.", from); + return true; + } catch (IOException e) { + GistError error = new GistError(GistErrorCode.ERR_METADATA_NOT_READABLE, "Could not load comments from working copy for this gist"); + logger.error(error.getFormattedMessage() + " with path {}", to); + throw new GistRepositoryError(error, e); + } + } + return false; + } } diff --git a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadataStore.java b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadataStore.java index dd78777..e75963e 100644 --- a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadataStore.java +++ b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadataStore.java @@ -8,6 +8,8 @@ import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -18,7 +20,7 @@ import org.springframework.stereotype.Component; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.io.Files; +import com.google.common.base.Preconditions; import com.mangosolutions.rcloud.rawgist.repository.GistError; import com.mangosolutions.rcloud.rawgist.repository.GistErrorCode; import com.mangosolutions.rcloud.rawgist.repository.GistRepositoryError; @@ -50,13 +52,13 @@ public ObjectMapper getObjectMapper() { public void setObjectMapper(ObjectMapper objectMapper) { this.objectMapper = objectMapper; } - - + @Override @Cacheable(value = "metadatastore", key = "#store.getAbsolutePath()") public GistMetadata load(File store) { GistMetadata metadata = null; - if(store.exists()) { + File workingCopy = workingCopyFor(store); + if(store.exists() || loadState(workingCopy, store)) { try { metadata = objectMapper.readValue(store, GistMetadata.class); } catch (IOException e) { @@ -74,12 +76,15 @@ public GistMetadata load(File store) { @CachePut(cacheNames = "metadatastore", key = "#store.getAbsolutePath()") public GistMetadata save(File store, GistMetadata metadata) { try { - File tmpStore = new File(store.getParent(), store.getName() + workingCopySuffix); - if(tmpStore.exists()) { - logger.warn("{} already exists, previous Gist metadata update seems to have failed.", tmpStore); + File workingCopy = workingCopyFor(store); + if(!store.exists()) { + loadState(workingCopy, store); + } + write(workingCopy, metadata); + if(store.exists()) { + Files.delete(store.toPath()); } - objectMapper.writeValue(tmpStore, metadata); - Files.move(tmpStore, store); + Files.move(workingCopy.toPath(), store.toPath(), StandardCopyOption.ATOMIC_MOVE); } catch (IOException e) { GistError error = new GistError(GistErrorCode.ERR_METADATA_NOT_WRITEABLE, "Could not update metadata for gist {}", metadata.getId()); logger.error(error.getFormattedMessage() + " with path {}", store); @@ -88,4 +93,37 @@ public GistMetadata save(File store, GistMetadata metadata) { return metadata; } + + private File workingCopyFor(File store) { + return new File(store.getParent(), store.getName() + workingCopySuffix); + } + + private boolean loadState(File from, File to) { + Preconditions.checkState(!to.exists(), "Target file '" + to.getAbsolutePath() + "' must not exist"); + if(from.exists()) { + try { + Files.move(from.toPath(), to.toPath(), StandardCopyOption.ATOMIC_MOVE); + logger.warn("{} recreated state from working copy.", from); + return true; + } catch (IOException e) { + GistError error = new GistError(GistErrorCode.ERR_METADATA_NOT_READABLE, "Could not load metadata from working copy for this gist"); + logger.error(error.getFormattedMessage() + " with path {}", to); + throw new GistRepositoryError(error, e); + } + } + return false; + } + + private void write(File output, GistMetadata metadata) throws IOException { + try { + objectMapper.writeValue(output, metadata); + } catch(IOException e) { + if(output.exists()) { + Files.delete(output.toPath()); + } + GistError error = new GistError(GistErrorCode.ERR_METADATA_NOT_WRITEABLE, "Could not update metadata for gist {}", metadata.getId()); + logger.error(error.getFormattedMessage() + " with path {}", output); + throw new GistRepositoryError(error, e); + } + } } diff --git a/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistCommentStoreTest.java b/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistCommentStoreTest.java index ebc0416..d062d39 100644 --- a/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistCommentStoreTest.java +++ b/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistCommentStoreTest.java @@ -1,3 +1,9 @@ +/******************************************************************************* +* Copyright (c) 2018 AT&T Intellectual Property, [http://www.att.com] +* +* SPDX-License-Identifier: MIT +* +*******************************************************************************/ package com.mangosolutions.rcloud.rawgist.repository; import static org.junit.Assert.assertEquals; @@ -19,6 +25,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.Lists; +import com.google.common.io.Files; import com.mangosolutions.rcloud.rawgist.model.GistCommentResponse; import com.mangosolutions.rcloud.rawgist.repository.git.GistCommentStore; @@ -97,7 +104,7 @@ public void shouldNotUpdateExistingCommentsFileInCaseOfError() throws IOExceptio instance.save(outputFile, comments2); fail("Expected gist error provoked by IOException thrown by ObjectMapper"); } catch (GistRepositoryError error) { - // not interested + // expected, no need to process } instance.setObjectMapper(functionalObjectMapper); @@ -106,4 +113,24 @@ public void shouldNotUpdateExistingCommentsFileInCaseOfError() throws IOExceptio assertEquals("Initial comments should be loaded from the comments file", comments, result); } + + @Test + public void shouldLoadStateFromWorkingCopyIfMainFileDoesNotExist() throws IOException { + File testDir = tempFolder.newFolder(); + File outputFile = new File(testDir, "comments.json"); + + GistCommentResponse comment = new GistCommentResponse(); + comment.setBody(MOCK_BODY); + comment.setId(MOCK_ID); + List comments = Lists.newArrayList(comment); + + instance.save(outputFile, comments); + + Files.move(outputFile, new File(outputFile.getParentFile(), outputFile.getName() + ".tmp")); + + List result = instance.load(outputFile); + + assertEquals("Initial comments should be loaded from the comments file", comments, result); + + } } diff --git a/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistMetadataStoreTest.java b/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistMetadataStoreTest.java index b5ba411..fbfbb80 100644 --- a/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistMetadataStoreTest.java +++ b/rcloud-gist-service/src/test/java/com/mangosolutions/rcloud/rawgist/repository/GistMetadataStoreTest.java @@ -1,3 +1,9 @@ +/******************************************************************************* +* Copyright (c) 2018 AT&T Intellectual Property, [http://www.att.com] +* +* SPDX-License-Identifier: MIT +* +*******************************************************************************/ package com.mangosolutions.rcloud.rawgist.repository; import static org.junit.Assert.assertEquals; @@ -17,6 +23,7 @@ import org.mockito.runners.MockitoJUnitRunner; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.io.Files; import com.mangosolutions.rcloud.rawgist.repository.git.GistMetadata; import com.mangosolutions.rcloud.rawgist.repository.git.GistMetadataStore; @@ -85,10 +92,10 @@ public void shouldNotUpdateExistingMetadataFileInCaseOfError() throws IOExceptio instance.setObjectMapper(mockObjectMapper); try { - instance.save(outputFile, gistMetadata); + instance.save(outputFile, gistMetadataNew); fail("Expected gist error provoked by IOException thrown by ObjectMapper"); } catch (GistRepositoryError error) { - // not interested + // expected, no need to process } instance.setObjectMapper(functionalObjectMapper); @@ -96,4 +103,21 @@ public void shouldNotUpdateExistingMetadataFileInCaseOfError() throws IOExceptio assertEquals("Initial Gist metadata should be loaded from metadata file", gistMetadata, result); } + + @Test + public void shouldLoadStateFromTmpStateFileIfMainFileDoesNotExist() throws IOException { + File testDir = tempFolder.newFolder(); + File outputFile = new File(testDir, "gist.json"); + + GistMetadata gistMetadata = new GistMetadata(); + gistMetadata.setDescription(MOCK_DESCRIPTION); + gistMetadata.setId(MOCK_ID); + instance.save(outputFile, gistMetadata); + + Files.move(outputFile, new File(outputFile.getParentFile(), outputFile.getName() + ".tmp")); + + GistMetadata result = instance.load(outputFile); + + assertEquals("Initial Gist metadata should be loaded from metadata file", gistMetadata, result); + } } From feae788259b8e62e8d3a2683a4602fc7d80ae6fa Mon Sep 17 00:00:00 2001 From: Mateusz Rogalski Date: Fri, 27 Jul 2018 12:56:01 +0200 Subject: [PATCH 3/3] Address peer review comments --- .../repository/git/GistCommentStore.java | 15 ++++++++++++--- .../repository/git/GistMetadataStore.java | 17 +++++++++++++---- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistCommentStore.java b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistCommentStore.java index 18a193e..62201e1 100644 --- a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistCommentStore.java +++ b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistCommentStore.java @@ -62,7 +62,7 @@ public void setObjectMapper(ObjectMapper objectMapper) { public List load(File store) { List comments = new ArrayList<>(); File workingCopy = workingCopyFor(store); - if (store.exists() || loadState(workingCopy, store)) { + if (store.exists() || restoreState(workingCopy, store)) { try { comments = objectMapper.readValue(store, new TypeReference>() { }); @@ -82,7 +82,7 @@ public List save(File store, List comm try { File workingCopy = new File(store.getParent(), store.getName() + workingCopySuffix); if(!store.exists()) { - loadState(workingCopy, store); + restoreState(workingCopy, store); } write(workingCopy, comments); if(store.exists()) { @@ -115,7 +115,16 @@ private File workingCopyFor(File store) { return new File(store.getParent(), store.getName() + workingCopySuffix); } - private boolean loadState(File from, File to) { + /** + * Restores state of to file from from by performing atomic move. + * + * @param from file + * @param to file + * @return true if the state was restored, false if from file does not exist. + * @throws GistRepositoryError if file could not be restored + * @throws IllegalStateException if to file already exists + */ + private boolean restoreState(File from, File to) { Preconditions.checkState(!to.exists(), "Target file '" + to.getAbsolutePath() + "' must not exist"); if(from.exists()) { try { diff --git a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadataStore.java b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadataStore.java index e75963e..305e02c 100644 --- a/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadataStore.java +++ b/rcloud-gist-service/src/main/java/com/mangosolutions/rcloud/rawgist/repository/git/GistMetadataStore.java @@ -58,7 +58,7 @@ public void setObjectMapper(ObjectMapper objectMapper) { public GistMetadata load(File store) { GistMetadata metadata = null; File workingCopy = workingCopyFor(store); - if(store.exists() || loadState(workingCopy, store)) { + if(store.exists() || restoreState(workingCopy, store)) { try { metadata = objectMapper.readValue(store, GistMetadata.class); } catch (IOException e) { @@ -78,7 +78,7 @@ public GistMetadata save(File store, GistMetadata metadata) { try { File workingCopy = workingCopyFor(store); if(!store.exists()) { - loadState(workingCopy, store); + restoreState(workingCopy, store); } write(workingCopy, metadata); if(store.exists()) { @@ -97,8 +97,17 @@ public GistMetadata save(File store, GistMetadata metadata) { private File workingCopyFor(File store) { return new File(store.getParent(), store.getName() + workingCopySuffix); } - - private boolean loadState(File from, File to) { + + /** + * Restores state of to file from from by performing atomic move. + * + * @param from file + * @param to file + * @return true if the state was restored, false if from file does not exist. + * @throws GistRepositoryError if file could not be restored + * @throws IllegalStateException if to file already exists + */ + private boolean restoreState(File from, File to) { Preconditions.checkState(!to.exists(), "Target file '" + to.getAbsolutePath() + "' must not exist"); if(from.exists()) { try {