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

Side-loading large quantities of associated records raises 414 (Request-URI Too Large) #651

Closed
ivanvanderbyl opened this issue Jan 20, 2013 · 13 comments
Labels
🏷️ feat This PR introduces a new feature
Milestone

Comments

@ivanvanderbyl
Copy link
Contributor

The API my app talks to uses 128bit UUIDs for IDs, and we get a lot of them. This causes the server to reject the request to side-load even a relatively small quantity of records (~100) because the request URI is so large it seems to exceed a server-side limit.

Now I'm not sure precisely what the limit is, although I've heard it's around 4096 chars.

Is there a way to only side-load small batches, say 10 at a time, or as needed?

@sly7-7
Copy link
Contributor

sly7-7 commented Jan 20, 2013

There is a related PR submitted: #531 I tried this at work, but I encountered application freeze during load of about 1000 models using App.MyModel.find(). But considering your use case, it could probably work.

@JaredSartin
Copy link

What is the status on this? I have a large number of IDs that need updating. I would love a safe batch size for the URL and just get all the data in batches... not possible yet?

@wycats
Copy link
Member

wycats commented Sep 2, 2013

This is a good idea. I'll move it into the Beta 2 milestone.

@wycats
Copy link
Member

wycats commented Sep 2, 2013

Note that the server can also return a URL for the association in links:

{
  "post": {
    "id": 1,
    "title": "Rails is omakase",
    "links": {
      "comments": "/posts/1/comments"
    }
  }
}

@JaredSartin
Copy link

That would bypass the URL problems encountered in large hasMany
associations. I like the association URL.

On Monday, September 2, 2013, Yehuda Katz wrote:

Note that the server can also return a URL for the association in links:

{
"post": {
"id": 1,
"title": "Rails is omakase",
"links": {
"comments": "/posts/1/comments"
}
}}


Reply to this email directly or view it on GitHubhttps://github.com//issues/651#issuecomment-23667521
.

@wycats wycats closed this as completed Sep 10, 2013
@wycats
Copy link
Member

wycats commented Sep 10, 2013

Closed in favor of association URLs for large collections.

@gucki
Copy link

gucki commented Nov 18, 2013

@wycats Should using links already work using ember-data beta3? It's not working for me, the association does not load at all and always returns 0 elements.

App.Facade = DS.Model.extend
  windows: DS.hasMany('window')

App.Window = DS.Model.extend
  facade: DS.belongsTo('facade')


JSON for /facades
{
  facades: [
    {
      id: "A",
      links: {
        windows: "/facades/A/windows"
      }
    },
    {
      id: "H",
      links: {
        windows: "/facades/H/windows"
      }
    }
  ]
}

@gucki
Copy link

gucki commented Nov 18, 2013

Ok, aftert digging into the ember-data code it seems one has to set the async flag like this:

App.Facade = DS.Model.extend
  windows: DS.hasMany('window', async: true)

Then it works... :)

@GetContented
Copy link

While I understand the rationale behind choosing this is a much better solution to issues like #804 , it feels very much like it's a bug to leave ember ABLE to load more objects than can fit on a single URL request. I'm currently experiencing this problem because of not being able to turn on the links feature because it doens't work in my case (a bug referenced and possibly fixed by #1791)... so I'm left with an app that silently crashes sometimes. :( I'm getting 400's atthe JS level because of the requests being too long to fit within a single line. I may have to implement @abdul's "hacky solution" from #241 I guess)

@igorT
Copy link
Member

igorT commented Jul 3, 2014

This will be fixed soon by the findCoalescing branch

@igorT igorT reopened this Jul 3, 2014
@GetContented
Copy link

