Skip to content

Commit 0ca703a

Browse files
committed
fix(config): support numeric values for TabularIngestSizeLimit, improve validation and logging
Enhanced `getTabularIngestSizeLimits` to accept numeric values in addition to strings for size limits. Improved validation to handle invalid types, numbers, or decimal values. Updated related tests and configuration documentation. This was done because the limitation to long numbers as string was artificial. Users can choose which way they like best. Also, the data migration uses numbers, so this lead to errors.
1 parent 586cac3 commit 0ca703a

File tree

3 files changed

+53
-20
lines changed

3 files changed

+53
-20
lines changed

doc/sphinx-guides/source/installation/config.rst

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4517,7 +4517,11 @@ Using a JSON-based setting, you can set a global default and per-format limits f
45174517

45184518
(In previous releases of Dataverse, a colon-separated form was used to specify per-format limits, such as ``:TabularIngestSizeLimit:Rdata``, but this is no longer supported. Now JSON is used.)
45194519

4520-
The expected JSON is an object with key/value pairs like the following. Format names are case-insensitive, and all fields are optional. The size limits must be strings with double quotes around them (e.g. ``"10"``) rather than numbers (e.g. ``10``).
4520+
The expected JSON is an object with key/value pairs like the following.
4521+
Format names are case-insensitive, and all fields are optional (an empty JSON object equals not restricted).
4522+
The size limits must be whole numbers, either presented as strings with double quotes around them (e.g. ``"10"``) or numeric values (e.g. ``10`` or ``10.0``).
4523+
Note that decimal numbers like ``10.5`` are invalid.
4524+
Any invalid setting will temporarily disable tabular ingest until corrected.
45214525

45224526
.. code:: json
45234527

