Do not override `object_id`

[MatheusRich]

When using the gem, I get the following warnings

/Users/me/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/microsoft_graph-0.15.0/lib/models/drive_recipient.rb:101: warning: redefining `object_id' may cause serious problems
/Users/me/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/microsoft_graph-0.15.0/lib/models/access_package_subject.rb:110: warning: redefining `object_id' may cause serious problems
/Users/me/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/microsoft_graph-0.15.0/lib/models/security/oauth_application_evidence.rb:85: warning: redefining `object_id' may cause serious problems

#object_id is a method on all Ruby objects used to identify objects uniquely, and no two active objects will share an id.

I believe we could fix this by changing the name of these variables/methods to something like azure_object_id.

[baywet]

Thanks for reporting this, and for your patience in the matter. The code here is generated using kiota and this is something that would need to be fixed in the generator itself, or the OpenAPI description used for the generation.

[stephenarifin]

Is there any way to prevent these warnings from appearing? I tried using the https://dev.to/qortex/ruby-suppress-those-warnings-you-can-t-do-anything-about-6ie method but it didn't work.

[baywet]

Not at the moment, we're working on the generator to deduplicate the conflicting properties

[MatheusRich]

@stephenarifin I would not recommend suppressing those warnings if you're doing anything slightly serious. Overring object_id is dangerous!

[stephenarifin]

Hi everyone,

We are trying to use this for a production level application and it looks like including the MS Graph gem took down our entire application.

I included the gem but did not use it at all (no references) and it caused a memory leak and hung every request and it the app exploded.

We run APM software on our application such as New Relic, Sentry, and Scout APM and I believe that overriding the object id function is causes some of these APM gems that use the object id of the ruby object and caused the crash.

We are resorting to writing our own rest client for MS Graph. I don't know if this is the cause for sure but I wanted to share my experience, and it might be worth upping this issue in your priority stack.

[j15e]

We are looking at using this gem, but this issue seems pretty concerning. Any update on the matter?

[MatheusRich]

@j15e for now, you are better off writing your own wrapper around their APIs.

[baywet]

Hi everyone, Thank you for your patience on the matter.

The technical aspect

This issue should be trivial to fix at the generator level. Adding a reserved name ObjectId in the provider should lead kiota to avoid using that reserved name.

The additional challenge to release a new version is the fact that weekly pull requests are blocked by cyclic dependencies at you can see in #133. So we'd need to implement something that flattens the models namespaces in kiota (we've already tried trimming cyclic properties, but it's not sufficient due to inheritance and discriminated instantiation)

What would really help here is for someone to:

1. clone the repository locally
2. link that dependency to the local clone instead of the distributed package
3. rename the 3 properties that are problematic (see the list provided in the initial post)
4. confirm it solves the warnings/memory usage aspects

The prioritization aspect

Our team has been stretched for a while now, we're not able to hire, so we need to prioritized our investments. We're working on wrapping up and making generally available PHP/Python/Java/TypeScript/CLI first before we invest in any "new" language.

The upside is we've had multiple internal teams and 3rd party partners asking for Ruby support recently, so it might help shuffling things so we can at least tackle the two technical aspects outlined above to unblock the community here.

Tagging @sebastienlevert for visibility so he can make a case for it internally.

[j15e]

Thanks for the update!

I am not sure I understand how to get around the latest build problem / cyclic dependencies issue?

I get that I should clone Kiota locally and generate the Ruby SDK adding ObjectId to reserved names, but I am not sure how to work around the other build issue? Should we simply use a prior version of Kiota that doesn't have the cyclic dependencies problem? I guess this is the problem described in https://github.com/microsoft/kiota/issues/2834 which has no solution yet?

[baywet]

Sorry, I might have been unclear in my request earlier: The only thing I'm asking from the community here on this thread is to test this repository by linking it locally and renaming the offending properties. So we know the change in property names fixes the "memory leak" issue.

As for the cyclic dependency, the suggestion on our internal thread was to use kiota instead to generate a client just for the endpoints you care about. By doing so, kiota will trim out all the models that are not needed. Which hopefully gets you out of the cyclic dependency issue without needing a fix for that. But that's a separate conversation and I'd like to keep it on our other thread if you don't mind. The current main branch of this repository is not impacted by the cyclic dependency issue, changes in the metadata trigger that issue and prevent use from releasing new versions without a fix at this point.

[j15e]

Ah I get it now, thanks for the precisions! Much simpler to test 💯

I will try that. I thought object_id was used in like 99% of the endpoints and that it would be a pain to fork, but it turns out there are indeed just a few lines of Ruby to changes in this repository to test (unless there are more metaprogramming cases I don't see at first).

[j15e]

Last question: how would Kiota rename the property if it was in reserved names? object_id would become something like object_id_escaped?

I tried to figure out how Kiota refiners works but that is a bit complex and the test case I found don't make sense to me (reserved word break become something escaped?).

Edit: Just answered myself, it just appends _escaped to the property as I just saw many properties are named like that in the gem.

[j15e]

I tested the suggested fix in this fork branch and it does remove the redefining `object_id' may cause serious problems warnings on boot.

I was not yet using the SDK much so I can't tell about the memory consumption in production, but it could indeed be linked as overwriting object_id would probably mess with Ruby garbage collection.

[baywet]

Thanks for confirming. I'll take that with program managers and see if we can spend a few cycles addressing the two issues I was mentioning earlier to unblock everyone here.

[baywet]

Hey everyone! Good news, we have a new version 0.22.1 with a fix for the object id and refreshed metadata. I'm going to go ahead and close this issue, let us know how things go. (we'll re-open if needed)

We're not quite in a place where we can release weekly, we have file paths that are too long for gem build, it'll be another fix for another day :)