This repository was archived by the owner on Nov 11, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 30
Fix open S3Object InputStream #71
Open
herdt-michael
wants to merge
8
commits into
eclipse-hawkbit:master
Choose a base branch
from
bosch-io:fix/openS3InputStream
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
544eec7
Open S3Object InputStream when creating an instance of S3Artifact to …
herdt-michael 5e8cbc3
Make S3Artifact a final class.
herdt-michael 0544b34
Add new bosch license header for 2021.
herdt-michael f1c319c
add new license header file to pom.
herdt-michael 584ea57
abort S3Object input stream on cancel.
herdt-michael aeced15
Introduce a wrapper for the S3ObjectInputStream to abort the connecti…
herdt-michael f6619af
Fix tests
herdt-michael 9c0ce6e
Make use of java.util.Base64.
herdt-michael File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,10 +8,19 @@ | |
| */ | ||
| package org.eclipse.hawkbit.artifact.repository; | ||
|
|
||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.util.Base64; | ||
|
|
||
| import com.amazonaws.services.s3.model.ObjectMetadata; | ||
| import com.amazonaws.services.s3.model.S3Object; | ||
| import com.amazonaws.services.s3.model.S3ObjectInputStream; | ||
| import com.google.common.io.BaseEncoding; | ||
| import org.apache.http.client.methods.HttpRequestBase; | ||
| import org.eclipse.hawkbit.artifact.repository.model.AbstractDbArtifact; | ||
| import org.eclipse.hawkbit.artifact.repository.model.DbArtifactHash; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import org.springframework.util.Assert; | ||
|
|
||
| import com.amazonaws.services.s3.AmazonS3; | ||
|
|
@@ -20,13 +29,25 @@ | |
| * An {@link AbstractDbArtifact} implementation which retrieves the | ||
| * {@link InputStream} from the {@link AmazonS3} client. | ||
| */ | ||
| public class S3Artifact extends AbstractDbArtifact { | ||
| public final class S3Artifact extends AbstractDbArtifact { | ||
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(S3Artifact.class); | ||
|
|
||
| private final AmazonS3 amazonS3; | ||
| private final S3RepositoryProperties s3Properties; | ||
| private final String key; | ||
| private S3Object s3Object; | ||
| private WrappedS3InputStream s3InputStream; | ||
|
|
||
| private S3Artifact(final S3Object s3Object, final AmazonS3 amazonS3, final S3RepositoryProperties s3Properties, | ||
| final String key, final String artifactId, final DbArtifactHash hashes, final Long size, | ||
| final String contentType) { | ||
| this(amazonS3, s3Properties, key, artifactId, hashes, size, contentType); | ||
| this.s3Object = s3Object; | ||
| this.s3InputStream = WrappedS3InputStream.wrap(s3Object.getObjectContent()); | ||
| } | ||
|
|
||
| S3Artifact(final AmazonS3 amazonS3, final S3RepositoryProperties s3Properties, final String key, | ||
| private S3Artifact(final AmazonS3 amazonS3, final S3RepositoryProperties s3Properties, final String key, | ||
| final String artifactId, final DbArtifactHash hashes, final Long size, final String contentType) { | ||
| super(artifactId, hashes, size, contentType); | ||
| Assert.notNull(amazonS3, "S3 cannot be null"); | ||
|
|
@@ -37,14 +58,132 @@ public class S3Artifact extends AbstractDbArtifact { | |
| this.key = key; | ||
| } | ||
|
|
||
| @Override | ||
| public InputStream getFileInputStream() { | ||
| return amazonS3.getObject(s3Properties.getBucketName(), key).getObjectContent(); | ||
| /** | ||
| * Get an S3Artifact for an already existing binary in the repository based on | ||
| * the given key. | ||
| * | ||
| * @param amazonS3 | ||
| * connection to the AmazonS3 | ||
| * @param s3Properties | ||
| * used to retrieve the bucket name | ||
| * @param key | ||
| * of the artifact | ||
| * @param artifactId | ||
| * sha1Hash to create the {@link DbArtifactHash} | ||
| * @return an instance of {@link S3Artifact} | ||
| * @throws S3ArtifactNotFoundException | ||
| * in case that no artifact could be found for the given values | ||
| */ | ||
| public static S3Artifact get(final AmazonS3 amazonS3, final S3RepositoryProperties s3Properties, final String key, | ||
| final String artifactId) { | ||
| final S3Object s3Object = getS3ObjectOrThrowException(amazonS3, s3Properties.getBucketName(), key); | ||
|
|
||
| final ObjectMetadata objectMetadata = s3Object.getObjectMetadata(); | ||
| final DbArtifactHash artifactHash = createArtifactHash(artifactId, objectMetadata); | ||
| return new S3Artifact(s3Object, amazonS3, s3Properties, key, artifactId, artifactHash, | ||
| objectMetadata.getContentLength(), objectMetadata.getContentType()); | ||
| } | ||
|
|
||
| /** | ||
| * Create a new instance of {@link S3Artifact}. In this case it is not checked | ||
| * if an artifact with the given values exists. The S3 object is empty. | ||
| * | ||
| * @param amazonS3 | ||
| * connection to the AmazonS3 | ||
| * @param s3Properties | ||
| * used to retrieve the bucket name | ||
| * @param key | ||
| * of the artifact | ||
| * @param hashes | ||
| * instance of {@link DbArtifactHash} | ||
| * @param size | ||
| * of the artifact | ||
| * @param contentType | ||
| * of the artifact | ||
| * @return an instance of {@link S3Artifact} with an empty {@link S3Object} | ||
| */ | ||
| public static S3Artifact create(final AmazonS3 amazonS3, final S3RepositoryProperties s3Properties, | ||
| final String key, final DbArtifactHash hashes, final Long size, final String contentType) { | ||
| return new S3Artifact(amazonS3, s3Properties, key, hashes.getSha1(), hashes, size, contentType); | ||
| } | ||
|
|
||
| /** | ||
| * Verify if the {@link S3Object} exists | ||
| * | ||
| * @return result of {@link AmazonS3}#doesObjectExist | ||
| */ | ||
| public boolean exists() { | ||
| return amazonS3.doesObjectExist(s3Properties.getBucketName(), key); | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "S3Artifact [key=" + key + ", getArtifactId()=" + getArtifactId() + ", getHashes()=" + getHashes() | ||
| + ", getSize()=" + getSize() + ", getContentType()=" + getContentType() + "]"; | ||
| } | ||
|
|
||
| @Override | ||
| public InputStream getFileInputStream() { | ||
| LOG.debug("Get file input stream for s3 object with key {}", key); | ||
| refreshS3ObjectIfNeeded(); | ||
| return s3InputStream; | ||
| } | ||
|
|
||
| private void refreshS3ObjectIfNeeded() { | ||
| if (s3Object == null || s3InputStream == null) { | ||
| LOG.info("Initialize S3Object in bucket {} with key {}", s3Properties.getBucketName(), key); | ||
| s3Object = amazonS3.getObject(s3Properties.getBucketName(), key); | ||
|
Contributor
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 isn't the |
||
| s3InputStream = WrappedS3InputStream.wrap(s3Object.getObjectContent()); | ||
| } | ||
| } | ||
|
|
||
| private static S3Object getS3ObjectOrThrowException(AmazonS3 amazonS3, String bucketName, String key) { | ||
| final S3Object s3Object = amazonS3.getObject(bucketName, key); | ||
|
Contributor
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. we should also catch |
||
| if (s3Object == null) { | ||
| throw new S3ArtifactNotFoundException("Cannot find s3 object by given arguments.", bucketName, key); | ||
| } | ||
| return s3Object; | ||
| } | ||
|
|
||
| private static DbArtifactHash createArtifactHash(final String artifactId, ObjectMetadata metadata) { | ||
| return new DbArtifactHash(artifactId, BaseEncoding.base16().lowerCase() | ||
| .encode(Base64.getDecoder().decode(sanitizeEtag(metadata.getETag()))), null); | ||
| } | ||
|
|
||
| private static String sanitizeEtag(final String etag) { | ||
| // base64 alphabet consist of alphanumeric characters and + / = (see RFC | ||
| // 4648) | ||
| return etag.trim().replaceAll("[^A-Za-z0-9+/=]", ""); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper to abort the http request of the S3 input stream before closing it | ||
| */ | ||
| static final class WrappedS3InputStream extends S3ObjectInputStream { | ||
|
|
||
| /** | ||
| * Constructor | ||
| */ | ||
| private WrappedS3InputStream(InputStream in, HttpRequestBase httpRequest) { | ||
| super(in, httpRequest); | ||
| } | ||
|
|
||
| /** | ||
| * Wrap an input stream of type {@link S3ObjectInputStream} to abort a | ||
| * connection before closing the stream | ||
| * | ||
| * @param inputStream | ||
| * the {@link S3ObjectInputStream} | ||
| * @return an instance of {@link WrappedS3InputStream} | ||
| */ | ||
| public static WrappedS3InputStream wrap(final S3ObjectInputStream inputStream) { | ||
| return new WrappedS3InputStream(inputStream, inputStream.getHttpRequest()); | ||
| } | ||
|
|
||
| @Override | ||
| public void close() throws IOException { | ||
| super.abort(); | ||
| super.close(); | ||
| } | ||
| } | ||
| } | ||
90 changes: 90 additions & 0 deletions
90
...s3/src/main/java/org/eclipse/hawkbit/artifact/repository/S3ArtifactNotFoundException.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| /** | ||
| * Copyright (c) 2021 Bosch.IO GmbH and others. | ||
| * | ||
| * All rights reserved. This program and the accompanying materials | ||
| * are made available under the terms of the Eclipse Public License v1.0 | ||
| * which accompanies this distribution, and is available at | ||
| * http://www.eclipse.org/legal/epl-v10.html | ||
| */ | ||
| package org.eclipse.hawkbit.artifact.repository; | ||
|
|
||
| /** | ||
| * An exception that is thrown as soon as an S3 object could not be found in a S3 bucket. | ||
| */ | ||
| public class S3ArtifactNotFoundException extends RuntimeException { | ||
|
|
||
| private final String bucket; | ||
| private final String key; | ||
|
|
||
| /** | ||
| * Constructor with individual error message and information about the searched | ||
| * artifact. | ||
| * | ||
| * @param message | ||
| * use an individual error message here. | ||
| * | ||
| * @param bucket | ||
| * the bucket of the searched artifact. | ||
| * @param key | ||
| * the key of the searched artifact (mostly kind of | ||
| * 'tenant/sha1hash'). | ||
| */ | ||
| public S3ArtifactNotFoundException(final String message, final String bucket, final String key) { | ||
| super(message); | ||
| this.bucket = bucket; | ||
| this.key = key; | ||
| } | ||
|
|
||
| /** | ||
| * Constructor with individual error message with a cause and information about | ||
| * the searched artifact. | ||
| * | ||
| * @param message | ||
| * use an individual error message here. | ||
| * | ||
| * @param cause | ||
| * the cause of the exception. | ||
| * @param bucket | ||
| * the bucket of the searched artifact. | ||
| * @param key | ||
| * the key of the searched artifact (mostly kind of | ||
| * 'tenant/sha1hash'). | ||
| */ | ||
| public S3ArtifactNotFoundException(final String message, final Throwable cause, final String bucket, | ||
| final String key) { | ||
| super(message, cause); | ||
| this.bucket = bucket; | ||
| this.key = key; | ||
| } | ||
|
|
||
| /** | ||
| * Constructor with a cause and information about the searched artifact. | ||
| * | ||
| * @param cause | ||
| * the cause of the exception. | ||
| * @param bucket | ||
| * the bucket of the searched artifact. | ||
| * @param key | ||
| * the key of the searched artifact (mostly kind of | ||
| * 'tenant/sha1hash'). | ||
| */ | ||
| public S3ArtifactNotFoundException(final Throwable cause, final String bucket, final String key) { | ||
| super(cause); | ||
| this.bucket = bucket; | ||
| this.key = key; | ||
| } | ||
|
|
||
| /** | ||
| * @return key (mostly kind of 'tenant/sha1hash'). | ||
| */ | ||
| public String getKey() { | ||
| return key; | ||
| } | ||
|
|
||
| /** | ||
| * @return the bucket name | ||
| */ | ||
| public String getBucket() { | ||
| return bucket; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,7 +28,6 @@ | |
| import com.amazonaws.services.s3.model.ObjectListing; | ||
| import com.amazonaws.services.s3.model.ObjectMetadata; | ||
| import com.amazonaws.services.s3.model.PutObjectResult; | ||
| import com.amazonaws.services.s3.model.S3Object; | ||
| import com.amazonaws.services.s3.model.S3ObjectSummary; | ||
| import com.google.common.io.BaseEncoding; | ||
|
|
||
|
|
@@ -77,7 +76,7 @@ protected AbstractDbArtifact store(final String tenant, final DbArtifactHash bas | |
| LOG.info("Storing file {} with length {} to AWS S3 bucket {} with key {}", file.getName(), file.length(), | ||
| s3Properties.getBucketName(), key); | ||
|
|
||
| if (existsByTenantAndSha1(tenant, base16Hashes.getSha1())) { | ||
| if (s3Artifact.exists()) { | ||
| LOG.debug("Artifact {} already exists on S3 bucket {}, don't need to upload twice", key, | ||
| s3Properties.getBucketName()); | ||
| return s3Artifact; | ||
|
|
@@ -100,8 +99,8 @@ protected AbstractDbArtifact store(final String tenant, final DbArtifactHash bas | |
|
|
||
| private S3Artifact createS3Artifact(final String tenant, final DbArtifactHash hashes, final String contentType, | ||
| final File file) { | ||
| return new S3Artifact(amazonS3, s3Properties, objectKey(tenant, hashes.getSha1()), hashes.getSha1(), hashes, | ||
| file.length(), contentType); | ||
| return S3Artifact.create(amazonS3, s3Properties, objectKey(tenant, hashes.getSha1()), hashes, file.length(), | ||
| contentType); | ||
| } | ||
|
|
||
| private ObjectMetadata createObjectMetadata(final String mdMD5Hash16, final String contentType, final File file) { | ||
|
|
@@ -131,37 +130,22 @@ private static String objectKey(final String tenant, final String sha1Hash) { | |
| @Override | ||
| public AbstractDbArtifact getArtifactBySha1(final String tenant, final String sha1Hash) { | ||
| final String key = objectKey(tenant, sha1Hash); | ||
|
|
||
| LOG.info("Retrieving S3 object from bucket {} and key {}", s3Properties.getBucketName(), key); | ||
| try (final S3Object s3Object = amazonS3.getObject(s3Properties.getBucketName(), key)) { | ||
| if (s3Object == null) { | ||
| return null; | ||
| } | ||
|
|
||
| final ObjectMetadata s3ObjectMetadata = s3Object.getObjectMetadata(); | ||
|
|
||
| // the MD5Content is stored in the ETag | ||
| return new S3Artifact(amazonS3, s3Properties, key, sha1Hash, | ||
| new DbArtifactHash(sha1Hash, | ||
| BaseEncoding.base16().lowerCase().encode( | ||
| BaseEncoding.base64().decode(sanitizeEtag(s3ObjectMetadata.getETag()))), | ||
| null), | ||
| s3ObjectMetadata.getContentLength(), s3ObjectMetadata.getContentType()); | ||
| } catch (final IOException e) { | ||
| LOG.error("Could not verify S3Object", e); | ||
| LOG.debug("Retrieving S3 object from bucket {} and key {}", s3Properties.getBucketName(), key); | ||
| try { | ||
| return S3Artifact.get(amazonS3, s3Properties, key, sha1Hash); | ||
| } catch (final S3ArtifactNotFoundException e) { | ||
| LOG.debug("Cannot find artifact for bucket {} with key {}", e.getBucket(), e.getKey(), e); | ||
|
Contributor
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. here, however I would use level "warn" because we always execute |
||
| return null; | ||
| } | ||
| } | ||
|
|
||
| private static String sanitizeEtag(final String etag) { | ||
| // base64 alphabet consist of alphanumeric characters and + / = (see RFC | ||
| // 4648) | ||
| return etag.trim().replaceAll("[^A-Za-z0-9+/=]", ""); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean existsByTenantAndSha1(final String tenant, final String sha1Hash) { | ||
| return amazonS3.doesObjectExist(s3Properties.getBucketName(), objectKey(tenant, sha1Hash)); | ||
| final boolean exists = amazonS3.doesObjectExist(s3Properties.getBucketName(), objectKey(tenant, sha1Hash)); | ||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("Search for artifact with sha1Hash {} results in status: {}", sha1Hash, exists); | ||
| } | ||
| return exists; | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| Copyright (c) 2021 Bosch.IO GmbH and others. | ||
|
|
||
| All rights reserved. This program and the accompanying materials | ||
| are made available under the terms of the Eclipse Public License v1.0 | ||
| which accompanies this distribution, and is available at | ||
| http://www.eclipse.org/legal/epl-v10.html |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 guess we could change the log level to debug otherwise we would get a lot of log messages for each file download