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

Offset(..) throws exception for subsequently calls #54

Open
Leh2 opened this issue Sep 20, 2022 · 15 comments
Open

Offset(..) throws exception for subsequently calls #54

Leh2 opened this issue Sep 20, 2022 · 15 comments

Comments

@Leh2
Copy link

Leh2 commented Sep 20, 2022

When paging a given resource, it should be allowed to call Offset more than once. When doing it today we get the this exception: System.ArgumentException: An item with the same key has already been added. Key: offset.

To reproduce, use this code and make sure more than 100 events is present (or lower pageSize)

var events = new List<Event>();

const int pageSize = 100;
string offset = null;

do
{
    var result = await Event
        .List()
        .Offset(offset)
        .Limit(pageSize)
        .RequestAsync();

    offset = result.NextOffset;

    events.AddRange(result.List.Select(x => x.Event));
} while (offset != null);

return events;
@flixius
Copy link

flixius commented Oct 20, 2022

I have exactly the same issue. @Leh2
Did you find a solution?
I tried to serialize the request but unfortunately it's not serializable.

A method like .ToUrl() would help, because then I could create a new instance of the class easily. :-(

@Birchsen
Copy link

I'm experiencing the exact same issue as @Leh2 :-(

@flixius
Copy link

flixius commented Nov 18, 2022

I'm experiencing the exact same issue as @Leh2 :-(

I spent hours on this and also contacted the chargebee support as a paid user.

Finally I found a solution which worked for me. Here is the code:

public static class DeepCopyExtension
    {
        private static readonly MethodInfo CloneMethod = typeof(Object).GetMethod("MemberwiseClone", BindingFlags.NonPublic | BindingFlags.Instance);

        public static bool IsPrimitive(this Type type)
        {
            if (type == typeof(String)) return true;
            return (type.IsValueType & type.IsPrimitive);
        }

        public static Object Copy(this Object originalObject)
        {
            return InternalCopy(originalObject, new Dictionary<Object, Object>(new ReferenceEqualityComparer()));
        }
        private static Object InternalCopy(Object originalObject, IDictionary<Object, Object> visited)
        {
            if (originalObject == null) return null;
            var typeToReflect = originalObject.GetType();
            if (IsPrimitive(typeToReflect)) return originalObject;
            if (visited.ContainsKey(originalObject)) return visited[originalObject];
            if (typeof(Delegate).IsAssignableFrom(typeToReflect)) return null;
            var cloneObject = CloneMethod.Invoke(originalObject, null);
            if (typeToReflect.IsArray)
            {
                var arrayType = typeToReflect.GetElementType();
                if (IsPrimitive(arrayType) == false)
                {
                    Array clonedArray = (Array)cloneObject;
                    clonedArray.ForEach((array, indices) => array.SetValue(InternalCopy(clonedArray.GetValue(indices), visited), indices));
                }

            }
            visited.Add(originalObject, cloneObject);
            CopyFields(originalObject, visited, cloneObject, typeToReflect);
            RecursiveCopyBaseTypePrivateFields(originalObject, visited, cloneObject, typeToReflect);
            return cloneObject;
        }

        private static void RecursiveCopyBaseTypePrivateFields(object originalObject, IDictionary<object, object> visited, object cloneObject, Type typeToReflect)
        {
            if (typeToReflect.BaseType != null)
            {
                RecursiveCopyBaseTypePrivateFields(originalObject, visited, cloneObject, typeToReflect.BaseType);
                CopyFields(originalObject, visited, cloneObject, typeToReflect.BaseType, BindingFlags.Instance | BindingFlags.NonPublic, info => info.IsPrivate);
            }
        }

        private static void CopyFields(object originalObject, IDictionary<object, object> visited, object cloneObject, Type typeToReflect, BindingFlags bindingFlags = BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.FlattenHierarchy, Func<FieldInfo, bool> filter = null)
        {
            foreach (FieldInfo fieldInfo in typeToReflect.GetFields(bindingFlags))
            {
                if (filter != null && filter(fieldInfo) == false) continue;
                if (IsPrimitive(fieldInfo.FieldType)) continue;
                var originalFieldValue = fieldInfo.GetValue(originalObject);
                var clonedFieldValue = InternalCopy(originalFieldValue, visited);
                fieldInfo.SetValue(cloneObject, clonedFieldValue);
            }
        }
        public static T Copy<T>(this T original)
        {
            return (T)Copy((Object)original);
        }
    }

    public class ReferenceEqualityComparer : EqualityComparer<Object>
    {
        public override bool Equals(object x, object y)
        {
            return ReferenceEquals(x, y);
        }
        public override int GetHashCode(object obj)
        {
            if (obj == null) return 0;
            return obj.GetHashCode();
        }
    }

    namespace ArrayExtensions
    {
        public static class ArrayExtensions
        {
            public static void ForEach(this Array array, Action<Array, int[]> action)
            {
                if (array.LongLength == 0) return;
                ArrayTraverse walker = new ArrayTraverse(array);
                do action(array, walker.Position);
                while (walker.Step());
            }
        }

        internal class ArrayTraverse
        {
            public int[] Position;
            private int[] maxLengths;

            public ArrayTraverse(Array array)
            {
                maxLengths = new int[array.Rank];
                for (int i = 0; i < array.Rank; ++i)
                {
                    maxLengths[i] = array.GetLength(i) - 1;
                }
                Position = new int[array.Rank];
            }

            public bool Step()
            {
                for (int i = 0; i < Position.Length; ++i)
                {
                    if (Position[i] < maxLengths[i])
                    {
                        Position[i]++;
                        for (int j = 0; j < i; j++)
                        {
                            Position[j] = 0;
                        }
                        return true;
                    }
                }
                return false;
            }
        }
    }`

and this is how you can call the extension method:

public static List<ListResult.Entry> RequestWithPaging(this Subscription.SubscriptionListRequest req1, int pageSize = 100)
        {
            try
            {
                var entries = new List<ListResult.Entry>();

                string offset = null;
                var finished = false;

                req1.Limit(pageSize);

                while (!finished)
                {
                    var req = req1.Copy();
                    
                    if (!string.IsNullOrEmpty(offset))
                        req.Offset(offset);

                    var result = req.Request();
                    offset = result.NextOffset;

                    if (result.List != null && result.List.Any())
                        entries.AddRange(result.List);
                    finished = string.IsNullOrEmpty(offset);
                }

                return entries;
            }
            catch (Exception e)
            {
                System.Console.WriteLine(e);
                System.Console.WriteLine("Error!!!");
                System.Console.ReadLine();
                return null;
            }
        }

        ``` 

I hope it helps

@cb-alish
Copy link
Collaborator

Hi @Leh2, same piece of code is working fine for us as of now, can you please give it a try once and let us know if that is still not working for you?

@cb-sriramthiagarajan
Copy link
Collaborator

Hi @Leh2, can you please confirm if this is working for you?

@Leh2
Copy link
Author

Leh2 commented May 14, 2024

Thank you for your quick response 🥲 We will give it a try.

@Leh2
Copy link
Author

Leh2 commented May 17, 2024

It's still the same error in version 3.17.1:

System.ArgumentException: An item with the same key has already been added. Key: offset
at System.Collections.Generic.Dictionary2.TryInsert(TKey key, TValue value, InsertionBehavior behavior) at System.Collections.Generic.Dictionary2.Add(TKey key, TValue value)
at ChargeBee.Api.Params.AddOpt(String key, Object value)
at ChargeBee.Api.ListRequestBase`1.Offset(String offset)

The code also seems unchanged, it doesn't try to update the value if present: https://github.com/chargebee/chargebee-dotnet/blob/master/ChargeBee/Api/Params.cs#L27

@cb-sriramthiagarajan
Copy link
Collaborator

Hi @Leh2, my apologies for the delayed response 😢

We still see the original code sample that you shared is working for us 😕. Could you please help with the .NET version as well? Please share the output of these two commands:

dotnet --list-sdks
dotnet --list-runtimes

We'll try with these exact versions and see if we can reproduce the error. Thanks for your patience 🙏

@flixius
Copy link

flixius commented May 20, 2024

@Leh2 @cb-sriramthiagarajan
Check my Code snippet from above. You need to make a "deep-copy" of the request obj. And then add the offset

@Leh2
Copy link
Author

Leh2 commented May 21, 2024

@cb-sriramthiagarajan did you try a resource that contains more items than the specified page size?

`dotnet --list-sdks``

6.0.407 [/usr/local/share/dotnet/sdk]
7.0.203 [/usr/local/share/dotnet/sdk]
8.0.101 [/usr/local/share/dotnet/sdk]

`dotnet --list-runtimes``

Microsoft.AspNetCore.App 6.0.15 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.5 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 8.0.1 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.NETCore.App 6.0.15 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.5 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 8.0.1 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]

@flixius it's just a hack, not a fix :)

@cb-sriramthiagarajan
Copy link
Collaborator

Thanks for the info @Leh2. Will come back on this.

@flixius — thanks for the suggestion. We'll try to get this working in a straightforward manner.

@cb-alish
Copy link
Collaborator

Hi @Leh2, we tried with a site that has more records than the page size specified (100 for example) and also on different versions of .NET (v6, v7 & v8) but we're still not able to reproduce this issue and pagination looks to be working fine.

I'm sharing a sample repository that contains the code that's working for us. Can you please clone and try running this? Please let us know if you still see the same error.

@Leh2
Copy link
Author

Leh2 commented May 31, 2024

Hi @cb-alish,

My apologies, the example we provided is slightly different from our actual use case. In our use case, we want to create a generic extension method to retrieve all entries for any type.

The method looks like this:

public static async Task<IList<TResult>> GetAll<T, TResult>(this ListRequestBase<T> @this,
    Func<ListResult.Entry, TResult> selector)
        where T : ListRequestBase<T> where TResult : Resource
{
    var entries = new List<TResult>();

    const int pageSize = 100;
    string offset = null;

    do
    {
        var result = await @this
        .Offset(offset)
        .Limit(pageSize)
        .RequestAsyncWithLogging();

        offset = result.NextOffset;

        entries.AddRange(result.List.Select(selector));

    } while (offset != null);

    return entries;
}

So the difference is that we are reusing the type for ListRequestBase<T>.

@flixius
Copy link

flixius commented May 31, 2024

You need to make a deep copy of the instance ListRequestBase. The Code how to make a copy of the obj, you can find here: #54 (comment)

@flixius
Copy link

flixius commented May 31, 2024

Here is the Extension Method using Copy();

public static List<ListResult.Entry> RequestWithPaging<T>(this ListRequestBase<T> req1, int pageSize = 100) where T : ListRequestBase<T>
{
    try
    {
        var entries = new List<ListResult.Entry>();

        string offset = null;
        var finished = false;

        req1.Limit(pageSize);

        while (!finished)
        {
            var req = req1.Copy();

            if (!string.IsNullOrEmpty(offset))
                req.Offset(offset);

            var result = req.Request();
            offset = result.NextOffset;

            if (result.List != null && result.List.Any())
                entries.AddRange(result.List);
            finished = string.IsNullOrEmpty(offset);
        }

        return entries;
    }
    catch (Exception e)
    {
        System.Console.WriteLine(e);`
        throw e;
     }

The reason why you need to make a copy:

request.Offset(offset); is doing a TryInsert() on a Dictionary. When you re-use your existing instance, the Offset is already set. Therefore you can't update the offset parameter and an Exception is thrown. To fix this, you can Copy the request (without offset param) and then apply the offet within a loop as often as you want.

You said it's a hack. In my opinion, it's a very smart solution for an issue you can not solve by re-using your instance.

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

5 participants