Skip to content

Commit

Permalink
Fix SimpleMailMessage nullability annotations
Browse files Browse the repository at this point in the history
Fixes the nullability annotations in `SimpleMailMessage` so the
`set*(...)` methods have their parameters marked as `@Nullable`.

These method parameters had no annotations and when `SimpleMailMessage`
is used from Kotlin with `-Xjsr305=strict` enabled, the `set*(...)`
methods were treated as being marked `@NonNull`. This also meant Kotlin
would not see matched pairs of get/set methods as `var` properties, so
calling `message.text = "Text"` would not compile in Kotlin.

Adds tests in `SimpleMailMessageJsr305ComplianceTests.kt` that verify
the nullability annotations for `SimpleMailMessage` are correct. This
file will actually fail to compile if the nullability annotations are
missing, causing the tests to fail.

Also modifies `spring-context-support.gradle` to force the Kotlin test
code to compile with `-Xjsr305=strict`, enabling the test code to work
properly.
  • Loading branch information
stgerhardt committed Sep 13, 2022
1 parent b0ee513 commit eb1a5d3
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 10 deletions.
11 changes: 11 additions & 0 deletions spring-context-support/spring-context-support.gradle
@@ -1,5 +1,7 @@
description = "Spring Context Support"

apply plugin: "kotlin"

