From 2b9aa9d551c3083878048594105a0f9366790bb9 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 2 Aug 2024 17:10:21 -0400 Subject: [PATCH 1/2] Optimize `JobLocalConfiguration` --- .../hudson/plugins/jobConfigHistory/FileHistoryDao.java | 2 +- .../plugins/jobConfigHistory/JobLocalConfiguration.java | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/java/hudson/plugins/jobConfigHistory/FileHistoryDao.java b/src/main/java/hudson/plugins/jobConfigHistory/FileHistoryDao.java index 00109eb4..a95f75db 100644 --- a/src/main/java/hudson/plugins/jobConfigHistory/FileHistoryDao.java +++ b/src/main/java/hudson/plugins/jobConfigHistory/FileHistoryDao.java @@ -376,6 +376,7 @@ private Optional removeChangeReasonComment(final XmlFile configFile) thr new Object[]{JobConfigHistoryConsts.JOB_LOCAL_CONFIGURATION_XML_TAG, configFile.getFile()}); return Optional.empty(); } else if (jobLocalConfigurationNodes.getLength() == 1) { + // Tag is found. Content ought to be nonempty (see JobLocalConfiguration.DescriptorImpl.newInstance). org.w3c.dom.Node jobLocalConfiguration = jobLocalConfigurationNodes.item(0); NodeList jlcChildren = jobLocalConfiguration.getChildNodes(); org.w3c.dom.Node changeReasonCommentNode = null; @@ -386,7 +387,6 @@ private Optional removeChangeReasonComment(final XmlFile configFile) thr } } if (changeReasonCommentNode != null) { - //tag is found. Might contain no comment (getTextContent() returns ""). String changeReasonComment = changeReasonCommentNode.getTextContent(); if (changeReasonComment != null) { //delete jobLocalConfiguration node from document diff --git a/src/main/java/hudson/plugins/jobConfigHistory/JobLocalConfiguration.java b/src/main/java/hudson/plugins/jobConfigHistory/JobLocalConfiguration.java index 63057401..254b7f35 100644 --- a/src/main/java/hudson/plugins/jobConfigHistory/JobLocalConfiguration.java +++ b/src/main/java/hudson/plugins/jobConfigHistory/JobLocalConfiguration.java @@ -8,6 +8,7 @@ import hudson.model.JobPropertyDescriptor; import hudson.util.FormValidation; import net.sf.json.JSONObject; +import org.apache.commons.lang.StringUtils; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; @@ -48,6 +49,12 @@ public boolean isApplicable(AbstractProject item) { return true; } + @Override + public JobProperty newInstance(StaplerRequest req, JSONObject formData) throws FormException { + JobLocalConfiguration jp = (JobLocalConfiguration) super.newInstance(req, formData); + return StringUtils.isBlank(jp.changeReasonComment) ? null : jp; + } + public boolean configure(StaplerRequest request, JSONObject jsonObject) throws FormException { LOG.info("CONFIGURE"); throw new FormException("form exception", "localValues.changeReasonComment"); From 8e11b899b0196d78955c40038d6c08cb5a0d007c Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 9 Aug 2024 16:03:59 -0400 Subject: [PATCH 2/2] Simpler to just keep the change comment in memory --- .../jobConfigHistory/FileHistoryDao.java | 72 +------------------ .../JobLocalConfiguration.java | 20 +++++- 2 files changed, 19 insertions(+), 73 deletions(-) diff --git a/src/main/java/hudson/plugins/jobConfigHistory/FileHistoryDao.java b/src/main/java/hudson/plugins/jobConfigHistory/FileHistoryDao.java index a95f75db..32af7430 100644 --- a/src/main/java/hudson/plugins/jobConfigHistory/FileHistoryDao.java +++ b/src/main/java/hudson/plugins/jobConfigHistory/FileHistoryDao.java @@ -35,16 +35,6 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.SystemUtils; -import org.w3c.dom.Document; -import org.w3c.dom.NodeList; -import org.xml.sax.SAXException; - -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.transform.TransformerException; -import javax.xml.transform.TransformerFactory; -import javax.xml.transform.dom.DOMSource; -import javax.xml.transform.stream.StreamResult; import java.io.BufferedOutputStream; import java.io.File; import java.io.FileFilter; @@ -353,69 +343,9 @@ private void createNewHistoryEntryAndCopyConfig(final XmlFile configFile, } } - /** - * Find this in the configFile:
- *   <hudson.plugins.jobConfigHistory.JobLocalConfiguration plugin="jobConfigHistory@2.25-SNAPSHOT">
- *     <changeReasonComment>MY_CHANGE_REASON_COMMENT</changeReasonComment>
- *   </hudson.plugins.jobConfigHistory.JobLocalConfiguration>
- * and delete it, receiving the changeReasonComment, if present. - * - * @param configFile the config file - * @return the String in changeReasonComment, if found. Optional.empty(), else. - * @throws IOException configFile is read. - * @throws SAXException parsing the configFile. - * @throws TransformerException parsing the configFile. - * @throws ParserConfigurationException configuring the xml parser. - */ - private Optional removeChangeReasonComment(final XmlFile configFile) throws IOException, SAXException, TransformerException, ParserConfigurationException { - Document configFiledocument = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(configFile.getFile()); - - NodeList jobLocalConfigurationNodes = configFiledocument.getElementsByTagName(JobConfigHistoryConsts.JOB_LOCAL_CONFIGURATION_XML_TAG); - if (jobLocalConfigurationNodes.getLength() > 1) { - LOG.log(FINEST, "tag \"{0}\" found twice in {1}, not saving the change reason comment.", - new Object[]{JobConfigHistoryConsts.JOB_LOCAL_CONFIGURATION_XML_TAG, configFile.getFile()}); - return Optional.empty(); - } else if (jobLocalConfigurationNodes.getLength() == 1) { - // Tag is found. Content ought to be nonempty (see JobLocalConfiguration.DescriptorImpl.newInstance). - org.w3c.dom.Node jobLocalConfiguration = jobLocalConfigurationNodes.item(0); - NodeList jlcChildren = jobLocalConfiguration.getChildNodes(); - org.w3c.dom.Node changeReasonCommentNode = null; - for (int i = 0; i < jlcChildren.getLength(); ++i) { - org.w3c.dom.Node node = jlcChildren.item(i); - if (node.getNodeName().equals(JobConfigHistoryConsts.CHANGE_REASON_COMMENT_XML_TAG)) { - changeReasonCommentNode = node; - } - } - if (changeReasonCommentNode != null) { - String changeReasonComment = changeReasonCommentNode.getTextContent(); - if (changeReasonComment != null) { - //delete jobLocalConfiguration node from document - jobLocalConfiguration.getParentNode().removeChild(jobLocalConfiguration); - //save xml - TransformerFactory.newInstance().newTransformer() - .transform(new DOMSource(configFiledocument), new StreamResult(configFile.getFile())); - - return changeReasonComment.isEmpty() ? Optional.empty() : Optional.of(changeReasonComment); - } else return Optional.empty(); - } else return Optional.empty(); - } else { - //no jobLocalConfiguration node found. Should be there even if the message field was empty! - LOG.log(FINEST, "tag \"{0}\" not found in {1}, no comment could be found.", - new Object[]{JobConfigHistoryConsts.JOB_LOCAL_CONFIGURATION_XML_TAG, configFile.getFile()}); - return Optional.empty(); - } - } - @Override public void saveItem(final XmlFile file) { - //remove the changeReasonComment entry from the xml file. (before checking duplicates!) - Optional changeReasonCommentOptional; - try { - changeReasonCommentOptional = removeChangeReasonComment(file); - } catch (IOException | SAXException | TransformerException | ParserConfigurationException e) { - LOG.log(WARNING, "Error occurred while trying to extract changeReasonComment from config file: {0}", e); - changeReasonCommentOptional = Optional.empty(); - } + Optional changeReasonCommentOptional = JobLocalConfiguration.lastChangeReasonComment(file); if (checkDuplicate(file)) { createNewHistoryEntryAndCopyConfig(file, diff --git a/src/main/java/hudson/plugins/jobConfigHistory/JobLocalConfiguration.java b/src/main/java/hudson/plugins/jobConfigHistory/JobLocalConfiguration.java index 254b7f35..1e8fa8df 100644 --- a/src/main/java/hudson/plugins/jobConfigHistory/JobLocalConfiguration.java +++ b/src/main/java/hudson/plugins/jobConfigHistory/JobLocalConfiguration.java @@ -1,14 +1,20 @@ package hudson.plugins.jobConfigHistory; import hudson.Extension; +import hudson.Util; +import hudson.XmlFile; import hudson.model.AbstractProject; import hudson.model.Item; import hudson.model.Job; import hudson.model.JobProperty; import hudson.model.JobPropertyDescriptor; import hudson.util.FormValidation; +import java.io.File; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; import net.sf.json.JSONObject; -import org.apache.commons.lang.StringUtils; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; @@ -21,6 +27,12 @@ public class JobLocalConfiguration extends JobProperty> { private static final Logger LOG = Logger .getLogger(JobLocalConfiguration.class.getName()); + private static final Map lastChangeReasonCommentByXmlFile = Collections.synchronizedMap(new HashMap<>()); + + static Optional lastChangeReasonComment(XmlFile file) { + return Optional.ofNullable(lastChangeReasonCommentByXmlFile.remove(file.getFile())); + } + private final String changeReasonComment; @@ -52,7 +64,11 @@ public boolean isApplicable(AbstractProject item) { @Override public JobProperty newInstance(StaplerRequest req, JSONObject formData) throws FormException { JobLocalConfiguration jp = (JobLocalConfiguration) super.newInstance(req, formData); - return StringUtils.isBlank(jp.changeReasonComment) ? null : jp; + Job job = req.findAncestorObject(Job.class); + if (job != null) { + lastChangeReasonCommentByXmlFile.put(job.getConfigFile().getFile(), Util.fixEmptyAndTrim(jp.changeReasonComment)); + } + return null; } public boolean configure(StaplerRequest request, JSONObject jsonObject) throws FormException {