Upserting text field with "null" with configured default value lead to strange update behavior

[SamuelHassine]

Description

Upserting text field with "null" lead to strange update behavior

mutation { threatActorGroupAdd( input: { name: "test test test Sam", description: null, primary_motivation: null } ) { id name description primary_motivation secondary_motivations } }

=> Description was "test test", new description is "test\n".

[labo-flg]

This was due to a default value set for the description field on the Threat Actor Group, in the test conducted.

Default value was indeed "test\n"

[labo-flg]

Analyzing this issue, we found a behavior that might be problematic though (cc @lndrtrbn)

1. add an object with a drescription

mutation {
  threatActorGroupAdd(
    input: {
      name: "AB AB AB"
      primary_motivation: null
      description: "description AB"
    }
  ) {
    id
    name
    description
    primary_motivation
    secondary_motivations
  }
}

2. set a default value for the description, in the customization of the platform.

3. create once again (upsert) the same object with null description

mutation {
  threatActorGroupAdd(
    input: {
      name: "AB AB AB"
      primary_motivation: null
      description: null
    }
  ) {
    id
    name
    description
    primary_motivation
    secondary_motivations
  }
}

=> the value is replaced with the default value.

As discussed with @richard-julien this is probably NOT what we want. We probably want to adapt the upsert method so that if the input is empty but there is an existing value, then we do not update the field even if a default value is set

WDYT @Jipegien ?

[richard-julien]

My opinion.

For me the default value must be considered as null if a data is already available in the database

[Jipegien]

default value has the purpose to replace "null". If there is already an existing value, default value must not be used.

[lndrtrbn]

To be able to fix correctly I need your thought @richard-julien on the scenario below (representing actual behavior of the upsert in octi):

1. I create a new entity (here a threat actor individual)

mutation UpsertTAI {
  threatActorIndividualAdd(
    input: {
      name: "AB AB AB"
      description: "description AB"
    }
  ) {
    id
    name
    description
  }
}

# Result
{
  "data": {
    "threatActorIndividualAdd": {
      "id": "e02a1e24-85de-40e8-b624-d9fbf5489277",
      "name": "AB AB AB",
      "description": "description AB"
    }
  }
}

2. Then I make an upsert on it with description null (no default value is set for the field and it is not required)

mutation UpsertTAI {
  threatActorIndividualAdd(
    input: {
      name: "AB AB AB"
      description: null
    }
  ) {
    id
    name
    description
  }
}

# Result
{
  "data": {
    "threatActorIndividualAdd": {
      "id": "e02a1e24-85de-40e8-b624-d9fbf5489277",
      "name": "AB AB AB",
      "description": "description AB"
    }
  }
}

The null description is not taking in account and we have kept "description AB" is the expected behavior? Or should we have set the description to null?

[richard-julien]

In upsert you have 2 modes. Normal and Full Sync (setup by query header)

• In normal mode Upsert MUST NOT remove any upsertable data. So description must remain "description AB" Labels will be added, etc. If default value is available as the field must be discarded, the default value must be also discarded

• In Full sync mode All upsertable elements must be overriden with the provided value. So description will be replace by null. In this case i think the default value should be applied. (maybe a quick control with the product is needed :))

[lndrtrbn]

So if I'm right the expected behavior, taking into account upsert mode and default value, should be the following:

[lndrtrbn]

I'm taking back this issue. @richard-julien could you validate (or unvalidate) the schema above made on what I understand from your explanation?

~~And I cannot see in the code where we check if we are in normal or full sync~~ Found

[lndrtrbn]

So after discussions with @richard-julien, here the new version of the schema:

So in the case of the bug, the default value should not be taken. Instead we keep null. Because default values must be used only when creating an entity that is not already in the platform. Upsert, which updates an existing entity, should never use default values. @nino-filigran seems good to you also?

[nino-filigran]

Fine for me @lndrtrbn