dependencies {
api(project(":spring-beans"))
api(project(":spring-context"))
Expand All @@ -12,6 +14,8 @@ dependencies {
optional("com.github.ben-manes.caffeine:caffeine")
optional("org.quartz-scheduler:quartz")
optional("org.freemarker:freemarker")
optional("org.jetbrains.kotlin:kotlin-reflect")
optional("org.jetbrains.kotlin:kotlin-stdlib")
testImplementation(project(":spring-context"))
testImplementation(testFixtures(project(":spring-beans")))
testImplementation(testFixtures(project(":spring-context")))
Expand All @@ -27,3 +31,10 @@ dependencies {
testFixturesImplementation("org.assertj:assertj-core")
testFixturesImplementation("org.mockito:mockito-core")
}

// Compile the Kotlin test code differently from the rest - it needs
// -Xjsr305=strict to ensure that parameter and return types are
// correctly annotated from Kotlin's perspective.
compileTestKotlin {
kotlinOptions.freeCompilerArgs += "-Xjsr305=strict"
}
Expand Up @@ -93,7 +93,7 @@ public SimpleMailMessage(SimpleMailMessage original) {


@Override
public void setFrom(String from) {
public void setFrom(@Nullable String from) {
this.from = from;
}

Expand All @@ -103,7 +103,7 @@ public String getFrom() {
}

@Override
public void setReplyTo(String replyTo) {
public void setReplyTo(@Nullable String replyTo) {
this.replyTo = replyTo;
}

Expand All @@ -113,7 +113,7 @@ public String getReplyTo() {
}

@Override
public void setTo(String to) {
public void setTo(@Nullable String to) {
this.to = new String[] {to};
}

Expand All @@ -128,12 +128,12 @@ public String[] getTo() {
}

@Override
public void setCc(String cc) {
public void setCc(@Nullable String cc) {
this.cc = new String[] {cc};
}

@Override
public void setCc(String... cc) {
public void setCc(@Nullable String... cc) {
this.cc = cc;
}

Expand All @@ -143,12 +143,12 @@ public String[] getCc() {
}

@Override
public void setBcc(String bcc) {
public void setBcc(@Nullable String bcc) {
this.bcc = new String[] {bcc};
}

@Override
public void setBcc(String... bcc) {
public void setBcc(@Nullable String... bcc) {
this.bcc = bcc;
}

Expand All @@ -158,7 +158,7 @@ public String[] getBcc() {
}

@Override
public void setSentDate(Date sentDate) {
public void setSentDate(@Nullable Date sentDate) {
this.sentDate = sentDate;
}

Expand All @@ -168,7 +168,7 @@ public Date getSentDate() {
}

@Override
public void setSubject(String subject) {
public void setSubject(@Nullable String subject) {
this.subject = subject;
}

Expand All @@ -178,7 +178,7 @@ public String getSubject() {
}

@Override
public void setText(String text) {
public void setText(@Nullable String text) {
this.text = text;
}

Expand Down Expand Up @@ -278,4 +278,8 @@ private static String[] copy(String[] state) {
return state.clone();
}

public void testFunction(String parameter) {
System.out.println(parameter);
}

}
@@ -0,0 +1,135 @@
package org.springframework.mail/*
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import org.junit.jupiter.api.Test

import java.util.*
import kotlin.reflect.KParameter
import kotlin.reflect.full.declaredMemberFunctions

/**
* These tests are intended to verify correct behavior of SimpleMailMessage when used from
* Kotlin with `-Xjsr305=strict` enabled to strictly enforce Java nullability annotations.
*
* Kotlin's JSR305 strict mode treats all non-annotated parameters and return types as
* non-nullable, while Java code does not perform any such checks and can still pass and
* capture null parameters and return values, respectively.
*
* Kotlin also treats null and definitely-non-null types as incompatible types. As such,
* The get/set methods in [SimpleMailMessage] must therefore have coherent nullability
* annotations for Kotlin to treat them as properties with getters and setters. Otherwise,
* Kotlin sees them as functions with incompatible get/set types and does not associate
* them.
*
* These errors often appear only at compile time and not in the IDE, making it somewhat
* confusing to troubleshoot.
*
* __If any of the annotations on [SimpleMailMessage] are missing or incoherent with their
* respective getter / setter method, this test will actually fail to compile, which is
* a form of test failure in and of itself.__
*
* @author Steve Gerhardt
*/
class SimpleMailMessageJsr305ComplianceTests {

private inline fun <reified T : Any> T.getUnsafeVarargSetterByReflection(name : String) : Function1<Array<Any?>?, Unit> {
return { arrayForVararg ->
this::class.declaredMemberFunctions
.first {
val isFirstValueParameterVararg = it.parameters
.firstOrNull { p -> p.kind == KParameter.Kind.VALUE }
?.isVararg == true

it.name == name && isFirstValueParameterVararg
}
.call(this, arrayForVararg)
}
}

@Test
fun `Null message parameters via Java setters should be null via Kotlin getters`() {
val message = SimpleMailMessage()
message.setFrom(null)
message.getUnsafeVarargSetterByReflection("setTo")(null)
message.setReplyTo(null)
message.getUnsafeVarargSetterByReflection("setCc")(null)
message.getUnsafeVarargSetterByReflection("setBcc")(null)
message.setSentDate(null)
message.setSubject(null)
message.setText(null)

assert(message.from == null)
assert(message.to == null)
assert(message.replyTo == null)
assert(message.cc == null)
assert(message.bcc == null)
assert(message.sentDate == null)
assert(message.subject == null)
assert(message.text == null)
}

@Test
fun `To, CC, and BCC with lists of null recipients should appear as valid lists with null entries via Kotlin getters`() {
val message = SimpleMailMessage()
message.setTo(*arrayOf(null, null, null, null))
message.setCc(*arrayOf(null, null, null))
message.setBcc(*arrayOf(null, null))

assert(message.to.let { it != null && it.size == 4 && it.all { item -> item == null } })
assert(message.cc.let { it != null && it.size == 3 && it.all { item -> item == null } })
assert(message.bcc.let { it != null && it.size == 2 && it.all { item -> item == null } })
}

@Test
fun `Non-null message parameters via Java setters should be non-null via Kotlin getters`() {
val message = SimpleMailMessage()
message.setFrom("me@mail.org")
message.setTo("you@mail.org")
message.setReplyTo("reply@mail.org")
message.setCc(*arrayOf("he@mail.org", "she@mail.org"))
message.setBcc(*arrayOf("us@mail.org", "them@mail.org"))
val sentDate = Date()
message.setSentDate(sentDate)
message.setSubject("my subject")
message.setText("my text")

assert(message.from == "me@mail.org")
assert(message.to.let { it != null && it.contains("you@mail.org") })
assert(message.replyTo == "reply@mail.org")
assert(message.cc.let { it != null && it.toList() == listOf("he@mail.org", "she@mail.org") })
assert(message.bcc.let { it != null && it.toList() == listOf("us@mail.org", "them@mail.org") })
assert(message.sentDate == sentDate)
assert(message.subject == "my subject")
assert(message.text == "my text")
}

@Test
fun `Message parameters can be set via Kotlin setters`() {
val message = SimpleMailMessage()
message.from = "me@mail.org"
message.replyTo = "reply@mail.org"
val sentDate = Date()
message.sentDate = sentDate
message.subject = "my subject"
message.text = "my text"

assert(message.from == "me@mail.org")
assert(message.replyTo == "reply@mail.org")
assert(message.sentDate == sentDate)
assert(message.subject == "my subject")
assert(message.text == "my text")
}
}

0 comments on commit eb1a5d3

Please sign in to comment.