Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CollectionSyncUpTarget is relying on Id not ExternalId #2337

Open
aureosouza opened this issue Aug 10, 2022 · 8 comments
Open

CollectionSyncUpTarget is relying on Id not ExternalId #2337

aureosouza opened this issue Aug 10, 2022 · 8 comments

Comments

@aureosouza
Copy link

aureosouza commented Aug 10, 2022

  1. Version of Mobile SDK Used: 10.1.0
  2. Issue found in Native App or Hybrid App: Native
  3. OS Version: All
  4. Device: All
  5. Steps to reproduce:

Some steps in debugging process that may help with investigation:

5.1) SyncUp with target:

{
   "androidImpl":"com.salesforce.androidsdk.mobilesync.target.CollectionSyncUpTarget",
   "idFieldName":"Id",
   "modificationDateFieldName":"LastModifiedDate",
   "createFieldlist":[
      "Field01__c",
      "Field02__c",
      "Field03__c",
   ],
   "externalIdFieldName":"ExternalId__c",
   "maxBatchSize":200
}

5.2) In com.salesforce.androidsdk.mobilesync.target.BatchSyncUpTarget syncUpRecords has records:

[{
   "Field01__c":"Value01",
   "Field02__c":"Value02",
   "Field03__c":"Value03",
   "__locally_created__":false,
   "__locally_deleted__":false,
   "__local__":true,
   "__locally_updated__":true,
   "attributes":{
      "type":"Soup01__c"
   },
   "ExternalId__c":"ExternalIdValue123",
   "Id":"local_47494233522985",
   "_soupEntryId":411,
   "_soupLastModifiedDate":1660041375340
}]

And mergeMode is OVERWRITE

5.3) In com.salesforce.androidsdk.mobilesync.target.CompositeRequestHelper the method sendAsCollectionRequests has request:

{
   "method":"PATCH",
   "url":"\/services\/data\/v55.0\/composite\/sobjects",
   "body":{
      "allOrNone":false,
      "records":[
         {
            "attributes":{
               "type":"Soup01__c"
            },
            "Field01__c":"Value01",
            "Field02__c":"Value02",
            "Field03__c":"Value03",
            "Id":"local_47494233522985"
         }
      ]
   }
}

We believe here the api to be chosen should be the composite/sobjects/SobjectName/ExternalIdFieldName from here, and the ExternalId__c should be passed . Since we should rely only on externalId not the SF Id, since this record is created in mobile app.

Obs. 1: We noticed in some cases the correct api composite/sobjects/SobjectName/ExternalIdFieldName is called on creation of the record and syncUp, but when updating the value it syncs up with the api composite/sobjects which relies on Id not externalId and we are getting MALFORMED_ID response error from SF.

Obs. 2: We noticed as well that the referenceId is always passed from PR as this is a part of sObject Tree not sObject Collections which could be possible issue:

RecordRequest request = buildRequestForRecord(record, fieldlist);
if (request != null) {
   request.referenceId = id;
   recordRequests.add(request);
}
  1. Actual behavior: Using api sobjects which relies on Id not externalId
  2. Expected Behavior: Use api sobjects/SobjectName/ExternalIdFieldName that does not rely on Id
  3. Error Log:
[
   {
      "success":false,
      "errors":[
         {
            "statusCode":"MALFORMED_ID",
            "message":"Record ID: id value of incorrect type: local_47494233522985",
            "fields":[
               "Id"
            ]
         }
      ]
   }
]
@gkotula
Copy link
Contributor

gkotula commented Aug 10, 2022

Hi @aureosouza, could you provide your entire test userstore.json and usersyncs.json instead of just the sync target? That would greatly help in debugging this, thanks!

@aureosouza
Copy link
Author

Sending you the globalsyncs.json, let me know if this is enough:

{  "syncs":
[
  {
    "syncName": "syncDownSoup01",
    "syncType": "syncDown",
    "soupName": "Soup01__c",
    "target": {"type":"soql",
      "query":"SELECT ExternalId__c, Field01__c, Field02__c, Field03__c FROM Soup01__c WHERE Field01__c >= LAST_N_DAYS:30"},
      "options": {"mergeMode":"LEAVE_IF_CHANGED"}
    },
  {
    "syncName": "syncUpSoup01",
    "syncType": "syncUp",
    "soupName": "Soup01__c",
    "target": {"idFieldName": "Id",
               "externalIdFieldName": "ExternalId__c",
      "createFieldlist":[
        "ExternalId__c",
        "Field01__c",
        "Field02__c",
        "Field03__c"]},
    "options": {"fieldlist":["Id",
      "ExternalId__c",
      "Field01__c",
      "Field02__c",
      "Field03__c"],
      "mergeMode":"OVERWRITE"}
  }
]
}

@aureosouza
Copy link
Author

We suspect it could have something to do with the decision making of which requestType and eventually the API to be choosen here in BatchSyncUpTarget:

image

Because it's seems only when isCreate is true we are able to set UPSERT (which eventually chooses API composite/sobjects/SobjectName/ExternalIdFieldName in getCollectionRequest in CompositeRequestHelper), if not it falls to UPDATE (which chooses previous API composite/sobjects). And we believe we should always choose UPSERT if we have externalId as the primary id, let us know if this helps with the investigation @gkotula.

@wmathurin
Copy link
Contributor

The way it is currently implemented and documented is to do an upsert instead of a create for locally created records if there is an external id field name in the target definition that is populated on the record.
When you update a record, the Id field should be populated already and therefore doing and update should work fine even if the external id field itself is updated. Why do you think it should be an upsert in that case?

@aureosouza
Copy link
Author

@wmathurin the issue we are facing is that we did 2 sync up, one creating and another editing, but no sync down. So we do not have yet the Id from Sales Force, that's why we believe that we should still perform an UPSERT with the sobjects/SobjectName/ExternalIdFieldName API relying only on externalId. This was not an issue with the previous versions.

The reproduction steps and the error output returned by Sales Force API are all described above and it's critical scenario in our application with the new version 10.1.0, please let us know if we can provide any more info.

@wmathurin
Copy link
Contributor

wmathurin commented Aug 18, 2022

There should be no need to sync down. After a sync up, we update the id on the local record with the id returned by the server. See this code which runs for both BatchSyncUpTarget and the new CollectionSyncUpTarget.

To be sure I follow, you run the sync up with the config you pasted above twice.

The first time after the record was locally created and the second time after it was locally updated

  • after the first sync up (which does an upsert to the server), you noticed that the server id is not saved back into the local record
  • that causes the second sync up (which does an update on the server) to fail with the MALFORMED_ID error code.

Before 10.1.0 you were using BatchSyncUpTarget, and it was working correctly?

@wmathurin
Copy link
Contributor

I have been testing locally and see the server id saved back on the record (in SmartStore) after the upsert as expected.
Are you reading the record back from SmartStore between the first and second sync up?

@aureosouza
Copy link
Author

aureosouza commented Aug 23, 2022

Thanks for info, we confirmed SDK was saving the SF Id after the first syncUp. Issue seems to be that when upserting locally the Id was not being passed, only the externalId so SDK was updating the value of Id to null (which seems like wrong behaviour as well in upsert function in com.salesforce.androidsdk.smartstore.store.SmartStore). Then the second syncUp was failing. If we pass the Id we do not get the error anymore, but we still believe that if we have externalId, this should always be the primary key for both local and remote upserts and SDK should not depend on the Id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants