exceptionfactory commented on code in PR #10431: URL: https://github.com/apache/nifi/pull/10431#discussion_r2440071105
########## nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/test/java/org/apache/nifi/events/BulletinFactoryStackTraceTest.java: ########## @@ -0,0 +1,61 @@ +/* + * 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.nifi.events; + +import org.apache.nifi.reporting.Bulletin; +import org.apache.nifi.reporting.ComponentType; +import org.junit.jupiter.api.Test; + +import java.io.PrintWriter; +import java.io.StringWriter; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class BulletinFactoryStackTraceTest { Review Comment: Minor formatting bit, the `public` modifiers are not needed on the class or method level in JUnit 5 tests. ########## nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/test/java/org/apache/nifi/events/BulletinFactoryStackTraceTest.java: ########## @@ -0,0 +1,61 @@ +/* + * 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.nifi.events; + +import org.apache.nifi.reporting.Bulletin; +import org.apache.nifi.reporting.ComponentType; +import org.junit.jupiter.api.Test; + +import java.io.PrintWriter; +import java.io.StringWriter; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class BulletinFactoryStackTraceTest { + + private static String toStackTrace(final Throwable t) { + final StringWriter sw = new StringWriter(); + try (PrintWriter pw = new PrintWriter(sw)) { + t.printStackTrace(pw); + } + return sw.toString(); + } + + @Test + public void testCreateBulletinWithThrowableIncludesPrintableStackTrace() { + final Exception cause = new IllegalStateException("inner"); + final RuntimeException ex = new RuntimeException("outer", cause); + + final Bulletin bulletin = BulletinFactory.createBulletin( + "pg1", "Process Group 1", "proc1", ComponentType.PROCESSOR, "MyProcessor", + "Log Message", "ERROR", "Something failed", "/root / Process Group 1", null, ex); + + assertNotNull(bulletin); + final String stackTrace = bulletin.getStackTrace(); + assertNotNull(stackTrace, "Stack trace should be set on bulletin"); + + final String expected = toStackTrace(ex); + assertEquals(expected, stackTrace, "Stack trace should match Throwable.printStackTrace output exactly"); + assertTrue(stackTrace.contains("RuntimeException")); + assertTrue(stackTrace.contains("IllegalStateException")); Review Comment: Minor adjustment, `cause.getClass().getSimpleName()` and `ex.getClass().getSimpleName()` could be used instead of the string values. ########## nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/api/dto/DtoFactoryBulletinStackTraceTest.java: ########## @@ -0,0 +1,67 @@ +/* + * 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.nifi.web.api.dto; + +import org.apache.nifi.events.BulletinFactory; +import org.apache.nifi.reporting.Bulletin; +import org.apache.nifi.reporting.ComponentType; +import org.junit.jupiter.api.Test; + +import java.io.PrintWriter; +import java.io.StringWriter; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +public class DtoFactoryBulletinStackTraceTest { Review Comment: ```suggestion class DtoFactoryBulletinStackTraceTest { ``` ########## nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java: ########## @@ -3474,10 +3474,25 @@ public List<BulletinDTO> createBulletinDtos(final List<Bulletin> bulletins) { /** * Creates a BulletinDTO for the specified Bulletin. * + * Note: By default, stack trace is not included as it should be exposed + * only for the Bulletin Board API. Use the overloaded method with + * includeStackTrace=true when serving the Bulletin Board. + * * @param bulletin bulletin * @return dto */ public BulletinDTO createBulletinDto(final Bulletin bulletin) { Review Comment: Is it necessary to retain this method? It might be better to remove this signature to make sure all references explicitly declare whether to include stack traces. ########## nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/jaxb/BulletinAdapterStackTraceTest.java: ########## @@ -0,0 +1,46 @@ +/* + * 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.nifi.jaxb; + +import org.apache.nifi.events.BulletinFactory; +import org.apache.nifi.reporting.Bulletin; +import org.apache.nifi.reporting.ComponentType; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +public class BulletinAdapterStackTraceTest { + + @Test + public void testMarshalUnmarshalCarriesStackTrace() throws Exception { Review Comment: ```suggestion void testMarshalUnmarshalCarriesStackTrace() throws Exception { ``` ########## nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/events/BulletinFactory.java: ########## @@ -126,4 +177,17 @@ private static ComponentType getComponentType(final Connectable connectable) { default -> ComponentType.PROCESSOR; }; } + + private static String formatStackTrace(final Throwable t) { + try { + final java.io.StringWriter sw = new java.io.StringWriter(); + final java.io.PrintWriter pw = new java.io.PrintWriter(sw); Review Comment: Is there a reason for using the fully-qualified class names? Although it should not matter with this in-memory usage, should the Writers be declared in a try-with-resources block? ########## nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/jaxb/BulletinAdapterStackTraceTest.java: ########## @@ -0,0 +1,46 @@ +/* + * 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.nifi.jaxb; + +import org.apache.nifi.events.BulletinFactory; +import org.apache.nifi.reporting.Bulletin; +import org.apache.nifi.reporting.ComponentType; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +public class BulletinAdapterStackTraceTest { Review Comment: ```suggestion class BulletinAdapterStackTraceTest { ``` -- 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]
