USACE / cwms-data-api

Corps Water Management System RESTful Data Service
MIT License
14 stars 14 forks source link

Timeseries POST gives Null Pointer for null Values #876

Open krowvin opened 2 months ago

krowvin commented 2 months ago

Using POST for Timeseries to set NULL values

Background

Working with @inguyen314 we were discussing the finer points of how to replace-all values in a timeseries with null values.

The idea being you want to write some data from a future forecast, but other data you'd want to make sure was null. Or perhaps the forecast changes and those values are now null for those dates.

Agreement

We both agreed that one should be able to interlace null values in a valid POST request where you may also want to write valid values.

Workaround

One could of course call the DELETE method and specify the date range.

Reasons not to?

But in our humble opinion Why not both?

Example

Doing the following results in a NULL POINTER exception in the logs:

  const payloadDeleteGraysPt = {
                    "name": "Grays Pt-Mississippi.Stage.Inst.~1Day.0.netmiss-fcst",
                    "office-id": "MVS",
                    "units": "ft",
                    "values": [
                        [
                            getDateWithTimeSet(0, 6, 0),
                            null,
                            0
                        ],
                        [
                            getDateWithTimeSet(1, 6, 0),
                            8675309,
                            0
                        ],
                        [
                            getDateWithTimeSet(2, 6, 0),
                            8675309,
                            0
                        ],
                        [
                            getDateWithTimeSet(3, 6, 0),
                            null,
                            0
                        ],
                        [
                            getDateWithTimeSet(4, 6, 0),
                            null,
                            0
                        ],
                        [
                            getDateWithTimeSet(5, 6, 0),
                            null,
                            0
                        ],
                        [
                            getDateWithTimeSet(6, 6, 0),
                            null,
                            0
                        ],
                    ]

This was done in javascript. I do not have a CURL command readily available but here is the Vanilla JS done to test this:

async function deleteTS() {
    // Create an array of promises to handle multiple payloads
    let promises = payloadDelete.map(ts_payload => {
        return fetch("[https://wm.mvs.ds.usace.army.mil/mvs-data/timeseries?store-rule=REPLACE%20ALL"](https://wm.mvs.ds.usace.army.mil/mvs-data/timeseries?store-rule=REPLACE%20ALL%22), {
            method: "POST",
            headers: {
                "accept": "*/*",
                "Content-Type": "application/json;version=2",
            },
            body: JSON.stringify(ts_payload)
        }).then(async r => {
            // Get the response message and status
            const message = await r.text();
            const status = r.status;
            return { 'message': message, 'status': status };
        }).catch(error => {
            // Handle fetch errors
            return { 'message': error.message, 'status': 'fetch_error' };
        });
    });

    // Wait for all promises to resolve
    const return_values = await Promise.all(promises);
    console.log("Return values from deleteTS:", return_values);

    // Check for errors based on status and message content
    const has_errors = return_values.some(v => v.status !== 200 || v.message.includes("error") || v.message.includes("fail"));
    return has_errors;
}
rma-rripken commented 1 month ago

The issue is this line: https://github.com/USACE/cwms-data-api/blob/297c7929f050895819df0dea6d3bfba1bb16bf8a/cwms-data-api/src/main/java/cwms/cda/data/dao/TimeSeriesDaoImpl.java#L1146C1-L1146C50

Its trying to put a null into a double[].

krowvin commented 1 month ago

@rma-rripken I was talking to Daniel about this

Does it make sense for POST to ONLY be able to create NEW timeseries values and not ALSO update existing with a null?

Because it WILL write over existing values (POST will) if there is a value given.

But I think we've established this is OK because you can update other values with values. It's just the null that doesn't work.

So then i'm not sure what the purpose of PATCH is?

msweier commented 1 month ago

Didn't realize this was a duplicate. Should be possible with missing value and missing quality code. value = -340282346638528859811704183484516925440 and quality code = 5 for missing https://github.com/USACE/cwms-data-api/issues/910

MikeNeilson commented 1 month ago

Yeah, try again with the latest RC were this was fixed.

MikeNeilson commented 1 month ago

@rma-rripken I was talking to Daniel about this

Does it make sense for POST to ONLY be able to create NEW timeseries values and not ALSO update existing with a null?

Because it WILL write over existing values (POST will) if there is a value given.

But I think we've established this is OK because you can update other values with values. It's just the null that doesn't work.

So then i'm not sure what the purpose of PATCH is?

Honestly we mostly put it in because it sematically made sense. I think there is a distinction that PATCH WILL NOT create the time series if it doesn't exist though.