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

Feat/content preservation #1406

Merged
merged 28 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
dad9789
wip
paulcwarren May 5, 2023
632a0c5
feat: add a new setContent method accepting a SetContentParams arg wi…
paulcwarren May 9, 2023
56a1cb9
test: fix tests broken by setContent(T, InputStream) refactor
paulcwarren May 10, 2023
c6aabed
feat: dont refactor setContent(T, InputStream)
paulcwarren May 10, 2023
ec1158c
ci: set up a tmate if ci fails
paulcwarren May 11, 2023
5081871
ci: redfine the action/cache key
paulcwarren May 11, 2023
2107217
ci: ensure maven cache is saved
paulcwarren May 11, 2023
73b7e51
c
paulcwarren May 11, 2023
e550235
wip
paulcwarren May 16, 2023
e608523
feat: implement the repository version of setContnt with SetContentPa…
paulcwarren May 16, 2023
12c0857
feat: s3 module implements setContent with SetContentParams (unsuppor…
paulcwarren May 16, 2023
abd5cd6
feat: wire overwriteExistingContent property through from auto config…
paulcwarren May 16, 2023
0d30aab
feat: implement new setContent with SetContentParams method in azure …
paulcwarren May 17, 2023
c5cac3c
feat: implement new setContent with SetContentParams method in the mo…
paulcwarren May 17, 2023
42d2c1d
feat: implement new setContent with SetContentParams method in gcs st…
paulcwarren May 18, 2023
fdfed43
feat: implement new setContent with SetContentParams method in jpa st…
paulcwarren May 18, 2023
c9498b3
feat: implement new setContent with SetContentParams method in s3 sto…
paulcwarren May 18, 2023
52b729e
feat: encryption storage module now support new ContentStore
paulcwarren May 19, 2023
b5e737a
fix: store rest controller multipart form put handler mis-specified a…
paulcwarren May 24, 2023
0191af8
feat: add unsetContent with UnsetContentParams to all storage modules…
paulcwarren May 31, 2023
764ec2d
feat: refactor SetContentParams.overwriteExistingContent as ContentDi…
paulcwarren Jun 1, 2023
8bacb29
feat: refactor boot and rest layer overwrite existing content propert…
paulcwarren Jun 6, 2023
cf9b0d2
test: update tests broken by introduction of UnsetContentParams
paulcwarren Jun 13, 2023
9476757
feat: encrypting content store interface should implement UnsetConten…
paulcwarren Jun 13, 2023
68d896d
ci: use feat/content-preservation branch of gettingstarted to verify
paulcwarren Jun 13, 2023
49e4921
code: refactor StoreImpl
paulcwarren Jun 14, 2023
ffb0406
fix: StoreImpl fragment should pass modified content through to delegate
paulcwarren Jun 15, 2023
045848b
docs: update docs to explain new SetContentParams, UnsetContentParams…
paulcwarren Jun 15, 2023
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
6 changes: 5 additions & 1 deletion .github/workflows/prs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ jobs:
git checkout main
mvn -B clean install
popd
- name: Setup tmate session if anything fails
if: ${{ failure() }}
uses: mxschmitt/action-tmate@v3
timeout-minutes: 360

validate-with-gettingstarteds:
runs-on: ubuntu-latest
Expand All @@ -93,7 +97,7 @@ jobs:
with:
repository: paulcwarren/spring-content-gettingstarted
path: spring-content-gettingstarted
ref: refs/heads/main
ref: refs/heads/feat/content-preservation
- name: Validate against Getting Started Guides
run: |
export AWS_REGION=us-west-1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.content.rest.config.RestConfiguration;
import org.springframework.content.rest.config.SetContentDisposition;
import org.springframework.content.rest.config.UnsetContentDisposition;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.stereotype.Component;

Expand All @@ -25,6 +26,10 @@ public static class ContentRestProperties {
private URI baseUri;
private boolean fullyQualifiedLinks = RestConfiguration.FULLY_QUALIFIED_DEFAULTS_DEFAULT;
private ShortcutRequestMappings requestMappings = new ShortcutRequestMappings();
private boolean overwriteExistingContent = RestConfiguration.OVERWRITE_EXISTING_CONTENT_DEFAULT;

private SetContentDisposition setContentDisposition = RestConfiguration.SETCONTENT_CONTENT_DISPOSITION_DEFAULT;
private UnsetContentDisposition unsetContentDisposition = RestConfiguration.UNSETCONTENT_CONTENT_DISPOSITION_DEFAULT;

public URI getBaseUri() {
return baseUri;
Expand All @@ -50,6 +55,30 @@ public void setShortcutRequestMappings(ShortcutRequestMappings requestMappings)
this.requestMappings = requestMappings;
}

public boolean getOverwriteExistingContent() {
return this.overwriteExistingContent;
}

public void setOverwriteExistingContent(boolean overwriteExistingContent) {
this.overwriteExistingContent = overwriteExistingContent;
}

public SetContentDisposition getSetContentDisposition() {
return this.setContentDisposition;
}

public void setSetContentDisposition(SetContentDisposition setContentDisposition) {
this.setContentDisposition = setContentDisposition;
}

public UnsetContentDisposition getUnsetContentDisposition() {
return this.unsetContentDisposition;
}

public void setUnsetContentDisposition(UnsetContentDisposition unsetContentDisposition) {
this.unsetContentDisposition = unsetContentDisposition;
}

public static class ShortcutRequestMappings {

private boolean disabled = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,10 @@ public void configure(RestConfiguration config) {
}
}
}

config.setOverwriteExistingContent(properties.getOverwriteExistingContent());

config.setSetContentDisposition(properties.getSetContentDisposition());
config.setUnsetContentDisposition(properties.getUnsetContentDisposition());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,14 @@ public class ContentRestAutoConfigurationTest {
System.setProperty("spring.content.rest.fully-qualified-links", "false");
System.setProperty("spring.content.rest.shortcut-request-mappings.disabled", "true");
System.setProperty("spring.content.rest.shortcut-request-mappings.excludes", "GET=a/b,c/d:PUT=*/*");
System.setProperty("spring.content.rest.overwrite-existing-content", "false");
});
AfterEach(() -> {
System.clearProperty("spring.content.rest.base-uri");
System.clearProperty("spring.content.rest.fully-qualified-links");
System.clearProperty("spring.content.rest.shortcut-request-mappings.disabled");
System.clearProperty("spring.content.rest.shortcut-request-mappings.excludes");
System.clearProperty("spring.content.rest.overwrite-existing-content");
});
It("should have a filesystem properties bean with the correct properties set", () -> {
AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();
Expand All @@ -72,6 +74,7 @@ public class ContentRestAutoConfigurationTest {
assertThat(context.getBean(ContentRestAutoConfiguration.ContentRestProperties.class).fullyQualifiedLinks(), is(false));
assertThat(context.getBean(ContentRestAutoConfiguration.ContentRestProperties.class).shortcutRequestMappings().disabled(), is(true));
assertThat(context.getBean(ContentRestAutoConfiguration.ContentRestProperties.class).shortcutRequestMappings().excludes(), is("GET=a/b,c/d:PUT=*/*"));
assertThat(context.getBean(ContentRestAutoConfiguration.ContentRestProperties.class).getOverwriteExistingContent(), is(false));

assertThat(context.getBean(SpringBootContentRestConfigurer.class), is(not(nullValue())));

Expand Down
19 changes: 10 additions & 9 deletions spring-content-azure-storage/src/main/asciidoc/azure.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -99,24 +99,25 @@ instance of `BlobId`

See <<configuring_converters,Configuring a Spring Converter>> for more information on how to register a converter.

=== Setting Content using a ContentStore
=== Setting Content

Storing content is achieved using the `ContentStore.setContent(entity, InputStream)` method.
Storing content is achieved using the `ContentStore.setContent(T entity, PropertyPath path, InputStream content, SetContentParams params)` method.

If content has not yet been stored with this entity and an Id has not been assigned one will be generated
based in `java.util.UUID`.
The `PropertyPath` will be used to resolve the content property to update.

If content has not yet been stored with this entity and an Id has not been assigned, one will be generated based in `java.util.UUID`.

The `@ContentId` and `@ContentLength` annotations will be updated on `entity`.

If content has previously been stored it will overwritten updating just the @ContentLength attribute, if present.
If content has previously been stored it will be overwritten also updating the @ContentLength attribute, if present. However, using `ContentDisposition.Create` on the `SetContentParams` a new Id will be assigned and content stored, leaving the existing content in place and orphaned.

=== Getting Content from a ContentStore
=== Getting Content

Content can be accessed using the `ContentStore.getContent(entity)` method.
Content can be accessed using the `ContentStore.getContent(T entity, PropertyPath path)` method.

=== Unsetting Content from a ContentStore
=== Unsetting Content

Content can be removed using the `ContentStore.unsetContent(entity)` method.
Content can be removed using the `ContentStore.unsetContent(T entity, PropertyPath path, UnsetContentParams params)` method. Using `ContentDisposition.Keep` on `UnsetContentParams` will leave the content in storage and orphaned.

=== Configuring a Spring Converter
[[configuring_converters]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@
import org.springframework.content.commons.mappingcontext.ContentProperty;
import org.springframework.content.commons.mappingcontext.MappingContext;
import org.springframework.content.commons.property.PropertyPath;
import org.springframework.content.commons.store.AssociativeStore;
import org.springframework.content.commons.store.GetResourceParams;
import org.springframework.content.commons.store.StoreAccessException;
import org.springframework.content.commons.repository.SetContentParams;
import org.springframework.content.commons.store.*;
import org.springframework.content.commons.utils.BeanUtils;
import org.springframework.content.commons.utils.Condition;
import org.springframework.content.commons.utils.PlacementService;
Expand All @@ -45,7 +44,7 @@ public class DefaultAzureStorageImpl<S, SID extends Serializable>
implements org.springframework.content.commons.repository.Store<SID>,
org.springframework.content.commons.repository.AssociativeStore<S, SID>,
org.springframework.content.commons.repository.ContentStore<S, SID>,
AssociativeStore<S, SID> {
ContentStore<S, SID> {

private static Log logger = LogFactory.getLog(DefaultAzureStorageImpl.class);

Expand Down Expand Up @@ -290,14 +289,26 @@ public S setContent(S entity, PropertyPath propertyPath, InputStream content) {
@Transactional
@Override
public S setContent(S entity, PropertyPath propertyPath, InputStream content, long contentLen) {
return this.setContent(entity, propertyPath, content, org.springframework.content.commons.store.SetContentParams.builder().contentLength(contentLen).build());
}

@Override
public S setContent(S entity, PropertyPath propertyPath, InputStream content, SetContentParams params) {
return this.setContent(entity, propertyPath, content,
org.springframework.content.commons.store.SetContentParams.builder()
.contentLength(params.getContentLength())
.overwriteExistingContent(params.isOverwriteExistingContent()).build());
}

@Override
public S setContent(S entity, PropertyPath propertyPath, InputStream content, org.springframework.content.commons.store.SetContentParams params) {
ContentProperty property = this.mappingContext.getContentProperty(entity.getClass(), propertyPath.getName());
if (property == null) {
throw new StoreAccessException(String.format("Content property %s does not exist", propertyPath.getName()));
}

Object contentId = property.getContentId(entity);
if (contentId == null) {
if (contentId == null || params.getDisposition().equals(org.springframework.content.commons.store.SetContentParams.ContentDisposition.CreateNew)) {

Serializable newId = UUID.randomUUID().toString();

Expand All @@ -323,7 +334,7 @@ public S setContent(S entity, PropertyPath propertyPath, InputStream content, lo
}

try {
long lenToSet = contentLen;
long lenToSet = params.getContentLength();
if (lenToSet == -1L) {
lenToSet = resource.contentLength();
}
Expand Down Expand Up @@ -446,7 +457,22 @@ public boolean matches(Field field) {
@Transactional
@Override
public S unsetContent(S entity, PropertyPath propertyPath) {
return this.unsetContent(entity, propertyPath, UnsetContentParams.builder().build());
}

@Transactional
@Override
public S unsetContent(S entity, PropertyPath propertyPath, org.springframework.content.commons.repository.UnsetContentParams params) {
int ordinal = params.getDisposition().ordinal();
UnsetContentParams params1 = UnsetContentParams.builder()
.disposition(UnsetContentParams.Disposition.values()[ordinal])
.build();
return this.unsetContent(entity, propertyPath, params1);
}

@Transactional
@Override
public S unsetContent(S entity, PropertyPath propertyPath, UnsetContentParams params) {
ContentProperty property = this.mappingContext.getContentProperty(entity.getClass(), propertyPath.getName());
if (property == null) {
throw new StoreAccessException(String.format("Content property %s does not exist", propertyPath.getName()));
Expand All @@ -456,7 +482,7 @@ public S unsetContent(S entity, PropertyPath propertyPath) {
return entity;

Resource resource = this.getResource(entity, propertyPath);
if (resource != null && resource.exists() && resource instanceof DeletableResource) {
if (resource != null && resource.exists() && resource instanceof DeletableResource && params.getDisposition().equals(UnsetContentParams.Disposition.Remove)) {

try {
((DeletableResource)resource).delete();
Expand All @@ -474,8 +500,8 @@ public boolean matches(TypeDescriptor descriptor) {
if ("jakarta.persistence.Id".equals(
annotation.annotationType().getCanonicalName())
|| "org.springframework.data.annotation.Id"
.equals(annotation.annotationType()
.getCanonicalName())) {
.equals(annotation.annotationType()
.getCanonicalName())) {
return false;
}
}
Expand All @@ -488,7 +514,7 @@ public boolean matches(TypeDescriptor descriptor) {
return entity;
}

private String absolutify(String bucket, String location) {
private String absolutify(String bucket, String location) {
String locationToUse = null;
Assert.state(location.startsWith("azure-blob://") == false);
if (location.startsWith("/")) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
package internal.org.springframework.content.azure.it;

import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.AfterEach;
import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.BeforeEach;
import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.Context;
import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.Describe;
import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.It;
import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.*;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
Expand All @@ -18,6 +15,7 @@
import java.io.OutputStream;
import java.util.UUID;

import com.azure.storage.blob.specialized.BlockBlobClient;
import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
Expand All @@ -37,6 +35,8 @@
import org.springframework.content.commons.repository.StoreAccessException;
import org.springframework.content.commons.store.ContentStore;
import org.springframework.content.commons.store.GetResourceParams;
import org.springframework.content.commons.store.SetContentParams;
import org.springframework.content.commons.store.UnsetContentParams;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
Expand Down Expand Up @@ -379,7 +379,31 @@ public class AzureStorageIT {
});
});

Context("when content is deleted", () -> {
Context("when content is updated and not overwritten", () -> {
It("should have the updated content", () -> {
BlobContainerClient c = builder.buildClient().getBlobContainerClient("azure-test-bucket");

String contentId = entity.getContentId();
assertThat(c.getBlobClient(contentId).getBlockBlobClient().exists(), is(true));

store.setContent(entity, PropertyPath.from("content"), new ByteArrayInputStream("Hello Updated Spring Content World!".getBytes()), SetContentParams.builder().disposition(SetContentParams.ContentDisposition.CreateNew).build());
entity = repo.save(entity);

boolean matches = false;
try (InputStream content = store.getContent(entity)) {
matches = IOUtils.contentEquals(new ByteArrayInputStream("Hello Updated Spring Content World!".getBytes()), content);
assertThat(matches, is(true));
}

assertThat(c.getBlobClient(contentId).getBlockBlobClient().exists(), is(true));

assertThat(entity.getContentId(), is(not(contentId)));

assertThat(c.getBlobClient(entity.getContentId()).getBlockBlobClient().exists(), is(true));
});
});

Context("when content is unset", () -> {
BeforeEach(() -> {
resourceLocation = entity.getContentId().toString();
entity = store.unsetContent(entity);
Expand All @@ -396,13 +420,37 @@ public class AzureStorageIT {
assertThat(entity.getContentId(), is(Matchers.nullValue()));
Assert.assertEquals(entity.getContentLen(), 0);

BlobContainerClient c = builder.buildClient().getBlobContainerClient("azure-test-bucket");
assertThat(c.getBlobClient(resourceLocation).getBlockBlobClient().exists(), is(false));

//rendition
try (InputStream content = store.getContent(entity, PropertyPath.from("rendition"))) {
assertThat(content, is(Matchers.nullValue()));
}

assertThat(entity.getRenditionId(), is(Matchers.nullValue()));
Assert.assertEquals(entity.getRenditionLen(), 0);
});
});

Context("when content is unset but kept", () -> {
BeforeEach(() -> {
resourceLocation = entity.getContentId().toString();
entity = store.unsetContent(entity, PropertyPath.from("content"), UnsetContentParams.builder().disposition(UnsetContentParams.Disposition.Keep).build());
entity = repo.save(entity);
});

It("should have no content", () -> {
//content
try (InputStream content = store.getContent(entity)) {
assertThat(content, is(Matchers.nullValue()));
}

assertThat(entity.getContentId(), is(Matchers.nullValue()));
Assert.assertEquals(entity.getContentLen(), 0);

BlobContainerClient c = builder.buildClient().getBlobContainerClient("azure-test-bucket");
assertThat(c.getBlobClient(resourceLocation).getBlockBlobClient().exists(), is(true));
});
});

Expand Down
Loading