I ended up rewriting my adapter's findMany...:

        findMany: function(store, type, ids) {
          self = this;
          return new Ember.RSVP.Promise(function(resolve, reject) {
            var idsPerRequest = 40;
            var totalIdsLength = ids.length;
            var numberOfBins = Math.ceil( totalIdsLength / idsPerRequest ); // number per bin
            var bins = [];
            ids.forEach( function(someId, index) {
              var thisBinIndex = index % numberOfBins;
              var thisBin = Ember.A( bins[thisBinIndex] );
              thisBin.pushObject(someId);
              bins[thisBinIndex] = thisBin;
            });

            // build an array of promises, then resolve using Ember.RSVP.all
            var requestPromises = bins.map(function(binOfIds) {
              return self.ajax(self.buildURL(type.typeKey), 'GET', { data: { ids: binOfIds } });
            });

          // build the required return object, which is an array containing the json
          // response object
            Ember.RSVP.all(requestPromises).then(function(resolvedBinRequests) {
            var pluralizedDecamelizedTypeKey = type.typeKey.decamelize().pluralize();
            var resolvedObjects = Em.A([]);
            var returnObject = {};
            returnObject[pluralizedDecamelizedTypeKey] = resolvedObjects;
              resolvedBinRequests.forEach(function(resolvedBin) {
              var theArray = resolvedBin[pluralizedDecamelizedTypeKey];
                resolvedObjects.addObjects(theArray);
              });
            var responsePromise = Ember.RSVP.Promise.cast(returnObject);
              resolve(responsePromise);
            }, function(error) {
              reject(error);
            });
          });
        }

@GetContented
Copy link

I still think punting this choice to users by default (rather than make it impossible to do something stupid) is a bad decision, but perhaps I'm not fully following the new default behaviour.

igorT added a commit that referenced this issue Jul 22, 2014
This PR brings following improvements/changes:

1. Find calls from the same runloop will be coalesced into findMany
requests if the adapter has a findMany method
2. Adds a groupRecordsForFindMany hook on the adapter that allows you
to decide how to coalesce the record requests. This will enable fixing
of bugs such as #651, by segmenting on the number of records
3. Allows users to preset attributes and relationships on a model when
doing a find call. For relationships you can either pass in a record or
an id
4. Gives rudimentary support for nested urls, by passing in the record
object to the buildUrl method
5. Removes the owner being passed to findMany becuase it can be looked
up on the original record
6. Adds couple special cased SSOT features that were needed for
buildUrl/removal of owner property to be viable

This PR lays the groundwork for:
1. Adding support for reloading hasManys
2. Making nice sugar for nestedUrls in the RestAdater
igorT added a commit that referenced this issue Jul 22, 2014
This PR brings following improvements/changes:

1. Find calls from the same runloop will be coalesced into findMany
requests if the adapter has a findMany method
2. Adds a groupRecordsForFindMany hook on the adapter that allows you
to decide how to coalesce the record requests. This will enable fixing
of bugs such as #651, by segmenting on the number of records
3. Allows users to preset attributes and relationships on a model when
doing a find call. For relationships you can either pass in a record or
an id
4. Gives rudimentary support for nested urls, by passing in the record
object to the buildUrl method
5. Removes the owner being passed to findMany becuase it can be looked
up on the original record
6. Adds couple special cased SSOT features that were needed for
buildUrl/removal of owner property to be viable

This PR lays the groundwork for:
1. Adding support for reloading hasManys
2. Making nice sugar for nestedUrls in the RestAdater
igorT added a commit that referenced this issue Jul 24, 2014
This PR brings following improvements/changes:

1. Find calls from the same runloop will be coalesced into findMany
requests if the adapter has a findMany method
2. Adds a groupRecordsForFindMany hook on the adapter that allows you
to decide how to coalesce the record requests. This will enable fixing
of bugs such as #651, by segmenting on the number of records
3. Allows users to preset attributes and relationships on a model when
doing a find call. For relationships you can either pass in a record or
an id
4. Gives rudimentary support for nested urls, by passing in the record
object to the buildUrl method
5. Removes the owner being passed to findMany becuase it can be looked
up on the original record
6. Adds couple special cased SSOT features that were needed for
buildUrl/removal of owner property to be viable

This PR lays the groundwork for:
1. Adding support for reloading hasManys
2. Making nice sugar for nestedUrls in the RestAdater
igorT added a commit that referenced this issue Jul 24, 2014
This PR brings following improvements/changes:

