Ma77Ball commented on code in PR #6116:
URL: https://github.com/apache/texera/pull/6116#discussion_r3607825747
##########
config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala:
##########
@@ -99,4 +117,114 @@ class ConfigResource {
// flags from the user-system.conf
"inviteOnly" -> UserSystemConfig.inviteOnly
)
+
+ // The site_settings keys that non-admin pages consume: dashboard branding,
+ // sidebar tab toggles, and dataset upload limits — exactly the gui.* and
+ // dataset.* sections of default.conf, which is also where the seeding
+ // pipeline gets them. Keys declared outside those sections (e.g.
+ // csv_parser_max_columns) are management-only. Deriving the set from the
+ // file keeps "which section does this default live in" the single place
+ // where visibility is decided.
+ private val publicSettingKeys: Set[String] =
+ DefaultsConfig.keysUnderSections(Set("gui", "dataset"))
+
+ // Read side for the public keys in one payload, so the dashboard doesn't
+ // fire a request per key. Anonymous by design: these values render on the
+ // logged-out shell (custom logo/favicon, Hub/About sidebar entries), so
+ // gating them behind a login would blank the public landing pages.
+ @GET
+ @PermitAll
+ @Path("/settings/public")
+ def getPublicSettings: Map[String, String] = {
+ ctx
+ .select(SITE_SETTINGS.KEY, SITE_SETTINGS.VALUE)
+ .from(SITE_SETTINGS)
+ .where(SITE_SETTINGS.KEY.in(publicSettingKeys.asJava))
+ .fetchMap(SITE_SETTINGS.KEY, SITE_SETTINGS.VALUE)
+ .asScala
+ .toMap
+ }
+
+ // Management read over the site_settings table this service seeds at
+ // startup: every row, including the ones not exposed through
+ // /settings/public, in one payload for the admin settings page.
+ @GET
+ @RolesAllowed(Array("ADMIN"))
+ @Path("/settings")
+ def getAllSettings: Map[String, String] = {
+ ctx
+ .select(SITE_SETTINGS.KEY, SITE_SETTINGS.VALUE)
+ .from(SITE_SETTINGS)
+ .fetchMap(SITE_SETTINGS.KEY, SITE_SETTINGS.VALUE)
+ .asScala
+ .toMap
+ }
+
+ // Single-key management read, kept for API completeness alongside the bulk
+ // read above.
+ @GET
+ @RolesAllowed(Array("ADMIN"))
+ @Path("/settings/{key}")
+ def getSetting(@PathParam("key") keyParam: String): ConfigSettingPojo = {
+ ctx
+ .select(SITE_SETTINGS.KEY, SITE_SETTINGS.VALUE)
+ .from(SITE_SETTINGS)
+ .where(SITE_SETTINGS.KEY.eq(keyParam))
+ .fetchOneInto(classOf[ConfigSettingPojo])
+ }
+
+ @PUT
+ @RolesAllowed(Array("ADMIN"))
+ @Path("/settings/{key}")
+ @Consumes(Array(MediaType.APPLICATION_JSON))
+ def updateSetting(
+ @Auth currentUser: SessionUser,
+ @PathParam("key") keyParam: String,
+ setting: ConfigSettingPojo
+ ): Response = {
+ if (setting.settingValue == null) {
Review Comment:
I recommend checking the request body (`setting` itself) for an empty value.
This will better help guard from malformed request bodies and avoid sending the
wrong error message (a 500 rather than a 400).
```suggestion
if (setting == null || setting.settingValue == null) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
