-
Notifications
You must be signed in to change notification settings - Fork 73
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
Fixes #24723: Tree group is slow to load up because it contains the list of nodes in the tree #5621
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,6 +67,7 @@ import com.normation.rudder.ncf.ResourceFile | |
import com.normation.rudder.ncf.TechniqueParameter | ||
import com.normation.rudder.repository.FullActiveTechnique | ||
import com.normation.rudder.repository.FullActiveTechniqueCategory | ||
import com.normation.rudder.repository.FullNodeGroupCategory | ||
import com.normation.rudder.rule.category.RuleCategory | ||
import com.normation.rudder.rule.category.RuleCategoryId | ||
import com.normation.rudder.services.queries.CmdbQueryParser | ||
|
@@ -76,6 +77,7 @@ import com.normation.utils.DateFormaterService | |
import com.softwaremill.quicklens.* | ||
import com.typesafe.config.ConfigRenderOptions | ||
import com.typesafe.config.ConfigValue | ||
import io.scalaland.chimney.Transformer | ||
import io.scalaland.chimney.dsl.* | ||
import zio.* | ||
import zio.Tag as _ | ||
|
@@ -477,6 +479,21 @@ object JsonResponseObjects { | |
override def toRuleTarget: TargetComposition = TargetUnion(list.map(_.toRuleTarget).toSet) | ||
} | ||
} | ||
|
||
implicit val transformer: Transformer[RuleTarget, JRRuleTarget] = apply _ | ||
} | ||
|
||
final case class JRRuleTargetInfo( | ||
id: JRRuleTarget, | ||
@jsonField("displayName") name: String, | ||
description: String, | ||
@jsonField("enabled") isEnabled: Boolean, | ||
target: JRRuleTarget | ||
) | ||
|
||
object JRRuleTargetInfo { | ||
implicit val transformer: Transformer[RuleTargetInfo, JRRuleTargetInfo] = | ||
Transformer.define[RuleTargetInfo, JRRuleTargetInfo].withFieldRenamed(_.target, _.id).buildTransformer | ||
} | ||
|
||
// CategoryKind is either JRRuleCategory or String (category id) | ||
|
@@ -870,6 +887,66 @@ object JsonResponseObjects { | |
} | ||
} | ||
|
||
/** | ||
* Representation of a group category with bare minimum group information | ||
*/ | ||
final case class JRGroupCategoryInfo( | ||
id: String, | ||
name: String, | ||
description: String, | ||
@jsonField("categories") subCategories: List[JRGroupCategoryInfo], | ||
groups: List[JRGroupCategoryInfo.JRGroupInfo], | ||
@jsonField("targets") targetInfos: List[JRRuleTargetInfo] | ||
) | ||
|
||
object JRGroupCategoryInfo { | ||
final case class JRGroupInfo( | ||
id: NodeGroupId, | ||
@jsonField("displayName") name: String, | ||
description: String, | ||
category: Option[NodeGroupCategoryId], | ||
@jsonField("dynamic") isDynamic: Boolean, | ||
@jsonField("enabled") isEnabled: Boolean, | ||
target: String | ||
) | ||
object JRGroupInfo { | ||
implicit def transformer(implicit categoryId: Option[NodeGroupCategoryId]): Transformer[NodeGroup, JRGroupInfo] = { | ||
Transformer | ||
.define[NodeGroup, JRGroupInfo] | ||
.enableBeanGetters | ||
.withFieldConst(_.category, categoryId) | ||
.withFieldComputed(_.target, x => GroupTarget(x.id).target) | ||
.buildTransformer | ||
} | ||
|
||
} | ||
|
||
implicit lazy val transformer: Transformer[FullNodeGroupCategory, JRGroupCategoryInfo] = { | ||
Transformer | ||
.define[FullNodeGroupCategory, JRGroupCategoryInfo] | ||
.withFieldComputed( | ||
_.subCategories, | ||
_.subCategories.sortBy(_.id.value).transformInto[List[JRGroupCategoryInfo]] | ||
) | ||
.withFieldComputed( | ||
_.groups, | ||
cat => { | ||
cat.ownGroups.values.toList.map(t => { | ||
implicit val categoryId: Option[NodeGroupCategoryId] = cat.categoryByGroupId.get(t.nodeGroup.id) | ||
t.nodeGroup.transformInto[JRGroupInfo] | ||
}) | ||
} | ||
) | ||
.withFieldComputed( | ||
_.targetInfos, | ||
_.targetInfos.collect { | ||
case t @ FullRuleTargetInfo(_: FullOtherTarget, _, _, _, _) => t.toTargetInfo.transformInto[JRRuleTargetInfo] | ||
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.
|
||
} | ||
) | ||
.buildTransformer | ||
} | ||
} | ||
|
||
final case class JRRuleNodesDirectives( | ||
id: String, // id is in format uid+rev | ||
|
||
|
@@ -923,7 +1000,11 @@ trait RudderJsonEncoders { | |
} | ||
} | ||
|
||
implicit val ruleIdEncoder: JsonEncoder[RuleId] = JsonEncoder[String].contramap(_.serialize) | ||
implicit val ruleTargetInfoEncoder: JsonEncoder[JRRuleTargetInfo] = DeriveJsonEncoder.gen[JRRuleTargetInfo] | ||
|
||
implicit val ruleIdEncoder: JsonEncoder[RuleId] = JsonEncoder[String].contramap(_.serialize) | ||
implicit val groupIdEncoder: JsonEncoder[NodeGroupId] = JsonEncoder[String].contramap(_.serialize) | ||
implicit val groupCategoryIdEncoder: JsonEncoder[NodeGroupCategoryId] = JsonEncoder[String].contramap(_.value) | ||
|
||
implicit val applicationStatusEncoder: JsonEncoder[JRApplicationStatus] = DeriveJsonEncoder.gen | ||
|
||
|
@@ -997,6 +1078,10 @@ trait RudderJsonEncoders { | |
implicit val groupEncoder: JsonEncoder[JRGroup] = DeriveJsonEncoder.gen | ||
implicit val objectInheritedObjectProperties: JsonEncoder[JRGroupInheritedProperties] = DeriveJsonEncoder.gen | ||
|
||
implicit val groupInfoEncoder: JsonEncoder[JRGroupCategoryInfo.JRGroupInfo] = | ||
DeriveJsonEncoder.gen[JRGroupCategoryInfo.JRGroupInfo] | ||
implicit lazy val groupCategoryInfoEncoder: JsonEncoder[JRGroupCategoryInfo] = DeriveJsonEncoder.gen[JRGroupCategoryInfo] | ||
|
||
implicit val revisionInfoEncoder: JsonEncoder[JRRevisionInfo] = DeriveJsonEncoder.gen | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -817,6 +817,23 @@ object RuleInternalApi extends Enum[RuleInternalApi] with ApiModuleProvide | |
def values = findValues | ||
} | ||
|
||
sealed trait GroupInternalApi extends EnumEntry with EndpointSchema with InternalApi with SortIndex { | ||
override def dataContainer: Option[String] = Some("groupsinternal") | ||
} | ||
|
||
object GroupInternalApi extends Enum[GroupInternalApi] with ApiModuleProvider[GroupInternalApi] { | ||
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. I think that for API used for a given (set of page / elm app), we should try to get a nomenclature based on the using app nomenclature. 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. (I think it's ok for that one, and since it's an internal API we can change it whenever we want, but it is something to consider cc @RaphaelGauthier @VinceMacBuche ) |
||
final case object GetGroupCategoryTree extends GroupInternalApi with ZeroParam with StartsAtVersion14 with SortIndex { | ||
val z: Int = implicitly[Line].value | ||
val description = "Get the tree of groups with bare minimum group information" | ||
val (action, path) = GET / "groupsinternal" / "categorytree" | ||
override def dataContainer: Option[String] = None | ||
} | ||
|
||
def endpoints: List[GroupInternalApi] = values.toList.sortBy(_.z) | ||
|
||
def values = findValues | ||
} | ||
|
||
sealed trait ScoreApi extends EnumEntry with EndpointSchema with InternalApi with SortIndex { | ||
override def dataContainer: Option[String] = Some("scores") | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
package com.normation.rudder.rest.internal | ||
|
||
import com.normation.errors.IOResult | ||
import com.normation.rudder.api.ApiVersion | ||
import com.normation.rudder.apidata.JsonResponseObjects.* | ||
import com.normation.rudder.apidata.implicits.* | ||
import com.normation.rudder.repository.RoNodeGroupRepository | ||
import com.normation.rudder.rest.ApiModuleProvider | ||
import com.normation.rudder.rest.ApiPath | ||
import com.normation.rudder.rest.AuthzToken | ||
import com.normation.rudder.rest.GroupInternalApi as API | ||
import com.normation.rudder.rest.implicits.* | ||
import com.normation.rudder.rest.lift.* | ||
import io.scalaland.chimney.syntax.* | ||
import net.liftweb.http.LiftResponse | ||
import net.liftweb.http.Req | ||
|
||
class GroupsInternalApi( | ||
groupsInternalApiService: GroupInternalApiService | ||
) extends LiftApiModuleProvider[API] { | ||
|
||
def schemas: ApiModuleProvider[API] = API | ||
|
||
def getLiftEndpoints(): List[LiftApiModule] = { | ||
API.endpoints.map(e => { | ||
e match { | ||
case API.GetGroupCategoryTree => GetGroupCategoryTree | ||
} | ||
}) | ||
} | ||
|
||
object GetGroupCategoryTree extends LiftApiModule0 { | ||
val schema: API.GetGroupCategoryTree.type = API.GetGroupCategoryTree | ||
|
||
def process0(version: ApiVersion, path: ApiPath, req: Req, params: DefaultParams, authzToken: AuthzToken): LiftResponse = { | ||
groupsInternalApiService.getGroupCategoryTree().toLiftResponseOne(params, schema, _ => None) | ||
} | ||
} | ||
|
||
} | ||
|
||
class GroupInternalApiService( | ||
readGroup: RoNodeGroupRepository | ||
) { | ||
def getGroupCategoryTree(): IOResult[JRGroupCategoryInfo] = { | ||
readGroup.getFullGroupLibrary().map(_.transformInto[JRGroupCategoryInfo]) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,175 @@ | ||
--- | ||
description: Get the tree of groups with bare minimum group information | ||
method: GET | ||
url: /secure/api/groupsinternal/categorytree | ||
response: | ||
code: 200 | ||
content: >- | ||
{ | ||
"action" : "getGroupCategoryTree", | ||
"result" : "success", | ||
"data" : { | ||
"id" : "GroupRoot", | ||
"name" : "GroupRoot", | ||
"description" : "root of group categories", | ||
"categories" : [ | ||
{ | ||
"id" : "219b9c98-3d1e-44c9-0001-95b4fc7c4ada", | ||
"name" : "category 2", | ||
"description" : "", | ||
"categories" : [], | ||
"groups" : [], | ||
"targets" : [] | ||
}, | ||
{ | ||
"id" : "219b9c98-3d1e-44c9-aff5-95b4fc7c4ada", | ||
"name" : "category 2", | ||
"description" : "", | ||
"categories" : [], | ||
"groups" : [], | ||
"targets" : [] | ||
}, | ||
{ | ||
"id" : "category1", | ||
"name" : "category 1", | ||
"description" : "the first category", | ||
"categories" : [ | ||
{ | ||
"id" : "219b9c98-3d1e-44c9-0001-95b4fc7c4ada", | ||
"name" : "category 2 update", | ||
"description" : "category 2", | ||
"categories" : [], | ||
"groups" : [], | ||
"targets" : [] | ||
}, | ||
{ | ||
"id" : "219b9c98-3d1e-44c9-aff5-95b4fc7c4ada", | ||
"name" : "category 2 update", | ||
"description" : "category2", | ||
"categories" : [], | ||
"groups" : [], | ||
"targets" : [] | ||
} | ||
], | ||
"groups" : [ | ||
{ | ||
"id" : "0000f5d3-8c61-4d20-88a7-bb947705ba8a", | ||
"displayName" : "Real nodes", | ||
"description" : "", | ||
"category" : "category 1", | ||
"dynamic" : false, | ||
"enabled" : true, | ||
"target" : "group:0000f5d3-8c61-4d20-88a7-bb947705ba8a" | ||
} | ||
], | ||
"targets" : [] | ||
} | ||
], | ||
"groups" : [ | ||
{ | ||
"id" : "00000000-cb9d-4f7b-abda-ca38c5d643ea", | ||
"displayName" : "clone from api of debian group", | ||
"description" : "Some long description", | ||
"category" : "GroupRoot", | ||
"dynamic" : true, | ||
"enabled" : true, | ||
"target" : "group:00000000-cb9d-4f7b-abda-ca38c5d643ea" | ||
}, | ||
{ | ||
"id" : "6666f5d3-8c61-4d20-88a7-bb947705ba8a", | ||
"displayName" : "Nodes id divided by 5", | ||
"description" : "", | ||
"category" : "GroupRoot", | ||
"dynamic" : false, | ||
"enabled" : true, | ||
"target" : "group:6666f5d3-8c61-4d20-88a7-bb947705ba8a" | ||
}, | ||
{ | ||
"id" : "4444f5d3-8c61-4d20-88a7-bb947705ba8a", | ||
"displayName" : "Odd nodes", | ||
"description" : "", | ||
"category" : "GroupRoot", | ||
"dynamic" : false, | ||
"enabled" : true, | ||
"target" : "group:4444f5d3-8c61-4d20-88a7-bb947705ba8a" | ||
}, | ||
{ | ||
"id" : "3333f5d3-8c61-4d20-88a7-bb947705ba8a", | ||
"displayName" : "Even nodes", | ||
"description" : "", | ||
"category" : "GroupRoot", | ||
"dynamic" : false, | ||
"enabled" : true, | ||
"target" : "group:3333f5d3-8c61-4d20-88a7-bb947705ba8a" | ||
}, | ||
{ | ||
"id" : "00000000-cb9d-4f7b-0001-ca38c5d643ea", | ||
"displayName" : "clone from api of debian group", | ||
"description" : "Some long description", | ||
"category" : "GroupRoot", | ||
"dynamic" : true, | ||
"enabled" : true, | ||
"target" : "group:00000000-cb9d-4f7b-0001-ca38c5d643ea" | ||
}, | ||
{ | ||
"id" : "5555f5d3-8c61-4d20-88a7-bb947705ba8a", | ||
"displayName" : "Nodes id divided by 3", | ||
"description" : "", | ||
"category" : "GroupRoot", | ||
"dynamic" : false, | ||
"enabled" : true, | ||
"target" : "group:5555f5d3-8c61-4d20-88a7-bb947705ba8a" | ||
}, | ||
{ | ||
"id" : "a-group-for-root-only", | ||
"displayName" : "Serveurs [€ðŋ] cassés", | ||
"description" : "Liste de l'ensemble de serveurs cassés à réparer", | ||
"category" : "GroupRoot", | ||
"dynamic" : true, | ||
"enabled" : true, | ||
"target" : "group:a-group-for-root-only" | ||
}, | ||
{ | ||
"id" : "2222f5d3-8c61-4d20-88a7-bb947705ba8a", | ||
"displayName" : "only root", | ||
"description" : "", | ||
"category" : "GroupRoot", | ||
"dynamic" : false, | ||
"enabled" : true, | ||
"target" : "group:2222f5d3-8c61-4d20-88a7-bb947705ba8a" | ||
}, | ||
{ | ||
"id" : "1111f5d3-8c61-4d20-88a7-bb947705ba8a", | ||
"displayName" : "Empty group", | ||
"description" : "", | ||
"category" : "GroupRoot", | ||
"dynamic" : false, | ||
"enabled" : true, | ||
"target" : "group:1111f5d3-8c61-4d20-88a7-bb947705ba8a" | ||
} | ||
], | ||
"targets" : [ | ||
{ | ||
"id" : "policyServer:root", | ||
"displayName" : "special:policyServer_root", | ||
"description" : "The root policy server", | ||
"enabled" : true, | ||
"target" : "policyServer:root" | ||
}, | ||
{ | ||
"id" : "special:all_exceptPolicyServers", | ||
"displayName" : "special:all_exceptPolicyServers", | ||
"description" : "All groups without policy servers", | ||
"enabled" : true, | ||
"target" : "special:all_exceptPolicyServers" | ||
}, | ||
{ | ||
"id" : "special:all", | ||
"displayName" : "special:all", | ||
"description" : "All nodes", | ||
"enabled" : true, | ||
"target" : "special:all" | ||
} | ||
] | ||
} | ||
} |
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.
for this one and other: why not using the json name directly ? Is it an choice given other constraints ?
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 imagine with chimney, but if so: is there a reason to chose that way ?
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.
It avoid a boilerplate for the
Transformer
definition in chimney : we can derive the field transformation if the name is the same as the source datatype. If we have thedisplayName
field, we would have to writewithFieldRenamed(_.displayName, _.name)
(it is even more verbose if the transformer could bederive
-d directly)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.
but then, you wrote
@jsonField("displayName")̀
:) OK, there's less chars in that latter option. And perhaps it's a bit clearer.