mbeckerle commented on a change in pull request #539: URL: https://github.com/apache/daffodil/pull/539#discussion_r623201424
########## File path: daffodil-core/src/test/scala/org/apache/daffodil/api/TestParseIndividualMessages.scala ########## @@ -0,0 +1,230 @@ +/* + * 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.daffodil.api + +import org.apache.daffodil.Implicits.intercept +import org.apache.daffodil.io.SocketPairTestRig +import org.apache.daffodil.util.SchemaUtils +import org.apache.daffodil.util.TestUtils +import org.junit.Assert._ +import org.junit.Test + +import java.io.InputStream +import java.io.OutputStream +import scala.concurrent.Await +import scala.concurrent.ExecutionContext.Implicits.global +import scala.concurrent.Future +import scala.concurrent.TimeoutException +import scala.concurrent.duration.DurationInt +import scala.xml.Node + + +/** + * Shows that we can parse exactly 1 message from a TCP network socket + * without blocking for bytes past the end of the messsage. + * + * This only works for DFDL schemas of formats that are specified length. + */ +class TestParseIndividualMessages { + + import SocketPairTestRig._ Review comment: Highlight: This socket pair test rig is an important addition to test capability. It spins up actual network connections so that we can simulate reading from a socket that has no data, and block, later add data to cause it to unblock, etc. ########## File path: daffodil-core/src/test/scala/org/apache/daffodil/api/TestParseIndividualMessages.scala ########## @@ -0,0 +1,230 @@ +/* + * 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.daffodil.api + +import org.apache.daffodil.Implicits.intercept +import org.apache.daffodil.io.SocketPairTestRig +import org.apache.daffodil.util.SchemaUtils +import org.apache.daffodil.util.TestUtils +import org.junit.Assert._ +import org.junit.Test + +import java.io.InputStream +import java.io.OutputStream +import scala.concurrent.Await +import scala.concurrent.ExecutionContext.Implicits.global +import scala.concurrent.Future +import scala.concurrent.TimeoutException +import scala.concurrent.duration.DurationInt +import scala.xml.Node + + +/** + * Shows that we can parse exactly 1 message from a TCP network socket + * without blocking for bytes past the end of the messsage. + * + * This only works for DFDL schemas of formats that are specified length. + */ +class TestParseIndividualMessages { + + import SocketPairTestRig._ + + // + // DFDL schema for element e1 which occupies exactly 4 bytes. + // + val exactly4ByteSch = SchemaUtils.dfdlTestSchema( + <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>, + <dfdl:format representation="binary" byteOrder="bigEndian" binaryNumberRep="binary" ref="tns:GeneralFormat"/>, + <xs:element name="e1" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="4"/>) + + /** + * Test shows that at least for simple fixed-length data, Daffodil parse returns + * without requiring more bytes to be read than the exact length required. + */ + @Test def testDaffodilParseFromNetwork1(): Unit = { + val sptr = new SocketPairTestRig { + override def test(pos: OutputStream, cis: InputStream): Unit = { + + val dp = TestUtils.compileSchema(exactly4ByteSch) + + // + // Write exactly 4 bytes to producer network stream + // + pos.write("1234".getBytes) + pos.flush() + + // + // Daffodil parse element e1 from consumer input stream + // + // If we need more than 4 bytes to successfully parse (we shouldn't for this schema) + // then this will hang, because only 4 bytes are in fact available. + // + // Caution: if debugging, this will timeout if you stop inside here! + // + val (pr: DFDL.ParseResult, xml: Node) = + withTimeout("Daffodil parse") { + TestUtils.runDataProcessorOnInputStream(dp, cis, areTracing = false) + } + + assertFalse(pr.isError) + assertEquals("1234", xml.text) + } + } + sptr.run() + } + + /** + * Test shows that the isAtEnd method does not block looking for additional data. Review comment: Comment now wrong. The underlying code isn't calling isAtEnd. This shows that reading 4 bytes does not block looking for additional data. This test is identical to the test above now, save that last assert. Combine these two tests. ########## File path: daffodil-io/src/main/scala/org/apache/daffodil/io/StringDataInputStreamForUnparse.scala ########## @@ -66,6 +64,9 @@ final class StringDataInputStreamForUnparse override def getSomeString(nChars: Long,finfo: FormatInfo): Maybe[String] = dis.getSomeString(nChars, finfo) override def getString(nChars: Long,finfo: FormatInfo): Maybe[String] = dis.getString(nChars, finfo) + // $COVERAGE-OFF$ Nothing should be calling these. Review comment: Highlight: This coverage-on/off stuff works. I got the coverage on this change set to pass codecov's criteria by putting these things around code that shouldn't have coverage. ########## File path: daffodil-io/src/test/scala/org/apache/daffodil/io/FormatInfoForUnitTest.scala ########## @@ -89,3 +89,28 @@ class FormatInfoForUnitTest private () } } } + Review comment: Just moved this. It was in the wrong place. ########## File path: daffodil-japi/src/main/scala/org/apache/daffodil/japi/Daffodil.scala ########## @@ -490,10 +490,15 @@ class DataLocation private[japi] (dl: SDataLocation) { override def toString() = dl.toString /** - * Determine if this data location is at the end of the input data + * Determine if this data location is known to be at the end of the data stream. Review comment: Revert comment. isAtEnd has the original blocking behavior. It's just deprecated now because that functionality is so problematic. ########## File path: daffodil-lib/src/test/scala/org/apache/daffodil/util/TestCoroutines.scala ########## @@ -0,0 +1,173 @@ +/* Review comment: Highlight: Just adds back the missing unit tests for Coroutines library. Somehow these had gotten lost. This fixes DAFFODIL-2503 ########## File path: daffodil-lib/src/main/scala/org/apache/daffodil/util/Coroutines.scala ########## @@ -98,6 +103,21 @@ protected def run(): Unit } + /** Review comment: Another minor tweak to coroutines. This pattern comes up everywhere coroutines are used. ########## File path: daffodil-io/src/test/scala/org/apache/daffodil/io/SocketPairTestRig.scala ########## @@ -0,0 +1,246 @@ +/* + * 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.daffodil.io + +import org.apache.daffodil.Implicits.intercept +import org.apache.daffodil.io.SocketPairTestRig.timeLimit +import org.apache.daffodil.io.SocketPairTestRig.withTimeout +import org.junit.Assert.assertEquals +import org.junit.Assert.fail +import org.junit.Test + +import java.io.InputStream +import java.io.OutputStream +import java.net.ServerSocket +import java.net.Socket +import scala.concurrent.Await +import scala.concurrent.ExecutionContext.Implicits.global +import scala.concurrent.Future +import scala.concurrent.TimeoutException +import scala.concurrent.duration.DurationInt +import scala.concurrent.duration.FiniteDuration +import scala.util.Try + + +/** + * Test rig sets up TCP socket unidirectional between a producer + * output stream, and a consumer input stream. + */ +abstract class SocketPairTestRig { Review comment: Highlight: This is the new test rig. ########## File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala ########## @@ -45,13 +45,17 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin def this(cause: Throwable) = this(1, cause, null) Review comment: Highlight: These changes are just to improve codecov results. No functional change here. Partial fix for DAFFODIL-2509 ########## File path: daffodil-lib/src/main/scala/org/apache/daffodil/util/Coroutines.scala ########## @@ -73,8 +71,12 @@ * Call when a co-routine resumes another (to provide a result of some sort) * and then terminates. The coroutine calling this must return from the run() * method immediately after calling this. + * Review comment: Highlight: minor tweak to Coroutines to allow different types of data flowing on each path between two coroutines. -- 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. For queries about this service, please contact Infrastructure at: [email protected]
