algoliasearch-client-csharp icon indicating copy to clipboard operation
algoliasearch-client-csharp copied to clipboard

Allow submitting batches that make changes to multiple indexes and multiple types at once

Open DrLeh opened this issue 4 years ago • 3 comments

NOTE: I cannot find documentation whether this functionality is supported by the API. The presence of the "IndexName" property on the BatchOperation class indicates, to me, that any index could be specified and the data could be sent to that index. Since Algolia is schemaless, I personally cannot see a reason why it would be disallowed, but perhaps that IndexName was only intended for replicas of the base index. However, The fact that the Batch() methods are present on the SearchIndex and not on the client object suggests to me that my assumption may be incorrect. If I am incorrect, then this functionality I wish for is impossible and this can be closed.

I was unable to run the unit tests myself, so I'm opening a PR to let the CI test it.

Q A
Bug fix? no
New feature? yes
BC breaks? no
Related Issue
Need Doc update yes

Describe your change

Presently, the Batch() methods require a specific type to be specified. This means you could only batch changes to a single index type at once. This change removes that requirement and allows you to specify a batch across multiple types and indexes.

What problem is this fixing?

Now, you can call a batch update to multiple indexes and multiple data types at once.

Before:

var operation1 = new List<BatchOperation<ObjectToBatch>
{
    new BatchOperation<ObjectToBatch>
    {
        Action = BatchActionType.UpdateObject,
        Body = new ObjectToBatch { ObjectID = "one", Key = "v" }
    }
};
var operation2 = new List<BatchOperation<ObjectToBatch2>
{
    new BatchOperation<ObjectToBatch2>
    {
        Action = BatchActionType.UpdateObject,
        Body = new ObjectToBatch2 { ObjectID = "two", Key = "v" },
    }
};

var batchOneResponse = await _index.BatchAsync(operation1);
batchTwoResponse.Wait();

var batchTwoResponse = await _index.BatchAsync(operation2);
batchTwoResponse.Wait();

Since the two batches would need to be called separately, you cannot do atomic changes to two typed indexes at once, which could lead to issues if one update fails.

Now: e.g.

var operations = new List<BatchOperation>
{
    new BatchOperation
    {
        Action = BatchActionType.UpdateObject,
        Body = new ObjectToBatch { ObjectID = "one", Key = "v" },
        IndexName = Index1Name,
    },
    new BatchOperation
    {
        Action = BatchActionType.UpdateObject,
        Body = new ObjectToBatch2 { ObjectID = "two", Key = "v" },
        IndexName = Index2Name,
    },
};

var batchTwoResponse = await _index.BatchAsync(operations);
batchTwoResponse.Wait();

This allows you to make atomic changes to arbitrary indexes.

DrLeh avatar Jan 19 '21 15:01 DrLeh

Hello,

Thanks a lot for submitting your PR, we appreciate the time you passed working on it. However, I am afraid we won't be able to merge it -as it- because the changes you proposed are breaking in terms of API (it's breaking the BatchOperation class) and we try to avoid that as much as possible.

Before we put more work on this PR. Could you please tell me if this existing method from SearchClient would work for you? If I understood correctly your use-case, this method is a perfect fit.

https://github.com/algolia/algoliasearch-client-csharp/blob/1d62daefc81f2883d9a817337faa37ad100a39bc/src/Algolia.Search.Test/EndToEnd/Client/MultipleOperationsTest.cs#L52-L81

If yours indices have different schemas, instead of having MultipleOperationClass you could have object or JSONObject to batch different kind of classes to different indices.

Ant-hem avatar Jan 27 '21 20:01 Ant-hem

@Ant-hem Are you suggesting that instead of BatchOperation<MultipleOperationClass> from your example, we'd do BatchOperation<object> and we'd just cast our types (e.g. ObjectToBatch and ObjectToBatch2) to an object instead?

That could potentially work for our case.

Also, I could propose this change which should leave the existing BatchOperation<T> unchanged for back-compat:

    /// <summary>
    /// Represent an algolia batch object
    /// </summary>
    public class BatchOperation<T>: BatchOperation
        where T : class
    {
        /// <summary>
        /// Body of the batch, objects you want to send, typed
        /// </summary>
        public new T Body
        {
            get
            {
                return base.Body as T;
            }
            set
            {
                base.Body = value;
            }
        }
    }

DrLeh avatar Jan 28 '21 15:01 DrLeh

@Ant-hem casting all my classes to object in the batch does seem to work, so that's a good workaround and solves my use-case. I do think that it would be better from a design standpoint to have the C# code match the API though. Since the API is type-indiscriminate, imo it would be good to have C# code paths that are also not type-dependent, as in this PR.

Having type-specific code is very helpful, but for something like this, a forced object cast is a bit of a code smell for C#.

Thanks for your help!

DrLeh avatar Jan 28 '21 16:01 DrLeh

A new Client has been release (7.0) and still represent the exact models and routes exposed by our APIs. We will not be able to merge your PR that will force Algolia to make changes in the Search API.

morganleroi avatar Feb 21 '24 12:02 morganleroi