Clean up and avoid reading the file twice if no row limit is set.

opencadc · Mar 11, 2024 · a9ab255 · a9ab255
1 parent 29058e6
commit a9ab255
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 29 deletions.
diff --git a/cadc-tap-server/src/main/java/ca/nrc/cadc/tap/upload/JDOMVOTableParser.java b/cadc-tap-server/src/main/java/ca/nrc/cadc/tap/upload/JDOMVOTableParser.java
@@ -118,35 +118,43 @@ private void init()
             VOTableReader r = new VOTableReader();
             VOTableDocument doc = r.read(upload.uri.toURL().openStream());
             VOTableResource vr = doc.getResourceByType("results");
-
             this.votable = vr.getTable();
-
             this.tableName = upload.tableName;
             if (!tableName.toUpperCase().startsWith(SCHEMA)) {
                 tableName = SCHEMA + "." + tableName;
             }
         }
     }
 
+    /**
+     * Ensure the Upload table conforms to specified limitations, if any.  This will only read through the file if
+     * the Upload table file falls into the acceptable size.
+     * @throws IOException  If any of the limitations are exceeded.
+     */
     void verifyUploadTable() throws IOException {
-        if (uploadLimits != null) {
+        // Only proceed if a size limitation is set.
+        if (uploadLimits != null && uploadLimits.byteLimit != null) {
             final VOTableReader voTableReader = new VOTableReader();
             try (final ByteCountInputStream byteCountInputStream =
-                         new ByteCountInputStream(upload.uri.toURL().openStream(), uploadLimits.getByteLimit())) {
+                         new ByteCountInputStream(upload.uri.toURL().openStream(), uploadLimits.byteLimit)) {
                 final VOTableDocument doc = voTableReader.read(byteCountInputStream);
                 final VOTableResource vr = doc.getResourceByType("results");
                 final VOTableTable voTableTable = vr.getTable();
 
-                if (voTableTable.getFields().size() > uploadLimits.getColumnLimit()) {
-                    throw new IOException("Column count exceeds maximum of " + uploadLimits.getColumnLimit());
+                if (uploadLimits.columnLimit != null && voTableTable.getFields().size() > uploadLimits.columnLimit) {
+                    throw new IOException("Column count exceeds maximum of " + uploadLimits.columnLimit);
                 }
 
-                int counter = 0;
-                for (final Iterator<List<Object>> iterator = voTableTable.getTableData().iterator(); iterator.hasNext(); ) {
-                    if (++counter > uploadLimits.getRowLimit()) {
-                        throw new IOException("Row count exceeds maximum of " + uploadLimits.getRowLimit());
-                    } else {
-                        iterator.next();
+                // If no row limit has been set, avoid reading through the file.
+                if (uploadLimits.rowLimit != null) {
+                    int counter = 0;
+                    for (final Iterator<List<Object>> iterator = voTableTable.getTableData().iterator();
+                         iterator.hasNext();) {
+                        if (++counter > uploadLimits.rowLimit) {
+                            throw new IOException("Row count exceeds maximum of " + uploadLimits.rowLimit);
+                        } else {
+                            iterator.next();
+                        }
                     }
                 }
             }

diff --git a/cadc-tap-server/src/main/java/ca/nrc/cadc/tap/upload/UploadLimits.java b/cadc-tap-server/src/main/java/ca/nrc/cadc/tap/upload/UploadLimits.java
@@ -69,25 +69,13 @@
 package ca.nrc.cadc.tap.upload;
 
 public class UploadLimits {
-    private final Long byteLimit;
-    private final Integer rowLimit;
-    private final Integer columnLimit;
+    final Long byteLimit;
+    final Integer rowLimit;
+    final Integer columnLimit;
 
     public UploadLimits(Long byteLimit, Integer rowLimit, Integer columnLimit) {
         this.byteLimit = byteLimit;
         this.rowLimit = rowLimit;
         this.columnLimit = columnLimit;
     }
-
-    long getByteLimit() {
-        return byteLimit == null ? Long.MAX_VALUE : byteLimit;
-    }
-
-    int getRowLimit() {
-        return rowLimit == null ? Integer.MAX_VALUE : rowLimit;
-    }
-
-    int getColumnLimit() {
-        return columnLimit == null ? Integer.MAX_VALUE : columnLimit;
-    }
 }
diff --git a/cadc-tap-server/src/test/java/ca/nrc/cadc/tap/upload/JDOMVOTableParserTest.java b/cadc-tap-server/src/test/java/ca/nrc/cadc/tap/upload/JDOMVOTableParserTest.java
@@ -112,7 +112,7 @@ public void testUploadValidateRowLimit() throws Exception {
 
         final UploadTable uploadTable = new UploadTable("test2", "jobid2", uploadFile.toURI());
 
-        testSubject.setUpload(uploadTable, new UploadLimits(null, 2, 2));
+        testSubject.setUpload(uploadTable, new UploadLimits(5L * 1024L * 1024L, 2, null));
 
         try {
             testSubject.verifyUploadTable();
@@ -132,7 +132,7 @@ public void testUploadValidateColumnLimit() throws Exception {
 
         final UploadTable uploadTable = new UploadTable("test3", "jobid3", uploadFile.toURI());
 
-        testSubject.setUpload(uploadTable, new UploadLimits(null, null, 4));
+        testSubject.setUpload(uploadTable, new UploadLimits(5L * 1024L * 1024L, null, 4));
 
         try {
             testSubject.verifyUploadTable();