Merge pull request #311 from apex-dev-tools/281-log-apex-code-on-vali…

…dation-failure

281 log apex code on validation failure

jvm/src/main/scala/com/nawforce/apexlink/org/ModuleRefresh.scala

@@ -42,7 +42,7 @@ trait ModuleRefresh {
     val holders   = existingLabels.getTypeDependencyHolders
     newLabels.setTypeDependencyHolders(holders)
     replaceType(newLabels.typeName, Some(newLabels))
-    newLabels.validate()
+    newLabels.safeValidate()
     Seq((newLabels.typeId, holders.toSet))
   }
 
@@ -84,7 +84,7 @@ trait ModuleRefresh {
 
               // Update and validate
               replaceType(newType.typeName, Some(newType))
-              newType.validate()
+              newType.safeValidate()
               (typeId, holders.toSet)
             })
           } else {
@@ -153,7 +153,7 @@ trait ModuleRefresh {
         // Hack: we don't need to revalidate here but MetadataValidator wipes out all
         // diagnostics so if we don't validate we can save to cache without important diagnostics,
         // especially the Missing ones which will break invalidation handling.
-        getDependentType(doc.controllingTypeName(namespace)).foreach(_.validate())
+        getDependentType(doc.controllingTypeName(namespace)).foreach(_.safeValidate())
         None
       case _ => Some(createSupportedTypes(doc, source))
     }

jvm/src/main/scala/com/nawforce/apexlink/org/OrgInfo.scala

@@ -14,9 +14,18 @@
 package com.nawforce.apexlink.org
 
 import com.nawforce.apexlink.org.OPM.OrgImpl
-import com.nawforce.pkgforce.diagnostics.{Diagnostic, ERROR_CATEGORY, Issue, MISSING_CATEGORY}
-import com.nawforce.pkgforce.path.PathLocation
+import com.nawforce.pkgforce.diagnostics.{
+  Diagnostic,
+  ERROR_CATEGORY,
+  Issue,
+  LoggerOps,
+  MISSING_CATEGORY
+}
+import com.nawforce.pkgforce.path.{Location, PathLike, PathLocation}
 
+import java.io.{File, PrintWriter, StringWriter}
+import java.nio.file.Files
+import scala.collection.compat.immutable.ArraySeq
 import scala.util.DynamicVariable
 
 /** Access to the 'current' org, this should be deprecated now we have the OPM hierarchy.
@@ -42,4 +51,37 @@ object OrgInfo {
     log(new Issue(pathLocation.path, Diagnostic(MISSING_CATEGORY, pathLocation.location, message)))
   }
 
+  /** Log an exception during processing. If at least one path is provided this logs the
+    * exception against the first. The files are copied to a temporary directory to aid debugging.
+    */
+  private[nawforce] def logException(ex: Throwable, paths: ArraySeq[PathLike]): Unit = {
+    if (paths.isEmpty) {
+      LoggerOps.info("Exception reported against no paths", ex)
+      return
+    }
+
+    try {
+      val writer = new StringWriter
+      writer.append("Validation failed: ")
+      val tempDir = Files.createTempDirectory("apex-ls-log")
+      paths.foreach(path => {
+        val from = new File(path.toString).toPath
+        if (Files.exists(from)) {
+          Files.copy(
+            from,
+            tempDir.resolve(from.getFileName),
+            java.nio.file.StandardCopyOption.REPLACE_EXISTING
+          )
+        }
+      })
+      writer.append("log directory ")
+      writer.append(tempDir.toString)
+      writer.append('\n')
+      ex.printStackTrace(new PrintWriter(writer))
+      log(Issue(ERROR_CATEGORY, PathLocation(paths.head, Location.empty), writer.toString))
+    } catch {
+      case ex: Throwable =>
+        LoggerOps.info("Failed to log an exception", ex)
+    }
+  }
 }

jvm/src/main/scala/com/nawforce/apexlink/org/PackageAPI.scala

