Skip to content

Commit

Permalink
feat(resources): Add standardized display name for resources (#1456)
Browse files Browse the repository at this point in the history
* feat(resources): Add standardized displayName field for resource specs

* fix(pr): Avoid persisting ResourceSpec.displayName
  • Loading branch information
luispollo committed Aug 24, 2020
1 parent e6e2607 commit 650c543
Show file tree
Hide file tree
Showing 11 changed files with 50 additions and 12 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ out/
*.rdb
/.shelf/
**/*/out/*
/plugins/
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,7 @@ interface Monikered : ResourceSpec {
*/
override val application: String
get() = moniker.app

override val displayName: String
get() = moniker.toString()
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,11 @@ interface ResourceSpec {
* [com.fasterxml.jackson.annotation.JsonIgnore].
*/
val application: String

/**
* A more descriptive name than the [id], intended for displaying in the UI. This property is
* not persisted, as it's expected to be calculated by the [ResourceSpec] implementation from
* other fields.
*/
val displayName: String
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonInclude
import com.fasterxml.jackson.annotation.JsonPropertyOrder
import com.netflix.spinnaker.keel.api.Locations
import com.netflix.spinnaker.keel.api.Moniker
import com.netflix.spinnaker.keel.api.Monikered
import com.netflix.spinnaker.keel.api.Resource
import com.netflix.spinnaker.keel.api.ResourceKind
import com.netflix.spinnaker.keel.api.artifacts.ArtifactType
Expand All @@ -32,17 +33,23 @@ import com.netflix.spinnaker.keel.persistence.ResourceStatus
* This powers the UI view of resource status.
*/
@JsonInclude(JsonInclude.Include.NON_EMPTY)
@JsonPropertyOrder(value = ["id", "kind", "status", "moniker", "locations", "artifact"])
@JsonPropertyOrder(value = ["id", "kind", "status", "moniker", "displayName", "locations", "artifact"])
data class ResourceSummary(
@JsonIgnore
val resource: Resource<*>,
val status: ResourceStatus,
val moniker: Moniker?,
val locations: Locations<*>?,
val artifact: ResourceArtifactSummary? = null
) {
val id: String = resource.id
val kind: ResourceKind = resource.kind
val displayName: String = resource.spec.displayName
val moniker: Moniker?
get() = if (resource.spec is Monikered) {
(resource.spec as Monikered).moniker
} else {
null
}
}

@JsonInclude(JsonInclude.Include.NON_NULL)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import com.netflix.spinnaker.keel.api.Locatable
import com.netflix.spinnaker.keel.api.Monikered
import com.netflix.spinnaker.keel.api.Resource
import com.netflix.spinnaker.keel.api.ResourceKind
import com.netflix.spinnaker.keel.api.ResourceSpec
import com.netflix.spinnaker.keel.api.StaggeredRegion
import com.netflix.spinnaker.keel.api.SubnetAwareRegionSpec
import com.netflix.spinnaker.keel.api.artifacts.DeliveryArtifact
Expand All @@ -39,6 +40,7 @@ import com.netflix.spinnaker.keel.jackson.mixins.LocatableMixin
import com.netflix.spinnaker.keel.jackson.mixins.MonikeredMixin
import com.netflix.spinnaker.keel.jackson.mixins.ResourceKindMixin
import com.netflix.spinnaker.keel.jackson.mixins.ResourceMixin
import com.netflix.spinnaker.keel.jackson.mixins.ResourceSpecMixin
import com.netflix.spinnaker.keel.jackson.mixins.StaggeredRegionMixin
import com.netflix.spinnaker.keel.jackson.mixins.SubnetAwareRegionSpecMixin

Expand All @@ -60,6 +62,7 @@ object KeelApiModule : SimpleModule("Keel API") {
setMixInAnnotations<StaggeredRegion, StaggeredRegionMixin>()
setMixInAnnotations<SubnetAwareRegionSpec, SubnetAwareRegionSpecMixin>()
setMixInAnnotations<Resource<*>, ResourceMixin>()
setMixInAnnotations<ResourceSpec, ResourceSpecMixin>()
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.netflix.spinnaker.keel.jackson.mixins

import com.fasterxml.jackson.annotation.JsonIgnore

interface ResourceSpecMixin {
@get:JsonIgnore
val displayName: String
}
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,6 @@ class ApplicationService(
ResourceSummary(
resource = this,
status = resourceStatusService.getStatus(id),
moniker = if (spec is Monikered) {
(spec as Monikered).moniker
} else {
null
},
locations = if (spec is Locatable<*>) {
(spec as Locatable<*>).locations
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,14 @@ private data class TestSubnetAwareLocatableResource(
override val id: String,
override val application: String,
override val locations: SubnetAwareLocations
) : Locatable<SubnetAwareLocations>
) : Locatable<SubnetAwareLocations> {
override val displayName: String = "$application-$id"
}

private data class TestSimpleLocatableResource(
override val id: String,
override val application: String,
override val locations: SimpleLocations
) : Locatable<SimpleLocations>
) : Locatable<SimpleLocations> {
override val displayName: String = "$application-$id"
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ data class SampleSpecWithContainer(

@JsonIgnore
override val application: String = "myapp"

@JsonIgnore
override val displayName: String = "myapp-sample-resource"
}

val SAMPLE_API_VERSION = ApiVersion("sample.resource")
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ internal class LegacySpecUpgradeTests : JUnit5Minutests {
) : ResourceSpec {
override val id = name
override val application = "fnord"
override val displayName: String = "$application-$name"
}

data class SpecV2(
Expand All @@ -36,6 +37,7 @@ internal class LegacySpecUpgradeTests : JUnit5Minutests {
) : ResourceSpec {
override val id = name
override val application = "fnord"
override val displayName: String = "$application-$name"
}

data class SpecV3(
Expand All @@ -45,6 +47,7 @@ internal class LegacySpecUpgradeTests : JUnit5Minutests {
) : ResourceSpec {
override val id = name
override val application = "fnord"
override val displayName: String = "$application-$name"
}

object Fixture {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ enum class DummyEnum { VALUE }
data class DummyResourceSpec(
override val id: String = randomString(),
val data: String = randomString(),
override val application: String = "fnord"
override val application: String = "fnord",
override val displayName: String = "fnord-dummy"
) : ResourceSpec {
val intData: Int = 1234
val boolData: Boolean = true
Expand All @@ -163,6 +164,7 @@ data class DummyLocatableResourceSpec(
override val id: String = randomString(),
val data: String = randomString(),
override val application: String = "fnord",
override val displayName: String = "fnord-locatable-dummy",
override val locations: SimpleLocations = SimpleLocations(
account = "test",
vpc = "vpc0",
Expand All @@ -177,7 +179,8 @@ data class DummyArtifactVersionedResourceSpec(
override val application: String = "fnord",
override val artifactVersion: String? = "fnord-42.0",
override val artifactName: String? = "fnord",
override val artifactType: ArtifactType? = DEBIAN
override val artifactType: ArtifactType? = DEBIAN,
override val displayName: String = "fnord-artifact-versioned-dummy",
) : ResourceSpec, VersionedArtifactProvider

data class DummyArtifactReferenceResourceSpec(
Expand All @@ -186,7 +189,8 @@ data class DummyArtifactReferenceResourceSpec(
val data: String = randomString(),
override val application: String = "fnord",
override val artifactType: ArtifactType? = DEBIAN,
override val artifactReference: String? = "fnord"
override val artifactReference: String? = "fnord",
override val displayName: String = "fnord-artifact-reference-dummy",
) : ResourceSpec, ArtifactReferenceProvider

data class DummyResource(
Expand Down

0 comments on commit 650c543

Please sign in to comment.