From 3fbdf98c9a5553bfb5dba18f12c079b86ae65a10 Mon Sep 17 00:00:00 2001
From: David Pursehouse <dpursehouse@collab.net>
Date: Thu, 7 Feb 2019 10:46:20 +0900
Subject: [PATCH] Remove remnants of project-specific themes support

Project-specific themes were only implemented in the GWT UI, which has
been removed.

Remove the ThemeInfo entity, and no longer include the 'theme' field in
the ConfigInfo entity returned by the project config REST API endpoint.
This is a breaking change for any client that still uses that endpoint.

The custom site header HTML, footer HTML, and CSS files are still used
by login screens and the internally managed Gitweb server. Update the
theme documentation to mention this.

PolyGerrit has site-wide theme support, but it is not documented in the
existing config-themes.txt, and adding it is not within the scope of
this change.

Change-Id: Ibe824a9416fac65bd365407b8ce22e03515b5b52
---
 Documentation/config-themes.txt               | 20 +++----
 Documentation/intro-project-owner.txt         | 12 ----
 Documentation/rest-api-projects.txt           | 18 ------
 .../extensions/api/projects/ConfigInfo.java   |  1 -
 .../extensions/api/projects/ThemeInfo.java    | 29 ----------
 .../gerrit/server/config/SitePaths.java       |  5 +-
 .../gerrit/server/project/ProjectState.java   | 56 -------------------
 .../restapi/project/ConfigInfoImpl.java       |  1 -
 .../server/permissions/RefControlTest.java    |  3 -
 9 files changed, 8 insertions(+), 137 deletions(-)
 delete mode 100644 java/com/google/gerrit/extensions/api/projects/ThemeInfo.java

diff --git a/Documentation/config-themes.txt b/Documentation/config-themes.txt
index 38bfc46e3b..a83c747a52 100644
--- a/Documentation/config-themes.txt
+++ b/Documentation/config-themes.txt
@@ -4,34 +4,28 @@ Gerrit supports some customization of the HTML it sends to
 the browser, allowing organizations to alter the look and
 feel of the application to fit with their general scheme.
 
-Configuration can either be sitewide or per-project. Projects without a
-specified theme inherit from their parents, or from the sitewide theme
-for `All-Projects`.
+== HTML Header/Footer and CSS
 
-Sitewide themes are stored in `'$site_path'/etc`, and per-project
-themes are stored in `'$site_path'/themes/{project-name}`. Files are
-only served from a single theme directory; if you want to modify or
-extend an inherited theme, you must copy it into the appropriate
-per-project directory.
-
-== HTML Header/Footer
+The HTML header, footer and CSS may be customized for login
+screens (LDAP, OAuth, OpenId) and the internally managed
+Gitweb servlet.
 
 At startup Gerrit reads the following files (if they exist) and
 uses them to customize the HTML page it sends to clients:
 
-* `<theme-dir>/GerritSiteHeader.html`
+* `etc/GerritSiteHeader.html`
 +
 HTML is inserted below the menu bar, but above any page content.
 This is a good location for an organizational logo, or links to
 other systems like bug tracking.
 
-* `<theme-dir>/GerritSiteFooter.html`
+* `etc/GerritSiteFooter.html`
 +
 HTML is inserted at the bottom of the page, below all other content,
 but just above the footer rule and the "Powered by Gerrit Code
 Review (v....)" message shown at the extreme bottom.
 
-* `<theme-dir>/GerritSite.css`
+* `etc/GerritSite.css`
 +
 The CSS rules are inlined into the top of the HTML page, inside
 of a `<style>` tag.  These rules can be used to support styling
diff --git a/Documentation/intro-project-owner.txt b/Documentation/intro-project-owner.txt
index cdc9c4e060..1f98291382 100644
--- a/Documentation/intro-project-owner.txt
+++ b/Documentation/intro-project-owner.txt
@@ -614,18 +614,6 @@ Project-specific download commands that are defined on a parent project
 are inherited by the child projects. A child project can overwrite an
 inherited download command, or remove it by assigning no value to it.
 
-[[theme]]
-== Theme
-
-Gerrit supports project-specific themes for customizing the appearance
-of the change screen and the diff screens. It is possible to define an
-HTML header and footer and to adapt Gerrit's CSS. Details about themes
-are explained in the link:config-themes.html[Themes] section.
-
-Project-specific themes can only be installed by Gerrit administrators
-since the theme files must be copied into the Gerrit installation
-folder.
-
 [[tool-integration]]
 == Integration with other tools
 
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 9c24c79c97..9803c19e09 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -3135,9 +3135,6 @@ Not set if the project state is `ACTIVE`.
 Map with the comment link configurations of the project. The name of
 the comment link configuration is mapped to a link:#commentlink-info[
 CommentlinkInfo] entity.