src/main/java/edu/harvard/iq/dataverse/util/SystemConfig.java

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -527,19 +527,30 @@ public Map<String, Long> getTabularIngestSizeLimits() {
527527
limitsMap.put(TABULAR_INGEST_SIZE_LIMITS_DEFAULT_KEY, -1L);
528528

529529
for (String formatName : limits.keySet()) {
530-
// We deliberatly do not validate the formatNames here for backward compatibility.
531-
// But we transform to lowercase here, so the casing doesn't matter for lookups.
532530
String lowercaseFormatName = formatName.toLowerCase();
533531

534532
try {
535-
Long sizeOption = Long.valueOf(limits.getString(formatName));
533+
JsonValue value = limits.get(formatName);
534+
Long sizeOption;
535+
536+
// We want to be able to use either numbers or string values, so detect which one it is.
537+
if (value.getValueType() == JsonValue.ValueType.STRING) {
538+
sizeOption = Long.valueOf(limits.getString(formatName));
539+
} else if (value.getValueType() == JsonValue.ValueType.NUMBER) {
540+
sizeOption = limits.getJsonNumber(formatName).longValueExact();
541+
} else {
542+
logger.warning("Invalid value type for format " + formatName + ": expected string or number");
543+
logger.warning("Disabling all tabular ingest completely until fixed!");
544+
return Map.of(TABULAR_INGEST_SIZE_LIMITS_DEFAULT_KEY, 0L);
545+
}
546+
536547
limitsMap.put(lowercaseFormatName, sizeOption);
537-
} catch (ClassCastException cce) {
538-
logger.warning("Could not convert " + SettingsServiceBean.Key.TabularIngestSizeLimit + " to long from JSON integer. You must provide the long number as string (use quotes) for format " + formatName);
548+
} catch (NumberFormatException nfe) {
549+
logger.warning("Could not convert " + SettingsServiceBean.Key.TabularIngestSizeLimit + " to long for format " + formatName + " (not a valid number)");
539550
logger.warning("Disabling all tabular ingest completely until fixed!");
540551
return Map.of(TABULAR_INGEST_SIZE_LIMITS_DEFAULT_KEY, 0L);
541-
} catch (NumberFormatException nfe) {
542-
logger.warning("Could not convert " + SettingsServiceBean.Key.TabularIngestSizeLimit + " to long for format " + formatName + " (not a number)");
552+
} catch (ArithmeticException ae) {
553+
logger.warning("Number too large or has fractional part for format " + formatName);
543554
logger.warning("Disabling all tabular ingest completely until fixed!");
544555
return Map.of(TABULAR_INGEST_SIZE_LIMITS_DEFAULT_KEY, 0L);
545556
}

src/test/java/edu/harvard/iq/dataverse/util/SystemConfigTest.java

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@
88
import org.junit.jupiter.api.extension.ExtendWith;
99
import org.junit.jupiter.params.ParameterizedTest;
1010
import org.junit.jupiter.params.provider.CsvSource;
11+
import org.junit.jupiter.params.provider.ValueSource;
1112
import org.mockito.InjectMocks;
1213
import org.mockito.Mock;
1314
import org.mockito.junit.jupiter.MockitoExtension;
15+
import org.springframework.beans.factory.annotation.Value;
1416

1517
import java.util.Map;
1618

@@ -202,10 +204,10 @@ void testGetTabularIngestSizeLimitsWithSingleInvalidValue() {
202204
assertEquals(0L, (long) result.get(SystemConfig.TABULAR_INGEST_SIZE_LIMITS_DEFAULT_KEY));
203205
}
204206

205-
@Test
206-
void testGetTabularIngestSizeLimitsWithJsonButUnsupportedJsonInt() {
207+
@ParameterizedTest
208+
@ValueSource(strings = {"", "{ invalid: }"})
209+
void testGetTabularIngestSizeLimitsWithInvalidJson(String invalidJson) {
207210
// given
208-
String invalidJson = "{\"default\": 0}";
209211
doReturn(invalidJson).when(settingsService).getValueForKey(SettingsServiceBean.Key.TabularIngestSizeLimit);
210212

211213
// when
@@ -216,10 +218,10 @@ void testGetTabularIngestSizeLimitsWithJsonButUnsupportedJsonInt() {
216218
assertEquals(0L, (long) result.get(SystemConfig.TABULAR_INGEST_SIZE_LIMITS_DEFAULT_KEY));
217219
}
218220

219-
@Test
220-
void testGetTabularIngestSizeLimitsWithInvalidJson() {
221+
@ParameterizedTest
222+
@ValueSource(strings = {"[]", "{[{\"tsv\": 1}]}", "{ \"invalid\": \"foobar\" }", "{ \"tsv\": 1.5}", "{ \"tsv\": \"1.0\"}"})
223+
void testGetTabularIngestSizeLimitsWithInvalidNumbersInValidJson(String invalidJson) {
221224
// given
222-
String invalidJson = "{invalid:}";
223225
doReturn(invalidJson).when(settingsService).getValueForKey(SettingsServiceBean.Key.TabularIngestSizeLimit);
224226

225227
// when
@@ -230,17 +232,33 @@ void testGetTabularIngestSizeLimitsWithInvalidJson() {
230232
assertEquals(0L, (long) result.get(SystemConfig.TABULAR_INGEST_SIZE_LIMITS_DEFAULT_KEY));
231233
}
232234

233-
@Test
234-
void testGetTabularIngestSizeLimitsWithInvalidNumberInValidJson() {
235+
@ParameterizedTest
236+
@ValueSource(strings = {"{ \"tSv\": 1.0 }", "{ \"tsv\": \"1\"}", "{ \"tsv\": 1 }"})
237+
void testGetTabularIngestSizeLimitsWithValidNumbersInValidJson(String validConfig) {
235238
// given
236-
String invalidJson = "{\"csv\": \"this-is-not-a-number\", \"tSv\": \"10000\"}";
237-
doReturn(invalidJson).when(settingsService).getValueForKey(SettingsServiceBean.Key.TabularIngestSizeLimit);
239+
doReturn(validConfig).when(settingsService).getValueForKey(SettingsServiceBean.Key.TabularIngestSizeLimit);
238240

239241
// when
240242
Map<String, Long> result = systemConfig.getTabularIngestSizeLimits();
241243

242244
// then
243-
assertEquals(1, result.size());
244-
assertEquals(0L, (long) result.get(SystemConfig.TABULAR_INGEST_SIZE_LIMITS_DEFAULT_KEY));
245+
assertEquals(2, result.size());
246+
assertEquals(-1L, (long) result.get(SystemConfig.TABULAR_INGEST_SIZE_LIMITS_DEFAULT_KEY));
247+
assertEquals(1L, (long) result.get("tsv"));
248+
}
249+
250+
@ParameterizedTest
251+
@ValueSource(strings = {"{ \"tSv\": 429496729600.0 }", "{ \"tsv\": \"429496729600\"}", "{ \"tsv\": 429496729600 }"})
252+
void testGetTabularIngestSizeLimitsWithValidLongNumbers(String validConfig) {
253+
// given
254+
doReturn(validConfig).when(settingsService).getValueForKey(SettingsServiceBean.Key.TabularIngestSizeLimit);
255+
256+
// when
257+
Map<String, Long> result = systemConfig.getTabularIngestSizeLimits();
258+
259+
// then
260+
assertEquals(2, result.size());
261+
assertEquals(-1L, (long) result.get(SystemConfig.TABULAR_INGEST_SIZE_LIMITS_DEFAULT_KEY));
262+
assertEquals(429496729600L, (long) result.get("tsv"));
245263
}
246264
}

0 commit comments

Comments
 (0)