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

Enable search for users by external_id where external_id is a String (not a long) #548

Closed
andy-may-at opened this issue Mar 6, 2023 · 6 comments
Labels

Comments

@andy-may-at
Copy link
Contributor

The method to retrieve users by external ID: (public List<User> getUsersByExternalIds(long externalId, long... externalIds)) expects the external IDs to be longs, but the underlying value is a String in the Zendesk API

In my use case, our external IDs are Strings, so we can't search for users by external ID using this method,..

I propose to implementing an additional public List<User> getUsersByExternalIds(String externalId, String... externalIds) method which

I'll look to raise a PR for this shortly if the change would be welcomed

@PierreBtz
Copy link
Collaborator

@andy-may-at indeed that doesn't seem correct. PR welcome, you can also deprecate the old method taking longs for future removal.

@PierreBtz PierreBtz added the bug label Mar 7, 2023
@andy-may-at
Copy link
Contributor Author

Thanks @PierreBtz
PR is #552 - I've included the deprecation of the old method

@colbya
Copy link

colbya commented Mar 7, 2023

Is there history on why the api often does varargs with the double parameters?

@andy-may-at
Copy link
Contributor Author

I assumed that it was a way of allowing 1...N parameters, rather than 0...N

@colbya
Copy link

colbya commented Mar 7, 2023

Is it worth considering making the idArray method generic and using it for the previous Long and the new String case?

    private static <R> List<R> idArray(R id, R... ids) {
        List<R> result = new ArrayList<>(ids.length + 1);
        result.add(id);
        for (R i : ids) {
            result.add(i);
        }
        return result;
    }

@andy-may-at
Copy link
Contributor Author

andy-may-at commented Mar 8, 2023

@colbya
That sounds good, but has some complications:

If we do that, but keep the signature of getUsersByExternalIds(long, long...), then it looks like we would have to convert it's vararg long[] into an Long[] before calling idArray()

And if we change the signature to getUsersByExternalIds(Long, Long...) then there's a risk that we break any calling code that was relying on Java doing type promotion.
(i.e. anything calling getusersByExternalIds(1,2,3) would no longer compile & would have to be replaced with getusersByExternalIds(1L, 2L, 3L) )

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

No branches or pull requests

3 participants