-|`theme`                                   |optional|
-The theme that is configured for the project as a link:#theme-info[
-ThemeInfo] entity.
 |`plugin_config`                           |optional|
 Plugin configuration as map which maps the plugin name to a map of
 parameter names to link:#config-parameter-info[ConfigParameterInfo]
@@ -3689,21 +3686,6 @@ as an annotated tag.
 |=========================
 
 
-[[theme-info]]
-=== ThemeInfo
-The `ThemeInfo` entity describes a theme.
-
-[options="header",cols="1,^2,4"]
-|=============================
-|Field Name      ||Description
-|`css`           |optional|
-The path to the `GerritSite.css` file.
-|`header`        |optional|
-The path to the `GerritSiteHeader.html` file.
-|`footer`        |optional|
-The path to the `GerritSiteFooter.html` file.
-|=============================
-
 GERRIT
 ------
 Part of link:index.html[Gerrit Code Review]
diff --git a/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java b/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
index 08ba486813..fb2a0feb0d 100644
--- a/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
+++ b/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
@@ -48,7 +48,6 @@ public class ConfigInfo {
   public Map<String, ActionInfo> actions;
 
   public Map<String, CommentLinkInfo> commentlinks;
-  public ThemeInfo theme;
 
   public Map<String, List<String>> extensionPanelNames;
 
diff --git a/java/com/google/gerrit/extensions/api/projects/ThemeInfo.java b/java/com/google/gerrit/extensions/api/projects/ThemeInfo.java
deleted file mode 100644
index d5d520f023..0000000000
--- a/java/com/google/gerrit/extensions/api/projects/ThemeInfo.java
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright (C) 2013 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.extensions.api.projects;
-
-public class ThemeInfo {
-  public static final ThemeInfo INHERIT = new ThemeInfo(null, null, null);
-
-  public final String css;
-  public final String header;
-  public final String footer;
-
-  public ThemeInfo(String css, String header, String footer) {
-    this.css = css;
-    this.header = header;
-    this.footer = footer;
-  }
-}
diff --git a/java/com/google/gerrit/server/config/SitePaths.java b/java/com/google/gerrit/server/config/SitePaths.java
index 11ec50cf04..47b6336ff3 100644
--- a/java/com/google/gerrit/server/config/SitePaths.java
+++ b/java/com/google/gerrit/server/config/SitePaths.java
@@ -43,7 +43,6 @@ public final class SitePaths {
   public final Path mail_dir;
   public final Path hooks_dir;
   public final Path static_dir;
-  public final Path themes_dir;
   public final Path index_dir;
 
   public final Path gerrit_sh;
@@ -67,8 +66,7 @@ public final class SitePaths {
   public final Path site_css;
   public final Path site_header;
   public final Path site_footer;
-  // For PolyGerrit UI only.
-  public final Path site_theme;
+  public final Path site_theme; // For PolyGerrit UI only.
   public final Path site_gitweb;
 
   /** {@code true} if {@link #site_path} has not been initialized. */
@@ -90,7 +88,6 @@ public final class SitePaths {
     mail_dir = etc_dir.resolve("mail");
     hooks_dir = p.resolve("hooks");
     static_dir = p.resolve("static");
-    themes_dir = p.resolve("themes");
     index_dir = p.resolve("index");
 
     gerrit_sh = bin_dir.resolve("gerrit.sh");
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index 78505ab20f..73c222dec9 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -15,12 +15,10 @@
 package com.google.gerrit.server.project;
 
 import static com.google.gerrit.common.data.PermissionRule.Action.ALLOW;
-import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.data.AccessSection;
@@ -32,7 +30,6 @@ import com.google.gerrit.common.data.PermissionRule;
 import com.google.gerrit.common.data.RefConfigSection;
 import com.google.gerrit.common.data.SubscribeSection;
 import com.google.gerrit.extensions.api.projects.CommentLinkInfo;
-import com.google.gerrit.extensions.api.projects.ThemeInfo;
 import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.index.project.ProjectData;
@@ -49,7 +46,6 @@ import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.account.CapabilityCollection;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.git.BranchOrderSection;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.TransferConfig;
@@ -57,8 +53,6 @@ import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -84,7 +78,6 @@ public class ProjectState {
 
   private final boolean isAllProjects;
   private final boolean isAllUsers;
-  private final SitePaths sitePaths;
   private final AllProjectsName allProjectsName;
   private final ProjectCache projectCache;
   private final GitRepositoryManager gitMgr;
@@ -105,11 +98,6 @@ public class ProjectState {
   /** Local access sections, wrapped in SectionMatchers for faster evaluation. */
   private volatile List<SectionMatcher> localAccessSections;
 
-  // TODO(dborowitz): Delete when the GWT UI gets deleted; in the meantime, don't bother with any
-  // refactoring.
-  /** Theme information loaded from site_path/themes. */
-  private volatile ThemeInfo theme;
-
   /** If this is all projects, the capabilities used by the server. */
   private final CapabilityCollection capabilities;
 
@@ -118,7 +106,6 @@ public class ProjectState {
 
   @Inject
   public ProjectState(
-      SitePaths sitePaths,
       ProjectCache projectCache,
       AllProjectsName allProjectsName,
       AllUsersName allUsersName,
@@ -128,7 +115,6 @@ public class ProjectState {
       TransferConfig transferConfig,
       MetricMaker metricMaker,
       @Assisted ProjectConfig config) {
-    this.sitePaths = sitePaths;
     this.projectCache = projectCache;
     this.isAllProjects = config.getProject().getNameKey().equals(allProjectsName);
     this.isAllUsers = config.getProject().getNameKey().equals(allUsersName);
@@ -548,24 +534,6 @@ public class ProjectState {
     return ret;
   }
 
-  public ThemeInfo getTheme() {
-    ThemeInfo theme = this.theme;
-    if (theme == null) {
-      synchronized (this) {
-        theme = this.theme;
-        if (theme == null) {
-          theme = loadTheme();
-          this.theme = theme;
-        }
-      }
-    }
-    if (theme == ThemeInfo.INHERIT) {
-      ProjectState parent = Iterables.getFirst(parents(), null);
-      return parent != null ? parent.getTheme() : null;
-    }
-    return theme;
-  }
-
   public Set<GroupReference> getAllGroups() {
     return getGroups(getAllSections());
   }
@@ -597,26 +565,6 @@ public class ProjectState {
     return all;
   }
 
-  private ThemeInfo loadTheme() {
-    String name = getConfig().getProject().getName();
-    Path dir = sitePaths.themes_dir.resolve(name);
-    if (!Files.exists(dir)) {
-      return ThemeInfo.INHERIT;
-    } else if (!Files.isDirectory(dir)) {
-      logger.atWarning().log("Bad theme for %s: not a directory", name);
-      return ThemeInfo.INHERIT;
-    }
-    try {
-      return new ThemeInfo(
-          readFile(dir.resolve(SitePaths.CSS_FILENAME)),
-          readFile(dir.resolve(SitePaths.HEADER_FILENAME)),
-          readFile(dir.resolve(SitePaths.FOOTER_FILENAME)));
-    } catch (IOException e) {
-      logger.atSevere().withCause(e).log("Error reading theme for %s", name);
-      return ThemeInfo.INHERIT;
-    }
-  }
-
   public ProjectData toProjectData() {
     ProjectData project = null;
     for (ProjectState state : treeInOrder()) {
@@ -625,10 +573,6 @@ public class ProjectState {
     return project;
   }
 
-  private String readFile(Path p) throws IOException {
-    return Files.exists(p) ? new String(Files.readAllBytes(p), UTF_8) : null;
-  }
-
   private LabelTypes loadLabelTypes() {
     Map<String, LabelType> types = new LinkedHashMap<>();
     for (ProjectState s : treeInOrder()) {
diff --git a/java/com/google/gerrit/server/restapi/project/ConfigInfoImpl.java b/java/com/google/gerrit/server/restapi/project/ConfigInfoImpl.java
index e1798962fb..37bc265dee 100644
--- a/java/com/google/gerrit/server/restapi/project/ConfigInfoImpl.java
+++ b/java/com/google/gerrit/server/restapi/project/ConfigInfoImpl.java
@@ -101,7 +101,6 @@ public class ConfigInfoImpl extends ConfigInfo {
     for (UiAction.Description d : uiActions.from(views, new ProjectResource(projectState, user))) {
       actions.put(d.getId(), new ActionInfo(d));
     }
-    this.theme = projectState.getTheme();
 
     this.extensionPanelNames = projectState.getConfig().getExtensionPanelSections();
   }
diff --git a/javatests/com/google/gerrit/server/permissions/RefControlTest.java b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
index 872cd911d7..3a9037c95f 100644
--- a/javatests/com/google/gerrit/server/permissions/RefControlTest.java
+++ b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
@@ -54,7 +54,6 @@ import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.AllProjectsNameProvider;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.AllUsersNameProvider;
-import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.git.TransferConfig;
 import com.google.gerrit.server.index.SingleVersionModule.SingleVersionListener;
 import com.google.gerrit.server.project.ProjectCache;
@@ -931,7 +930,6 @@ public class RefControlTest extends GerritBaseTests {
   }
 
   private InMemoryRepository add(ProjectConfig pc) {
-    SitePaths sitePaths = null;
     List<CommentLinkInfo> commentLinks = null;
 
     InMemoryRepository repo;
@@ -946,7 +944,6 @@ public class RefControlTest extends GerritBaseTests {
     all.put(
         pc.getName(),
         new ProjectState(
-            sitePaths,
             projectCache,
             allProjectsName,
             allUsersName,