Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

281 log apex code on validation failure #311

Merged
merged 2 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down Expand Up @@ -84,7 +84,7 @@ trait ModuleRefresh {

// Update and validate
replaceType(newType.typeName, Some(newType))
newType.validate()
newType.safeValidate()
(typeId, holders.toSet)
})
} else {
Expand Down Expand Up @@ -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))
}
Expand Down
46 changes: 44 additions & 2 deletions jvm/src/main/scala/com/nawforce/apexlink/org/OrgInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
19 changes: 6 additions & 13 deletions jvm/src/main/scala/com/nawforce/apexlink/org/StreamDeployer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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() }
}
}

Expand Down Expand Up @@ -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)
}
Expand All @@ -301,7 +294,7 @@ class StreamDeployer(
.create(module, doc.path, data)
.map(td => {
types.put(td)
td.validate()
td.safeValidate()
})
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ class SummaryDeclaration(
// Nothing to do here
}

override def validate(): Unit = {
override protected def validate(): Unit = {
propagateOuterDependencies(new TypeCache())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class BasicTypeDeclaration(
override def constructors: ArraySeq[ConstructorDeclaration] =
ConstructorDeclaration.emptyConstructorDeclarations

override def validate(): Unit = {}
override protected def validate(): Unit = {}
}

class InnerBasicTypeDeclaration(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
88 changes: 88 additions & 0 deletions jvm/src/test/scala/com/nawforce/apexlink/org/OrgInfoTest.scala
Original file line number Diff line number Diff line change
@@ -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)
}
}
}

}
Loading