Fixes #26046: Migrate compliance status from lift-json to zio-json #6086

fanf · 2024-12-16T07:04:59Z

https://issues.rudder.io/issues/26046

Main change:

upgrade to chimney 1.6.0 so that we can avoid ...withFieldComputed(_.a, _.aa.transformInto[AA]) and just use .withFieldRenamed(_.a, _.aa) given a transformer A to AA in scope
Tags needed to be migrated. Only encode is needed at that point, which was created with an intermediate JsonTag that is not exposed so that we can keep Tags in Json objects in place of JValue.
where the Json Lift objects were used directly, I mapped to the corresponding zio.json.ast objects
I introduced a OptPosNum type for compliance and compliance percent serialization. It's an Option[Numeric] (so that it works with int and float, but some quirks because js/java/scala) and so can be eliminated in the json when not present.
the json object ComplianceSerializable and ComplianceLevelSerialisation have their fields with the correct naming and order. There's a lot of chimney to map to/from the corresponding business objects.
I did on more time the encoding for AggregatedStatusReport (it already existed in a more compact fashion for database, now with full name - and IIRC, there's some other little differences)

⚠️ For now, we don't migrate RuleLine, because it's an horror mixing json and full js. And perhaps we can just delete it at some point.

I think other things are rather automatics.
Not a lot of net.liftweb.json removed, but still some more, especially in rudder-core.

fanf · 2025-01-08T20:16:25Z

PR updated with a new commit

fanf · 2025-01-08T22:02:10Z

PR updated with a new commit

fanf · 2025-01-08T22:19:16Z

PR updated with a new commit

fanf · 2025-01-08T22:20:36Z

PR updated with a new commit

fanf · 2025-01-08T22:21:24Z

.../rudder/rudder-core/src/main/scala/com/normation/rudder/domain/reports/ComplianceLevel.scala

-    noReport:                   Option[Double],
-    reportsDisabled:            Option[Double],
-    badPolicyMode:              Option[Double]
-)


moved in the end of the file, with other serialization things

fanf · 2025-01-08T22:39:02Z

PR updated with a new commit

fanf · 2025-01-08T22:39:26Z

PR updated with a new commit

clarktsiory

LGTM, just some suggestions/questions

clarktsiory · 2025-01-09T13:25:10Z

...es/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/reports/StatusReports.scala

+      .withFieldComputed(_.run, _._1.transformInto[ApiRunInfo])
+      .withFieldComputed(_.status, _._2.transformInto[ApiStatusInfo])


So now with the last chimney version, it could be :

Suggested change

.withFieldComputed(_.run, _._1.transformInto[ApiRunInfo])

.withFieldComputed(_.status, _._2.transformInto[ApiStatusInfo])

.withFieldRenamed(_._1, _.run)

.withFieldRenamed(_._1, _.status)

yep,it works now (with a 2 in second line)

clarktsiory · 2025-01-09T15:44:47Z

...es/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/reports/StatusReports.scala

+  // here, I'm not sure that we want compliance or
+  // compliance percents. Having a normalized value
+  // seems far better for queries in the future.
+  // but in that case, we should also keep the total
+  // number of events to be able to rebuild raw data
+
+  // always map compliance field from ComplianceLevel to ComplianceSerializable automatically
+  implicit private lazy val transformComplianceLevel: Transformer[ComplianceLevel, ComplianceSerializable] = {
+    case c: ComplianceLevel => c.computePercent().transformInto[ComplianceSerializable]
+  }


that seems like a transformation that should be reusable, is there a reason why it is here instead of in the ComplianceLevel companion object ?
In fact there already is a transformComplianceSerializable: Transformer[ComplianceLevel, ComplianceLevelSerialisation], I am not sure about the difference between ComplianceSerialization and ComplianceLevelSerialisation...

the existing one is for thing of the same type: ComplianceLevel is the number of things in each kind, and ComplianceLevelSerialisation the corresponding json.
This one is changing the type from level to percent. It's a local tool because almost all reports have the percent, so just removing boilerplate in the other transformers.

clarktsiory · 2025-01-09T16:01:13Z

...es/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/reports/StatusReports.scala

+/*
+ * This is the serialization for API.
+ * We are again redefining roughly the same objects.
+ */
+// not sure why it doesn't see that they are used
+@nowarn("msg=private val .* in object NodeStatusReportSerialization is never used")


