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

Ref: Remove Attachment ContentType since the Server infers it #1874

Merged
merged 4 commits into from
Jan 18, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

* Feat: Relax TransactionNameProvider (#1861)
* Ref: Simplify DateUtils with ISO8601Utils (#1837)
* Ref: Remove Attachment ContentType since the Server infers it (#1874)
* Ref: Add shutdownTimeoutMillis in favor of shutdownTimeout (#1873)

## 6.0.0-alpha.1
Expand Down
32 changes: 12 additions & 20 deletions sentry/src/main/java/io/sentry/Attachment.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,12 @@ public final class Attachment {
private @Nullable byte[] bytes;
private @Nullable String pathname;
private final @NotNull String filename;
private final @NotNull String contentType;
private final @Nullable String contentType;
private final boolean addToTransactions;

/** The special type of this attachment */
private @Nullable String attachmentType = DEFAULT_ATTACHMENT_TYPE;

/**
* We could use Files.probeContentType(path) to determine the content type of the filename. This
* needs a path, but file.toPath or Paths.get only work on above Android API level 26, see
* https://developer.android.com/reference/java/nio/file/Paths. There are also ways via
* URLConnection, but we don't want to use this in constructors. Therefore we use the default
* content type of Sentry.
*/
private static final String DEFAULT_CONTENT_TYPE = "application/octet-stream";

/** A standard attachment without special meaning */
private static final String DEFAULT_ATTACHMENT_TYPE = "event.attachment";

Expand All @@ -36,7 +27,7 @@ public final class Attachment {
* @param filename The name of the attachment to display in Sentry.
*/
public Attachment(final @NotNull byte[] bytes, final @NotNull String filename) {
this(bytes, filename, DEFAULT_CONTENT_TYPE);
this(bytes, filename, null);
}

/**
Expand All @@ -50,7 +41,7 @@ public Attachment(final @NotNull byte[] bytes, final @NotNull String filename) {
public Attachment(
final @NotNull byte[] bytes,
final @NotNull String filename,
final @NotNull String contentType) {
final @Nullable String contentType) {
this(bytes, filename, contentType, false);
}

Expand All @@ -66,7 +57,7 @@ public Attachment(
public Attachment(
final @NotNull byte[] bytes,
final @NotNull String filename,
final @NotNull String contentType,
final @Nullable String contentType,
final boolean addToTransactions) {
this.bytes = bytes;
this.filename = filename;
Expand Down Expand Up @@ -100,7 +91,7 @@ public Attachment(final @NotNull String pathname) {
* @param filename The name of the attachment to display in Sentry.
*/
public Attachment(final @NotNull String pathname, final @NotNull String filename) {
this(pathname, filename, DEFAULT_CONTENT_TYPE);
this(pathname, filename, null);
}

/**
Expand All @@ -118,7 +109,7 @@ public Attachment(final @NotNull String pathname, final @NotNull String filename
public Attachment(
final @NotNull String pathname,
final @NotNull String filename,
final @NotNull String contentType) {
final @Nullable String contentType) {
this(pathname, filename, contentType, false);
}

Expand All @@ -138,7 +129,7 @@ public Attachment(
public Attachment(
final @NotNull String pathname,
final @NotNull String filename,
final @NotNull String contentType,
final @Nullable String contentType,
final boolean addToTransactions) {
this.pathname = pathname;
this.filename = filename;
Expand All @@ -164,7 +155,7 @@ public Attachment(
public Attachment(
final @NotNull String pathname,
final @NotNull String filename,
final @NotNull String contentType,
final @Nullable String contentType,
final boolean addToTransactions,
final @Nullable String attachmentType) {
this.pathname = pathname;
Expand Down Expand Up @@ -202,11 +193,12 @@ public Attachment(
}

/**
* Gets the content type of the attachment. Default is "application/octet-stream".
* Gets the content type of the attachment. The server infers "application/octet-stream" if not
* set.
*
* @return the content type.
* @return the content type or null if not set.
*/
public @NotNull String getContentType() {
public @Nullable String getContentType() {
return contentType;
}

Expand Down
4 changes: 0 additions & 4 deletions sentry/src/test/java/io/sentry/AttachmentTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import kotlin.test.assertTrue
class AttachmentTest {

private class Fixture {
val defaultContentType = "application/octet-stream"
val contentType = "application/json"
val filename = "logs.txt"
val bytes = "content".toByteArray()
Expand All @@ -25,7 +24,6 @@ class AttachmentTest {
assertEquals(fixture.bytes, attachment.bytes)
assertNull(attachment.pathname)
assertEquals(fixture.filename, attachment.filename)
assertEquals(fixture.defaultContentType, attachment.contentType)
}

@Test
Expand All @@ -35,7 +33,6 @@ class AttachmentTest {
assertEquals(fixture.pathname, attachment.pathname)
assertNull(attachment.bytes)
assertEquals(fixture.filename, attachment.filename)
assertEquals(fixture.defaultContentType, attachment.contentType)
}

@Test
Expand All @@ -62,7 +59,6 @@ class AttachmentTest {
assertEquals(fixture.pathname, attachment.pathname)
assertNull(attachment.bytes)
assertEquals(otherFileName, attachment.filename)
assertEquals(fixture.defaultContentType, attachment.contentType)
}

@Test
Expand Down
3 changes: 1 addition & 2 deletions sentry/src/test/java/io/sentry/JsonSerializerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -703,8 +703,7 @@ class JsonSerializerTest {
val actualJson = serializeToString(envelope)

val expectedJson = "{\"event_id\":\"${eventID}\"}\n" +
"{\"content_type\":\"${attachment.contentType}\"," +
"\"filename\":\"${attachment.filename}\"," +
"{\"filename\":\"${attachment.filename}\"," +
"\"type\":\"attachment\"," +
"\"attachment_type\":\"event.attachment\"," +
"\"length\":${attachment.bytes?.size}}\n" +
Expand Down