@@ -394,7 +394,7 @@ trait PackageAPI extends Package {
       td.paths.foreach(path => org.issues.pop(path))
       td.preReValidate()
     })
-    collectedTypes.foreach(_.validate())
+    collectedTypes.foreach(_.safeValidate())
   }
 
   /* Collect all classes in a super class hierarchy that have an abstract ancestor */

jvm/src/main/scala/com/nawforce/apexlink/org/StreamDeployer.scala

@@ -61,8 +61,8 @@ class StreamDeployer(
       consumeSObjects(bufferedIterator)
       consumeClasses(bufferedIterator)
       consumeTriggers(bufferedIterator)
-      components.validate()
-      pages.validate()
+      components.safeValidate()
+      pages.safeValidate()
     }
 
     // Run plugins over loaded types DependentTypes
@@ -113,7 +113,7 @@ class StreamDeployer(
     })
 
     // Run custom validation to setup dependencies
-    sobjects.foreach(_.validate())
+    sobjects.foreach(_.safeValidate())
   }
 
   /** Consume Apex class events, this is a bit more involved as we try and load first via cache and
@@ -162,14 +162,7 @@ class StreamDeployer(
         )
 
       // Validate the classes, this must be last due to mutual dependence
-      decls.foreach { decl =>
-        try {
-          decl.validate()
-        } catch {
-          case ex: Throwable =>
-            module.log(decl.paths.head, "Validation failed", ex)
-        }
-      }
+      decls.foreach { _.safeValidate() }
     }
   }
 
@@ -281,7 +274,7 @@ class StreamDeployer(
       localAccum.entrySet.forEach(kv => {
         types.put(kv.getKey, kv.getValue)
       })
-      localAccum.values().asScala.foreach(_.validate())
+      localAccum.values().asScala.foreach(_.safeValidate())
     }
     ArraySeq.from(failedDocuments.asScala.toSeq)
   }
@@ -301,7 +294,7 @@ class StreamDeployer(
                 .create(module, doc.path, data)
                 .map(td => {
                   types.put(td)
-                  td.validate()
+                  td.safeValidate()
                 })
           }
         })

jvm/src/main/scala/com/nawforce/apexlink/types/apex/FullDeclaration.scala

@@ -116,7 +116,7 @@ abstract class FullDeclaration(
     }
   }
 
-  override def validate(): Unit = {
+  override protected def validate(): Unit = {
     LoggerOps.debugTime(s"Validated ${location.path}") {
       // Validate inside a parsing context as LazyBlock may call parser
       CST.sourceContext.withValue(Some(source)) {

jvm/src/main/scala/com/nawforce/apexlink/types/apex/SummaryDeclaration.scala

@@ -393,7 +393,7 @@ class SummaryDeclaration(
     // Nothing to do here
   }
 
-  override def validate(): Unit = {
+  override protected def validate(): Unit = {
     propagateOuterDependencies(new TypeCache())
   }
 

jvm/src/main/scala/com/nawforce/apexlink/types/apex/TriggerDeclaration.scala

@@ -86,7 +86,7 @@ final case class TriggerDeclaration(
   private var depends: Option[SkinnySet[Dependent]] = None
   private val objectTypeName = TypeName(objectNameId.name, Nil, Some(TypeNames.Schema))
 
-  override def validate(): Unit = {
+  override protected def validate(): Unit = {
     LoggerOps.debugTime(s"Validated ${location.path}") {
       val context = new TypeVerifyContext(None, this, None, enablePlugins = true)
       val tdOpt   = context.getTypeAndAddDependency(objectTypeName, this)

jvm/src/main/scala/com/nawforce/apexlink/types/core/BasicTypeDeclaration.scala

@@ -46,7 +46,7 @@ class BasicTypeDeclaration(
   override def constructors: ArraySeq[ConstructorDeclaration] =
     ConstructorDeclaration.emptyConstructorDeclarations
 
-  override def validate(): Unit = {}
+  override protected def validate(): Unit = {}
 }
 
 class InnerBasicTypeDeclaration(

jvm/src/main/scala/com/nawforce/apexlink/types/core/TypeDeclaration.scala

@@ -31,11 +31,14 @@ import com.nawforce.apexlink.types.synthetic.{
   CustomFieldDeclaration,
   LocatableCustomFieldDeclaration
 }
+import com.nawforce.pkgforce.diagnostics.{ERROR_CATEGORY, Issue}
 import com.nawforce.pkgforce.modifiers._
 import com.nawforce.pkgforce.names.{Name, Names, TypeName}
 import com.nawforce.pkgforce.parsers.{CLASS_NATURE, INTERFACE_NATURE, Nature}
-import com.nawforce.pkgforce.path.{PathLike, UnsafeLocatable}
+import com.nawforce.pkgforce.path.{Location, PathLike, PathLocation, UnsafeLocatable}
 
+import java.io.{PrintWriter, StringWriter}
+import java.nio.file.Files
 import scala.collection.immutable.ArraySeq
 import scala.collection.mutable
 
@@ -425,7 +428,15 @@ trait TypeDeclaration extends AbstractTypeDeclaration with Dependent with PreReV
       .flatMap(typeName => TypeResolver(typeName, this).toOption)
       .getOrElse(this)
 
-  def validate(): Unit
+  def safeValidate(): Unit = {
+    try {
+      validate()
+    } catch {
+      case ex: Throwable => OrgInfo.logException(ex, paths)
+    }
+  }
+
+  protected def validate(): Unit
 
   override def findNestedType(name: Name): Option[TypeDeclaration] = {
     nestedTypes.find(_.name == name)

jvm/src/main/scala/com/nawforce/apexlink/types/other/ComponentDeclaration.scala

@@ -70,7 +70,7 @@ final case class Component(
   override def dependencies(): Iterable[Dependent] =
     depends.getOrElse(Array[Dependent]())
 
-  override def validate(): Unit = {
+  override protected def validate(): Unit = {
     super.validate()
     vfContainer.foreach(vf => {
       depends = Some(vf.validate())
@@ -122,8 +122,8 @@ final case class ComponentDeclaration(
   // Propagate dependencies to nested
   nestedComponents.foreach(_.addTypeDependencyHolder(typeId))
 
-  override def validate(): Unit = {
-    components.foreach(_.validate())
+  override protected def validate(): Unit = {
+    components.foreach(_.safeValidate())
     propagateOuterDependencies(new TypeCache())
   }
 

jvm/src/main/scala/com/nawforce/apexlink/types/other/PageDeclaration.scala

@@ -107,7 +107,7 @@ final case class PageDeclaration(
     new PageDeclaration(sourceInfo, module, newPages)
   }
 
-  override def validate(): Unit = {
+  override protected def validate(): Unit = {
     // We may create multiple Pages for each .page file to handle namespaces
     // We only want to validate one of them to avoid duplicate diagnostics
     val uniquePages = pages.map(page => (page.location.path, page)).toMap.values

jvm/src/main/scala/com/nawforce/apexlink/types/platform/PlatformTypeDeclaration.scala

@@ -174,7 +174,7 @@ class PlatformTypeDeclaration(val native: Any, val outer: Option[PlatformTypeDec
       .map(c => new PlatformConstructor(c, this))
   }
 
-  override def validate(): Unit = {
+  override protected def validate(): Unit = {
     // Not required
   }
 }

jvm/src/main/scala/com/nawforce/apexlink/types/schema/GhostSObjectDeclaration.scala

@@ -42,7 +42,7 @@ final case class GhostSObjectDeclaration(module: OPM.Module, _typeName: TypeName
     TypeResolver(superClass.get, this).toOption
   }
 
-  override def validate(): Unit = {
+  override protected def validate(): Unit = {
     // Not required
   }
 

jvm/src/main/scala/com/nawforce/apexlink/types/schema/SObjectDeclaration.scala

@@ -118,7 +118,7 @@ final case class SObjectDeclaration(
   override lazy val superClassDeclaration: Option[TypeDeclaration] =
     TypeResolver(superClass.get, this).toOption
 
-  override def validate(): Unit = {
+  override protected def validate(): Unit = {
     // Check field types, can be ignored for Feed, Share & History synthetic SObjects
     if (isSynthetic) return
 

jvm/src/test/scala/com/nawforce/apexlink/org/OrgInfoTest.scala

@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2024 Certinia Inc. All rights reserved.
+ */
+package com.nawforce.apexlink.org
+
+import com.nawforce.apexlink.TestHelper
+import com.nawforce.pkgforce.diagnostics.LoggerOps.{INFO_LOGGING, NO_LOGGING}
+import com.nawforce.pkgforce.diagnostics.{Logger, LoggerOps}
+import com.nawforce.pkgforce.path.PathLike
+import com.nawforce.runtime.FileSystemHelper
+import org.scalatest.funsuite.AnyFunSuite
+
+import java.io.{File, StringWriter}
+import scala.collection.immutable.ArraySeq
+
+class OrgInfoTest extends AnyFunSuite with TestHelper {
+
+  final class CaptureLogger extends Logger {
+    val writer: StringWriter = new StringWriter()
+
+    override def info(message: String): Unit = {
+      writer.append(s"INFO: $message\n")
+    }
+
+    override def debug(message: String): Unit = {
+      writer.append(s"DEBUG: $message\n")
+    }
+
+    override def trace(message: String): Unit = {
+      writer.append(s"TRACE: $message\n")
+    }
+  }
+
+  test("Log exception without files creates info message") {
+    FileSystemHelper.run(Map()) { root: PathLike =>
+      createOrg(root)
+
+      val captureLogger = new CaptureLogger
+      val oldLogger     = LoggerOps.setLogger(captureLogger)
+      LoggerOps.setLoggingLevel(INFO_LOGGING)
+
+      OrgInfo.logException(new Exception("Hello"), ArraySeq())
+
+      assert(captureLogger.writer.toString.startsWith("INFO: Exception reported against no paths"))
+      assert(captureLogger.writer.toString.contains("INFO: java.lang.Exception: Hello"))
+
+      LoggerOps.setLoggingLevel(NO_LOGGING)
+      LoggerOps.setLogger(oldLogger)
+
+    }
+  }
+
+  test("Log exception captures files") {
+    FileSystemHelper.runTempDir(
+      Map("a.txt" -> "a.txt", "dir1/b.txt" -> "b.txt", "dir1/dir2/c.txt" -> "c.txt")
+    ) { root: PathLike =>
+      createOrg(root)
+
+      val captureLogger = new CaptureLogger
+      val oldLogger     = LoggerOps.setLogger(captureLogger)
+      LoggerOps.setLoggingLevel(INFO_LOGGING)
+
+      try {
+
+        withOrg { _ =>
+          OrgInfo.logException(
+            new Exception("Hello"),
+            ArraySeq(root.join("a.txt"), root.join("dir1/b.txt"), root.join("dir1/dir2/c.txt"))
+          )
+        }
+
+        assert(captureLogger.writer.toString.isEmpty)
+        assert(getMessages().contains("/a.txt: Error: line 1: Validation failed: log directory"))
+        assert(getMessages().contains("java.lang.Exception: Hello"))
+
+        val tmpDir     = getMessages().split("\n").head.split("directory").last.trim
+        val tmpDirPath = new File(tmpDir)
+        assert(tmpDirPath.isDirectory)
+        assert(tmpDirPath.listFiles().map(_.getName).toSet == Set("a.txt", "b.txt", "c.txt"))
+
+      } finally {
+        LoggerOps.setLoggingLevel(NO_LOGGING)
+        LoggerOps.setLogger(oldLogger)
+      }
+    }
+  }
+
+}