I prefer the solution of declaring everything in a private object InternalDatastructures, this avoids the issue of private implicits being reported as unused by the compiler.
Plus, in all the code below, there are really many things that end up being private, the structure and the dependencies are becoming hard to follow...

clarktsiory · 2025-01-09T16:02:55Z

...r-core/src/main/scala/com/normation/rudder/services/quicksearch/QuickSearchBackendImpl.scala

@@ -49,6 +49,7 @@ import com.normation.rudder.domain.RudderDit
 import com.normation.rudder.domain.RudderLDAPConstants.*
 import com.normation.rudder.domain.policies.Tag
 import com.normation.rudder.domain.policies.TagName
+import com.normation.rudder.domain.policies.Tags as TAGS


BECAUSE
(there is a name clash, and this one is used only one time)

.../rudder/rudder-core/src/main/scala/com/normation/rudder/domain/reports/ComplianceLevel.scala

Normation-Quality-Assistant · 2025-01-10T10:32:45Z

OK, merging this PR

fanf requested a review from clarktsiory December 16, 2024 07:05

fanf marked this pull request as draft December 16, 2024 07:05

fanf force-pushed the arch_26046/migrate_compliance_status_from_lift_json_to_zio_json branch from 485f1d2 to 94f34fa Compare December 16, 2024 21:54

fanf force-pushed the arch_26046/migrate_compliance_status_from_lift_json_to_zio_json branch from 94f34fa to f097743 Compare January 8, 2025 20:15

fanf marked this pull request as ready for review January 8, 2025 22:12

fanf force-pushed the arch_26046/migrate_compliance_status_from_lift_json_to_zio_json branch from 9949e4e to e922c62 Compare January 8, 2025 22:13

fanf commented Jan 8, 2025

View reviewed changes

fanf force-pushed the arch_26046/migrate_compliance_status_from_lift_json_to_zio_json branch from 5271ae6 to 8124190 Compare January 8, 2025 22:39

clarktsiory reviewed Jan 9, 2025

View reviewed changes

clarktsiory requested changes Jan 9, 2025

View reviewed changes

.../rudder/rudder-core/src/main/scala/com/normation/rudder/domain/reports/ComplianceLevel.scala Outdated Show resolved Hide resolved

Fixes #26046: Migrate compliance status from lift-json to zio-json

8b1aab6

fanf force-pushed the arch_26046/migrate_compliance_status_from_lift_json_to_zio_json branch from 8124190 to 8b1aab6 Compare January 9, 2025 21:14

fanf requested a review from clarktsiory January 9, 2025 21:15

clarktsiory approved these changes Jan 10, 2025

View reviewed changes

Normation-Quality-Assistant merged commit 6cb85f1 into Normation:master Jan 10, 2025
19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #26046: Migrate compliance status from lift-json to zio-json #6086

Fixes #26046: Migrate compliance status from lift-json to zio-json #6086

fanf commented Dec 16, 2024 •

edited

Loading

fanf commented Jan 8, 2025

fanf commented Jan 8, 2025

fanf commented Jan 8, 2025

fanf commented Jan 8, 2025

fanf Jan 8, 2025

fanf commented Jan 8, 2025

fanf commented Jan 8, 2025

clarktsiory left a comment

clarktsiory Jan 9, 2025

fanf Jan 9, 2025

clarktsiory Jan 9, 2025

fanf Jan 9, 2025

clarktsiory Jan 9, 2025

fanf Jan 9, 2025

clarktsiory Jan 9, 2025

fanf Jan 9, 2025

Normation-Quality-Assistant commented Jan 10, 2025

		.withFieldComputed(_.run, _._1.transformInto[ApiRunInfo])
		.withFieldComputed(_.status, _._2.transformInto[ApiStatusInfo])

Fixes #26046: Migrate compliance status from lift-json to zio-json #6086

Fixes #26046: Migrate compliance status from lift-json to zio-json #6086

Conversation

fanf commented Dec 16, 2024 • edited Loading

fanf commented Jan 8, 2025

fanf commented Jan 8, 2025

fanf commented Jan 8, 2025

fanf commented Jan 8, 2025

Choose a reason for hiding this comment

fanf commented Jan 8, 2025

fanf commented Jan 8, 2025

clarktsiory left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Normation-Quality-Assistant commented Jan 10, 2025

fanf commented Dec 16, 2024 •

edited

Loading