1. Find calls from the same runloop will be coalesced into findMany
requests if the adapter has a findMany method
2. Adds a groupRecordsForFindMany hook on the adapter that allows you
to decide how to coalesce the record requests. This will enable fixing
of bugs such as #651, by segmenting on the number of records
3. Allows users to preset attributes and relationships on a model when
doing a find call. For relationships you can either pass in a record or
an id
4. Gives rudimentary support for nested urls, by passing in the record
object to the buildUrl method
5. Removes the owner being passed to findMany becuase it can be looked
up on the original record
6. Adds couple special cased SSOT features that were needed for
buildUrl/removal of owner property to be viable

This PR lays the groundwork for:
1. Adding support for reloading hasManys
2. Making nice sugar for nestedUrls in the RestAdater
igorT added a commit that referenced this issue Jul 24, 2014
This PR brings following improvements/changes:

1. Find calls from the same runloop will be coalesced into findMany
requests if the adapter has a findMany method
2. Adds a groupRecordsForFindMany hook on the adapter that allows you
to decide how to coalesce the record requests. This will enable fixing
of bugs such as #651, by segmenting on the number of records
3. Allows users to preset attributes and relationships on a model when
doing a find call. For relationships you can either pass in a record or
an id
4. Gives rudimentary support for nested urls, by passing in the record
object to the buildUrl method
5. Removes the owner being passed to findMany becuase it can be looked
up on the original record
6. Adds couple special cased SSOT features that were needed for
buildUrl/removal of owner property to be viable

This PR lays the groundwork for:
1. Adding support for reloading hasManys
2. Making nice sugar for nestedUrls in the RestAdater
igorT added a commit that referenced this issue Jul 24, 2014
This PR brings following improvements/changes:

1. Find calls from the same runloop will be coalesced into findMany
requests if the adapter has a findMany method
2. Adds a groupRecordsForFindMany hook on the adapter that allows you
to decide how to coalesce the record requests. This will enable fixing
of bugs such as #651, by segmenting on the number of records
3. Allows users to preset attributes and relationships on a model when
doing a find call. For relationships you can either pass in a record or
an id
4. Gives rudimentary support for nested urls, by passing in the record
object to the buildUrl method
5. Removes the owner being passed to findMany becuase it can be looked
up on the original record
6. Adds couple special cased SSOT features that were needed for
buildUrl/removal of owner property to be viable

This PR lays the groundwork for:
1. Adding support for reloading hasManys
2. Making nice sugar for nestedUrls in the RestAdater
igorT added a commit that referenced this issue Jul 24, 2014
This PR brings following improvements/changes:

1. Find calls from the same runloop will be coalesced into findMany
requests if the adapter has a findMany method
2. Adds a groupRecordsForFindMany hook on the adapter that allows you
to decide how to coalesce the record requests. This will enable fixing
of bugs such as #651, by segmenting on the number of records
3. Allows users to preset attributes and relationships on a model when
doing a find call. For relationships you can either pass in a record or
an id
4. Gives rudimentary support for nested urls, by passing in the record
object to the buildUrl method
5. Removes the owner being passed to findMany becuase it can be looked
up on the original record
6. Adds couple special cased SSOT features that were needed for
buildUrl/removal of owner property to be viable

This PR lays the groundwork for:
1. Adding support for reloading hasManys
2. Making nice sugar for nestedUrls in the RestAdater
igorT added a commit that referenced this issue Jul 24, 2014
This PR brings following improvements/changes:

1. Find calls from the same runloop will be coalesced into findMany
requests if the adapter has a findMany method
2. Adds a groupRecordsForFindMany hook on the adapter that allows you
to decide how to coalesce the record requests. This will enable fixing
of bugs such as #651, by segmenting on the number of records
3. Allows users to preset attributes and relationships on a model when
doing a find call. For relationships you can either pass in a record or
an id
4. Gives rudimentary support for nested urls, by passing in the record
object to the buildUrl method
5. Removes the owner being passed to findMany becuase it can be looked
up on the original record
6. Adds couple special cased SSOT features that were needed for
buildUrl/removal of owner property to be viable

This PR lays the groundwork for:
1. Adding support for reloading hasManys
2. Making nice sugar for nestedUrls in the RestAdater
@igorT
Copy link
Member

igorT commented Aug 12, 2014

Fixed by @jcollins1991

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ feat This PR introduces a new feature
Projects
None yet
Development

No branches or pull requests

8 participants