github-advanced-security[bot] commented on code in PR #40:
URL: 
https://github.com/apache/sling-org-apache-sling-committer-cli/pull/40#discussion_r3715942916


##########
src/main/java/org/apache/sling/cli/impl/release/UpdateLocalSiteCommand.java:
##########
@@ -46,62 +71,238 @@
         })
 @CommandLine.Command(
         name = UpdateLocalSiteCommand.NAME,
-        description = "Updates the Sling website with the new release 
information, " + "based on a local checkout",
+        description = "Updates the Sling website with the new release 
information, based on a local checkout",
         subcommands = CommandLine.HelpCommand.class)
 public class UpdateLocalSiteCommand extends AbstractReleaseCommand {
 
     static final String GROUP = "release";
     static final String NAME = "update-local-site";
 
-    private static final String GIT_CHECKOUT = "/tmp/sling-site";
+    static final String GIT_CHECKOUT = "/tmp/sling-site";
+
+    /** Cloned over https from gitbox so the same ASF credentials that commit 
to dist.apache.org can push. */
+    static final String SITE_GIT_URL = 
"https://gitbox.apache.org/repos/asf/sling-site.git";;
+
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(UpdateLocalSiteCommand.class);
 
     @Reference
     private RepositoryService repositoryService;
 
-    private final Logger logger = LoggerFactory.getLogger(getClass());
+    @Reference
+    private CredentialsService credentialsService;
+
+    @Reference
+    private MembersFinder membersFinder;
+
+    @CommandLine.Mixin
+    private ReusableCLIOptions reusableCLIOptions;
 
     @Override
     public Integer call() {
         try {
-            ensureRepo();
-            try (Git git = Git.open(new File(GIT_CHECKOUT))) {
+            Set<Release> releases = resolveReleases(repositoryService);
+            if (releases.isEmpty()) {
+                LOGGER.error("Provide either --repository or --release.");
+                return CommandLine.ExitCode.USAGE;
+            }
+            StagingRepository repository = repositoryId != null ? 
repositoryService.find(repositoryId) : null;
 
-                Set<Release> releases = resolveReleases(repositoryService);
-                if (releases.isEmpty()) {
-                    logger.error("Provide either --repository or --release.");
-                    return CommandLine.ExitCode.USAGE;
-                }
+            SiteUpdate update = updateLocalSite(repositoryService, repository, 
releases);
+            applySiteUpdate(
+                    update,
+                    reusableCLIOptions.executionMode,
+                    credentialsService.getAsfCredentials(),
+                    membersFinder.getCurrentMember());
+        } catch (GitAPIException | IOException e) {
+            LOGGER.warn("Failed executing command", e);
+            return CommandLine.ExitCode.SOFTWARE;
+        }
+        return CommandLine.ExitCode.OK;
+    }
 
-                JBakeContentUpdater updater = new JBakeContentUpdater();
+    /**
+     * The result of editing the site checkout.
+     *
+     * @param hasChanges          whether the checkout has anything to commit
+     * @param releaseNames        the releases the edit covered, for the 
commit message
+     * @param downloadsNotListed  releases with no downloads-page entry at 
all, which a human must add;
+     *                            releases skipped because the page tracks 
another major version are not
+     *                            listed here, since for those there is 
legitimately nothing to do
+     */
+    record SiteUpdate(boolean hasChanges, String releaseNames, List<String> 
downloadsNotListed) {}
 
-                Path templatePath = Paths.get(GIT_CHECKOUT, "src", "main", 
"jbake", "templates", "downloads.tpl");
-                Path releasesPath = Paths.get(GIT_CHECKOUT, "src", "main", 
"jbake", "content", "releases.md");
-                LocalDateTime now = LocalDateTime.now();
-                for (Release release : releases) {
-                    updater.updateDownloads(templatePath, 
release.getComponent(), release.getVersion());
-                    updater.updateReleases(releasesPath, 
release.getComponent(), release.getVersion(), now);
-                }
+    /**
+     * Clones or refreshes the site checkout and applies the release 
information to {@code releases.md} and
+     * {@code downloads.tpl}. Nothing is committed; shared with {@link 
FinalizeCommand} so the editing flow
+     * is not duplicated there.
+     */
+    static SiteUpdate updateLocalSite(
+            RepositoryService repositoryService, StagingRepository repository, 
Set<Release> releases)
+            throws GitAPIException, IOException {
+
+        ensureRepo();
+        JBakeContentUpdater updater = new JBakeContentUpdater();
+        Path templatePath = Paths.get(GIT_CHECKOUT, "src", "main", "jbake", 
"templates", "downloads.tpl");
+        Path releasesPath = Paths.get(GIT_CHECKOUT, "src", "main", "jbake", 
"content", "releases.md");
+
+        List<String> notListed = new ArrayList<>();
+        LocalDateTime now = LocalDateTime.now();
+
+        for (Release release : releases) {
+            updater.updateReleases(releasesPath, release.getComponent(), 
release.getVersion(), now);
+            updateDownloadsFor(repositoryService, repository, release, 
updater, templatePath, notListed);
+        }
+
+        String releaseNames =
+                
releases.stream().map(Release::getFullName).sorted().collect(Collectors.joining(",
 "));
+
+        try (Git git = Git.open(new File(GIT_CHECKOUT))) {
+            git.diff().setOutputStream(System.out).call();
+            boolean hasChanges = !git.status().call().isClean();
+            return new SiteUpdate(hasChanges, releaseNames, notListed);
+        }
+    }
+
+    /** Updates every downloads-page entry belonging to {@code release}, 
recording why nothing changed. */
+    private static void updateDownloadsFor(
+            RepositoryService repositoryService,
+            StagingRepository repository,
+            Release release,
+            JBakeContentUpdater updater,
+            Path templatePath,
+            List<String> notListed)
+            throws IOException {
 
-                git.diff().setOutputStream(System.out).call();
+        Set<String> artifactIds = resolveArtifactIds(repositoryService, 
repository, release);
+        if (artifactIds.isEmpty()) {
+            LOGGER.warn(
+                    "Could not determine the artifact id(s) for {}; 
downloads.tpl not updated for it.",
+                    release.getFullName());
+            notListed.add(release.getFullName());
+            return;
+        }
+
+        int updated = 0;
+        int otherMajor = 0;
+        for (String artifactId : artifactIds) {
+            JBakeContentUpdater.DownloadsUpdate result =
+                    updater.updateDownloadsByArtifactId(templatePath, 
artifactId, release.getVersion());
+            updated += result.updated();
+            otherMajor += result.skippedOtherMajor();
+        }
+
+        if (updated > 0) {
+            LOGGER.info("Updated {} downloads.tpl entry/entries for {}", 
updated, release.getFullName());
+        } else if (otherMajor > 0) {
+            // dist.apache.org keeps several major streams published while the 
downloads page lists only the
+            // latest; a maintenance release of an older line therefore has 
nothing to update here
+            LOGGER.info(
+                    "downloads.tpl lists {} only for another major version; 
leaving it unchanged for {}.",
+                    artifactIds,
+                    release.getFullName());
+        } else {
+            LOGGER.warn(
+                    "downloads.tpl has no entry for {} ({}); it may need to be 
added by hand.",
+                    release.getFullName(),
+                    artifactIds);
+            notListed.add(release.getFullName());
+        }
+    }
+
+    /**
+     * Resolves the artifact ids of {@code release}, preferring the staged 
POMs and falling back to the
+     * released POMs on dist.apache.org so a run after promotion still works.
+     */
+    private static Set<String> resolveArtifactIds(
+            RepositoryService repositoryService, StagingRepository repository, 
Release release) throws IOException {
+        if (repository != null) {
+            Set<String> staged = repositoryService.getArtifactIds(repository, 
release);
+            if (!staged.isEmpty()) {
+                return new TreeSet<>(staged);
             }
-        } catch (GitAPIException | IOException e) {
-            logger.warn("Failed executing command", e);
-            return CommandLine.ExitCode.SOFTWARE;
         }
-        return CommandLine.ExitCode.OK;
+        List<String> candidates = 
UpdateDistCommand.listReleasePomFileNames(release.getVersion());
+        if (candidates.isEmpty()) {
+            return Set.of();
+        }
+        return new TreeSet<>(
+                
repositoryService.getArtifactIdsFromPomUrls(UpdateDistCommand.DIST_RELEASE_URL, 
candidates, release));
+    }
+
+    /** Commits and pushes the site update, honouring the execution mode. */
+    static void applySiteUpdate(SiteUpdate update, ExecutionMode mode, 
Credentials credentials, Member author)
+            throws GitAPIException, IOException {
+
+        if (!update.hasChanges()) {
+            LOGGER.info("The Sling website is already up to date; nothing to 
commit.");
+            return;
+        }
+        commitAndPushSiteChanges(
+                "Released " + update.releaseNames(),
+                "Commit the website changes above and push to sling-site?",
+                mode,
+                credentials,
+                author);
+    }
+
+    /**
+     * Commits everything staged under the site content directory and pushes 
it, honouring the execution
+     * mode. Shared by every command that edits the site checkout.
+     */
+    static void commitAndPushSiteChanges(
+            String message, String confirmQuestion, ExecutionMode mode, 
Credentials credentials, Member author)
+            throws GitAPIException, IOException {
+        switch (mode) {
+            case DRY_RUN:
+                LOGGER.info(
+                        "Would commit the changes above to {} with message 
\"{}\" and push.", SITE_GIT_URL, message);
+                break;
+            case INTERACTIVE:
+                if (InputOption.YES.equals(UserInput.yesNo(confirmQuestion, 
InputOption.YES))) {
+                    commitAndPush(message, credentials, author);
+                } else {
+                    LOGGER.info("Aborted; the changes are left in {}.", 
GIT_CHECKOUT);
+                }
+                break;
+            case AUTO:
+                commitAndPush(message, credentials, author);
+                break;
+        }
+    }
+
+    private static void commitAndPush(String message, Credentials credentials, 
Member author)
+            throws GitAPIException, IOException {
+        try (Git git = Git.open(new File(GIT_CHECKOUT))) {

Review Comment:
   ## SonarCloud / Temporary files should not be created in publicly writable 
directories
   
   <!--SONAR_ISSUE_KEY:AZ_Odx_wFzwiTi2kGWiD-->Make sure publicly writable 
directories are used safely here. <p>See more on <a 
href="https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-committer-cli&issues=AZ_Odx_wFzwiTi2kGWiD&open=AZ_Odx_wFzwiTi2kGWiD&pullRequest=40";>SonarQube
 Cloud</a></p>
   
   [Show more 
details](https://github.com/apache/sling-org-apache-sling-committer-cli/security/code-scanning/4)



##########
src/main/java/org/apache/sling/cli/impl/release/UpdateNewsCommand.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 org.apache.sling.cli.impl.release;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.LocalDateTime;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.sling.cli.impl.Command;
+import org.apache.sling.cli.impl.CredentialsService;
+import org.apache.sling.cli.impl.jbake.JBakeContentUpdater;
+import org.apache.sling.cli.impl.nexus.RepositoryService;
+import org.apache.sling.cli.impl.people.MembersFinder;
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Reference;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import picocli.CommandLine;
+
+/**
+ * Announces a release on the Sling website's news page.
+ *
+ * <p>Deliberately <em>not</em> part of {@link FinalizeCommand}: the release 
management guide only asks for a
+ * news entry when a release warrants an announcement, which is a judgement 
call, and most module releases do
+ * not get one. Run this by hand for the releases that do.
+ */
+@Component(
+        service = Command.class,
+        property = {
+            Command.PROPERTY_NAME_COMMAND_GROUP + "=" + 
UpdateNewsCommand.GROUP,
+            Command.PROPERTY_NAME_COMMAND_NAME + "=" + UpdateNewsCommand.NAME
+        })
[email protected](
+        name = UpdateNewsCommand.NAME,
+        description = "Announces a release on the Sling website's news page. 
Run only for releases worth announcing;"
+                + " this is not part of finalize.",
+        subcommands = CommandLine.HelpCommand.class)
+public class UpdateNewsCommand extends AbstractReleaseCommand {
+
+    static final String GROUP = "release";
+    static final String NAME = "update-news";
+
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(UpdateNewsCommand.class);
+
+    @CommandLine.Option(
+            names = {"--link"},
+            description = "Optional page the announcement should link to, e.g."
+                    + " /news/sling-14-released.html or 
/documentation/bundles/sling-pipes.html")
+    private String link;
+
+    @CommandLine.Mixin
+    private ReusableCLIOptions reusableCLIOptions;
+
+    @Reference
+    private RepositoryService repositoryService;
+
+    @Reference
+    private CredentialsService credentialsService;
+
+    @Reference
+    private MembersFinder membersFinder;
+
+    @Override
+    public Integer call() {
+        try {
+            Set<Release> releases = resolveReleases(repositoryService);
+            if (releases.isEmpty()) {
+                LOGGER.error("Provide either --repository or --release.");
+                return CommandLine.ExitCode.USAGE;
+            }
+
+            UpdateLocalSiteCommand.ensureRepo();
+            Path newsPath =
+                    Paths.get(UpdateLocalSiteCommand.GIT_CHECKOUT, "src", 
"main", "jbake", "content", "news.md");

Review Comment:
   ## SonarCloud / Temporary files should not be created in publicly writable 
directories
   
   <!--SONAR_ISSUE_KEY:AZ_Odx_4FzwiTi2kGWiE-->Make sure publicly writable 
directories are used safely here. <p>See more on <a 
href="https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-committer-cli&issues=AZ_Odx_4FzwiTi2kGWiE&open=AZ_Odx_4FzwiTi2kGWiE&pullRequest=40";>SonarQube
 Cloud</a></p>
   
   [Show more 
details](https://github.com/apache/sling-org-apache-sling-committer-cli/security/code-scanning/2)



##########
src/main/java/org/apache/sling/cli/impl/release/UpdateNewsCommand.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 org.apache.sling.cli.impl.release;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.LocalDateTime;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.sling.cli.impl.Command;
+import org.apache.sling.cli.impl.CredentialsService;
+import org.apache.sling.cli.impl.jbake.JBakeContentUpdater;
+import org.apache.sling.cli.impl.nexus.RepositoryService;
+import org.apache.sling.cli.impl.people.MembersFinder;
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Reference;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import picocli.CommandLine;
+
+/**
+ * Announces a release on the Sling website's news page.
+ *
+ * <p>Deliberately <em>not</em> part of {@link FinalizeCommand}: the release 
management guide only asks for a
+ * news entry when a release warrants an announcement, which is a judgement 
call, and most module releases do
+ * not get one. Run this by hand for the releases that do.
+ */
+@Component(
+        service = Command.class,
+        property = {
+            Command.PROPERTY_NAME_COMMAND_GROUP + "=" + 
UpdateNewsCommand.GROUP,
+            Command.PROPERTY_NAME_COMMAND_NAME + "=" + UpdateNewsCommand.NAME
+        })
[email protected](
+        name = UpdateNewsCommand.NAME,
+        description = "Announces a release on the Sling website's news page. 
Run only for releases worth announcing;"
+                + " this is not part of finalize.",
+        subcommands = CommandLine.HelpCommand.class)
+public class UpdateNewsCommand extends AbstractReleaseCommand {
+
+    static final String GROUP = "release";
+    static final String NAME = "update-news";
+
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(UpdateNewsCommand.class);
+
+    @CommandLine.Option(
+            names = {"--link"},
+            description = "Optional page the announcement should link to, e.g."
+                    + " /news/sling-14-released.html or 
/documentation/bundles/sling-pipes.html")
+    private String link;
+
+    @CommandLine.Mixin
+    private ReusableCLIOptions reusableCLIOptions;
+
+    @Reference
+    private RepositoryService repositoryService;
+
+    @Reference
+    private CredentialsService credentialsService;
+
+    @Reference
+    private MembersFinder membersFinder;
+
+    @Override
+    public Integer call() {
+        try {
+            Set<Release> releases = resolveReleases(repositoryService);
+            if (releases.isEmpty()) {
+                LOGGER.error("Provide either --repository or --release.");
+                return CommandLine.ExitCode.USAGE;
+            }
+
+            UpdateLocalSiteCommand.ensureRepo();
+            Path newsPath =
+                    Paths.get(UpdateLocalSiteCommand.GIT_CHECKOUT, "src", 
"main", "jbake", "content", "news.md");
+
+            JBakeContentUpdater updater = new JBakeContentUpdater();
+            LocalDateTime now = LocalDateTime.now();
+            boolean changed = false;
+            for (Release release : releases) {
+                if (updater.updateNews(newsPath, release.getFullName(), link, 
now)) {
+                    LOGGER.info("Added a news entry for {}", 
release.getFullName());
+                    changed = true;
+                } else {
+                    LOGGER.info("The news page already announces {}; 
skipping.", release.getFullName());
+                }
+            }
+
+            if (!changed) {
+                LOGGER.info("Nothing to commit.");
+                return CommandLine.ExitCode.OK;
+            }
+
+            try (Git git = Git.open(new 
File(UpdateLocalSiteCommand.GIT_CHECKOUT))) {

Review Comment:
   ## SonarCloud / Temporary files should not be created in publicly writable 
directories
   
   <!--SONAR_ISSUE_KEY:AZ_Odx_4FzwiTi2kGWiG-->Make sure publicly writable 
directories are used safely here. <p>See more on <a 
href="https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-committer-cli&issues=AZ_Odx_4FzwiTi2kGWiG&open=AZ_Odx_4FzwiTi2kGWiG&pullRequest=40";>SonarQube
 Cloud</a></p>
   
   [Show more 
details](https://github.com/apache/sling-org-apache-sling-committer-cli/security/code-scanning/3)



-- 
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]

Reply via email to