Skip to content

Commit

Permalink
Checking for unit test failure
Browse files Browse the repository at this point in the history
Signed-off-by: @akbhatta
  • Loading branch information
akbhatta committed Oct 20, 2021
1 parent eb05f58 commit 82707ff
Show file tree
Hide file tree
Showing 11 changed files with 97 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@ opensearch.notifications:
general:
operationTimeoutMs: 60000 # 60 seconds, Minimum 100ms
defaultItemsQueryCount: 100 # default number of items to query
filterSendByBackendRoles: false # Does sendNotification needs to validate user's backend roles
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import org.opensearch.common.xcontent.ToXContent
import org.opensearch.common.xcontent.XContentBuilder
import org.opensearch.common.xcontent.XContentParser
import org.opensearch.common.xcontent.XContentParserUtils
import org.opensearch.commons.notifications.NotificationConstants.CREATED_TIME_TAG
import org.opensearch.commons.notifications.NotificationConstants.UPDATED_TIME_TAG
import org.opensearch.commons.utils.logger
import org.opensearch.commons.utils.stringList
Expand All @@ -47,7 +48,6 @@ data class DocMetadata(
companion object {
private val log by logger(DocMetadata::class.java)
const val METADATA_TAG = "metadata"
const val CREATED_TIME_TAG = "created_time_ms"
const val ACCESS_LIST_TAG = "access"

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import org.opensearch.notifications.model.DocMetadata.Companion.METADATA_TAG
import java.io.IOException

/**
* Data class representing Notification config.
* Data class representing Notification config with metadata.
*/
data class NotificationConfigDoc(
val metadata: DocMetadata,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ data class NotificationEventDoc(
@Throws(IOException::class)
fun parse(parser: XContentParser): NotificationEventDoc {
var metadata: DocMetadata? = null
var config: NotificationEvent? = null
var event: NotificationEvent? = null

XContentParserUtils.ensureExpectedToken(
XContentParser.Token.START_OBJECT,
Expand All @@ -69,18 +69,18 @@ data class NotificationEventDoc(
parser.nextToken()
when (fieldName) {
METADATA_TAG -> metadata = DocMetadata.parse(parser)
EVENT_TAG -> config = NotificationEvent.parse(parser)
EVENT_TAG -> event = NotificationEvent.parse(parser)
else -> {
parser.skipChildren()
log.info("Unexpected field: $fieldName, while parsing event doc")
}
}
}
metadata ?: throw IllegalArgumentException("$METADATA_TAG field absent")
config ?: throw IllegalArgumentException("$EVENT_TAG field absent")
event ?: throw IllegalArgumentException("$EVENT_TAG field absent")
return NotificationEventDoc(
metadata,
config
event
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,9 @@ interface UserAccess {
* validate if user has access based on given access list
*/
fun doesUserHasAccess(user: User?, access: List<String>): Boolean

/**
* validate if user has send-notification access based on given access list
*/
fun doesUserHasSendAccess(user: User?, access: List<String>): Boolean
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,11 @@ internal object UserAccessManager : UserAccess {
}
return user.backendRoles.any { it in access }
}

/**
* {@inheritDoc}
*/
override fun doesUserHasSendAccess(user: User?, access: List<String>): Boolean {
return !PluginSettings.filterSendByBackendRoles || doesUserHasAccess(user, access)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ object SendMessageActionHelper {
listOf(),
DeliveryStatus(RestStatus.NOT_FOUND.status.toString(), "Channel ${channelEntry.key} not found")
)
} else if (!userAccess.doesUserHasAccess(user, channelEntry.value!!.configDoc.metadata.access)) {
} else if (!userAccess.doesUserHasSendAccess(user, channelEntry.value!!.configDoc.metadata.access)) {
Metrics.NOTIFICATIONS_PERMISSION_USER_ERROR.counter.increment()
return EventStatus(
channelEntry.key,
Expand Down Expand Up @@ -418,7 +418,7 @@ object SendMessageActionHelper {
"Sender ${email.emailAccountID} not found"
)
)
} else if (!userAccess.doesUserHasAccess(user, accountDocInfo.configDoc.metadata.access)) {
} else if (!userAccess.doesUserHasSendAccess(user, accountDocInfo.configDoc.metadata.access)) {
Metrics.NOTIFICATIONS_PERMISSION_USER_ERROR.counter.increment()
return eventStatus.copy(
emailRecipientStatus = listOf(),
Expand All @@ -429,7 +429,7 @@ object SendMessageActionHelper {
)
}
val accessDeniedGroupIds = childConfigMap.filterValues {
it != null && !userAccess.doesUserHasAccess(user, it.configDoc.metadata.access)
it != null && !userAccess.doesUserHasSendAccess(user, it.configDoc.metadata.access)
}.keys
val invalidGroupIds = childConfigMap.filterValues { it == null }.keys
val groups = childConfigMap.values.filterNotNull()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ internal object PluginSettings {
*/
private const val DEFAULT_ITEMS_QUERY_COUNT_KEY = "$GENERAL_KEY_PREFIX.defaultItemsQueryCount"

/**
* Setting to choose filter send by backend role.
*/
private const val FILTER_SEND_BY_BACKEND_ROLES_KEY = "$GENERAL_KEY_PREFIX.filterSendByBackendRoles"

/**
* Legacy alerting plugin filter_by_backend_roles setting.
*/
Expand Down Expand Up @@ -110,6 +115,12 @@ internal object PluginSettings {
@Volatile
var defaultItemsQueryCount: Int

/**
* Filter send by backend role.
*/
@Volatile
var filterSendByBackendRoles: Boolean

private const val DECIMAL_RADIX: Int = 10

private val log by logger(javaClass)
Expand All @@ -130,9 +141,12 @@ internal object PluginSettings {
operationTimeoutMs = (settings?.get(OPERATION_TIMEOUT_MS_KEY)?.toLong()) ?: DEFAULT_OPERATION_TIMEOUT_MS
defaultItemsQueryCount = (settings?.get(DEFAULT_ITEMS_QUERY_COUNT_KEY)?.toInt())
?: DEFAULT_ITEMS_QUERY_COUNT_VALUE
filterSendByBackendRoles = (settings?.get(FILTER_SEND_BY_BACKEND_ROLES_KEY)?.toBoolean())
?: false
defaultSettings = mapOf(
OPERATION_TIMEOUT_MS_KEY to operationTimeoutMs.toString(DECIMAL_RADIX),
DEFAULT_ITEMS_QUERY_COUNT_KEY to defaultItemsQueryCount.toString(DECIMAL_RADIX)
DEFAULT_ITEMS_QUERY_COUNT_KEY to defaultItemsQueryCount.toString(DECIMAL_RADIX),
FILTER_SEND_BY_BACKEND_ROLES_KEY to filterSendByBackendRoles.toString()
)
}

Expand All @@ -150,6 +164,12 @@ internal object PluginSettings {
NodeScope, Dynamic
)

val FILTER_SEND_BY_BACKEND_ROLES: Setting<Boolean> = Setting.boolSetting(
FILTER_SEND_BY_BACKEND_ROLES_KEY,
defaultSettings[FILTER_SEND_BY_BACKEND_ROLES_KEY]!!.toBoolean(),
NodeScope, Dynamic
)

private val LEGACY_FILTER_BY_BACKEND_ROLES: Setting<Boolean> = Setting.boolSetting(
LEGACY_FILTER_BY_BACKEND_ROLES_KEY,
false,
Expand Down Expand Up @@ -178,7 +198,8 @@ internal object PluginSettings {
fun getAllSettings(): List<Setting<*>> {
return listOf(
OPERATION_TIMEOUT_MS,
DEFAULT_ITEMS_QUERY_COUNT
DEFAULT_ITEMS_QUERY_COUNT,
FILTER_SEND_BY_BACKEND_ROLES
)
}

Expand All @@ -189,6 +210,7 @@ internal object PluginSettings {
private fun updateSettingValuesFromLocal(clusterService: ClusterService) {
operationTimeoutMs = OPERATION_TIMEOUT_MS.get(clusterService.settings)
defaultItemsQueryCount = DEFAULT_ITEMS_QUERY_COUNT.get(clusterService.settings)
filterSendByBackendRoles = FILTER_SEND_BY_BACKEND_ROLES.get(clusterService.settings)
}

/**
Expand All @@ -207,6 +229,11 @@ internal object PluginSettings {
log.debug("$LOG_PREFIX:$DEFAULT_ITEMS_QUERY_COUNT_KEY -autoUpdatedTo-> $clusterDefaultItemsQueryCount")
defaultItemsQueryCount = clusterDefaultItemsQueryCount
}
val clusterFilterSendByBackendRole = clusterService.clusterSettings.get(FILTER_SEND_BY_BACKEND_ROLES)
if (clusterFilterSendByBackendRole != null) {
log.debug("$LOG_PREFIX:$FILTER_SEND_BY_BACKEND_ROLES_KEY -autoUpdatedTo-> $clusterFilterSendByBackendRole")
filterSendByBackendRoles = clusterFilterSendByBackendRole
}
}

/**
Expand All @@ -228,12 +255,17 @@ internal object PluginSettings {
defaultItemsQueryCount = it
log.info("$LOG_PREFIX:$DEFAULT_ITEMS_QUERY_COUNT_KEY -updatedTo-> $it")
}
clusterService.clusterSettings.addSettingsUpdateConsumer(FILTER_SEND_BY_BACKEND_ROLES) {
filterSendByBackendRoles = it
log.info("$LOG_PREFIX:$FILTER_SEND_BY_BACKEND_ROLES_KEY -updatedTo-> $it")
}
}

// reset the settings values to default values for testing purpose
@OpenForTesting
fun reset() {
operationTimeoutMs = DEFAULT_OPERATION_TIMEOUT_MS
defaultItemsQueryCount = DEFAULT_ITEMS_QUERY_COUNT_VALUE
filterSendByBackendRoles = false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import io.mockk.mockkObject
import org.junit.jupiter.api.Assertions
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.mockito.Mockito.anyLong
import org.mockito.Mockito.mock
Expand Down Expand Up @@ -49,22 +48,16 @@ import java.time.Instant

internal class NotificationEventIndexTests {

private lateinit var client: Client
private val client: Client = mock(Client::class.java, "client")

private val indexName = ".opensearch-notifications-event"

private lateinit var clusterService: ClusterService

@BeforeEach
fun setUp() {
client = mock(Client::class.java, "client")
clusterService = mock(ClusterService::class.java, "clusterService")
NotificationEventIndex.initialize(client, clusterService)
}
private val clusterService: ClusterService = mock(ClusterService::class.java, "clusterService")

@Suppress("UNCHECKED_CAST")
@Test
fun `index operation to get single event`() {
NotificationEventIndex.initialize(client, clusterService)
// creating expected value
val id = "index-1"
val docInfo = DocInfo("index-1", 1, 1, 1)
Expand Down Expand Up @@ -130,6 +123,7 @@ internal class NotificationEventIndexTests {
@Suppress("UNCHECKED_CAST")
@Test
fun `NotificationEventIndex should safely return null if response source is null`() {
NotificationEventIndex.initialize(client, clusterService)
val id = "index-1"
// mocking the dependencies for isIndexExists function
mockIsIndexExists()
Expand Down Expand Up @@ -159,6 +153,7 @@ internal class NotificationEventIndexTests {
@Suppress("UNCHECKED_CAST")
@Test
fun `NotificationEventIndex should throw exception if response isn't acknowledged`() {
NotificationEventIndex.initialize(client, clusterService)
val id = "index-1"
// mocking the dependencies for isIndexExists function
mockIsIndexExists()
Expand All @@ -174,6 +169,7 @@ internal class NotificationEventIndexTests {
@Suppress("UNCHECKED_CAST")
@Test
fun `NotificationEventIndex should throw exception if index couldn't be created`() {
NotificationEventIndex.initialize(client, clusterService)
val id = "index-1"
// mocking the dependencies for isIndexExists function
mockIsIndexExists()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ internal class NotificationEventDocTests {
@Test
fun `Event doc serialize and deserialize using json config object should be equal`() {
val lastUpdatedTimeMs = Instant.ofEpochMilli(Instant.now().toEpochMilli())
val createdTimeMs = lastUpdatedTimeMs.minusSeconds(1000)
val createdTimeMs = Instant.ofEpochMilli(Instant.now().minusSeconds(2000).toEpochMilli())
val metadata = DocMetadata(
lastUpdatedTimeMs,
createdTimeMs,
Expand Down Expand Up @@ -74,7 +74,7 @@ internal class NotificationEventDocTests {
@Test
fun `Event doc should safely ignore extra field in json object`() {
val lastUpdatedTimeMs = Instant.ofEpochMilli(Instant.now().toEpochMilli())
val createdTimeMs = lastUpdatedTimeMs.minusSeconds(1000)
val createdTimeMs = Instant.ofEpochMilli(Instant.now().minusSeconds(2000).toEpochMilli())
val metadata = DocMetadata(
lastUpdatedTimeMs,
createdTimeMs,
Expand Down Expand Up @@ -167,7 +167,7 @@ internal class NotificationEventDocTests {
@Test
fun `Event doc should throw exception if event is absent in json`() {
val lastUpdatedTimeMs = Instant.ofEpochMilli(Instant.now().toEpochMilli())
val createdTimeMs = lastUpdatedTimeMs.minusSeconds(1000)
val createdTimeMs = Instant.ofEpochMilli(Instant.now().minusSeconds(2000).toEpochMilli())
val jsonString = """
{
"metadata":{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@ internal class PluginSettingsTests {
private val generalKeyPrefix = "$keyPrefix.general"
private val operationTimeoutKey = "$generalKeyPrefix.operationTimeoutMs"
private val defaultItemQueryCountKey = "$generalKeyPrefix.defaultItemsQueryCount"
private val filterSendByBackendRolesKey = "$generalKeyPrefix.filterSendByBackendRoles"

private val defaultSettings = Settings.builder()
.put(operationTimeoutKey, 60000L)
.put(defaultItemQueryCountKey, 100L)
.put(filterSendByBackendRolesKey, false)
.build()

@BeforeEach
Expand All @@ -54,28 +56,41 @@ internal class PluginSettingsTests {

Assert.assertTrue(
settings.containsAll(
listOf<Any> (
listOf<Any>(
PluginSettings.OPERATION_TIMEOUT_MS,
PluginSettings.DEFAULT_ITEMS_QUERY_COUNT
PluginSettings.DEFAULT_ITEMS_QUERY_COUNT,
PluginSettings.FILTER_SEND_BY_BACKEND_ROLES
)
)
)
Assertions.assertEquals(defaultSettings[operationTimeoutKey], PluginSettings.operationTimeoutMs.toString())
Assertions.assertEquals(defaultSettings[defaultItemQueryCountKey], PluginSettings.defaultItemsQueryCount.toString())
Assertions.assertEquals(
defaultSettings[defaultItemQueryCountKey],
PluginSettings.defaultItemsQueryCount.toString()
)
Assertions.assertEquals(
defaultSettings[filterSendByBackendRolesKey],
PluginSettings.filterSendByBackendRoles.toString()
)
}

@Test
fun `test update settings should take cluster settings if available`() {
val clusterSettings = Settings.builder()
.put(operationTimeoutKey, 50000L)
.put(defaultItemQueryCountKey, 200)
.put(filterSendByBackendRolesKey, true)
.build()

whenever(clusterService.settings).thenReturn(defaultSettings)
whenever(clusterService.clusterSettings).thenReturn(
ClusterSettings(
clusterSettings,
setOf(PluginSettings.OPERATION_TIMEOUT_MS, PluginSettings.DEFAULT_ITEMS_QUERY_COUNT)
setOf(
PluginSettings.OPERATION_TIMEOUT_MS,
PluginSettings.DEFAULT_ITEMS_QUERY_COUNT,
PluginSettings.FILTER_SEND_BY_BACKEND_ROLES
)
)
)
PluginSettings.addSettingsUpdateConsumer(clusterService)
Expand All @@ -87,6 +102,10 @@ internal class PluginSettingsTests {
200,
clusterService.clusterSettings.get(PluginSettings.DEFAULT_ITEMS_QUERY_COUNT)
)
Assertions.assertEquals(
true,
clusterService.clusterSettings.get(PluginSettings.FILTER_SEND_BY_BACKEND_ROLES)
)
}

@Test
Expand All @@ -96,7 +115,11 @@ internal class PluginSettingsTests {
whenever(clusterService.clusterSettings).thenReturn(
ClusterSettings(
clusterSettings,
setOf(PluginSettings.OPERATION_TIMEOUT_MS, PluginSettings.DEFAULT_ITEMS_QUERY_COUNT)
setOf(
PluginSettings.OPERATION_TIMEOUT_MS,
PluginSettings.DEFAULT_ITEMS_QUERY_COUNT,
PluginSettings.FILTER_SEND_BY_BACKEND_ROLES
)
)
)
PluginSettings.addSettingsUpdateConsumer(clusterService)
Expand All @@ -108,5 +131,9 @@ internal class PluginSettingsTests {
defaultSettings[defaultItemQueryCountKey],
clusterService.clusterSettings.get(PluginSettings.DEFAULT_ITEMS_QUERY_COUNT).toString()
)
Assertions.assertEquals(
defaultSettings[filterSendByBackendRolesKey],
clusterService.clusterSettings.get(PluginSettings.FILTER_SEND_BY_BACKEND_ROLES).toString()
)
}
}

0 comments on commit 82707ff

Please sign in to comment.