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

Recursive requests from redirect_handle_rest_rel_request() are broken for self-referential relationships #201

Closed
deven opened this issue Nov 18, 2021 · 1 comment

Comments

@deven
Copy link
Contributor

deven commented Nov 18, 2021

Given a self-referential relationship like this for MyApp::Schema::Result::Customer:

__PACKAGE__->has_many(
  "child_customers",
  "MyApp::Schema::Result::Customer",
  { 'foreign.parent_customer_id' => 'self.id' },
  { cascade_copy => 0, cascade_delete => 0 },
);

With this self-referential relationship, a URI path like /main/db/db_customer/item/1234/rel/child_customers is broken. Instead of returning the matching customer records, this will return all customers.

After extensive tracing and debugging of the core RapidApp code, and discussing this issue with @vanstyn, I believe I have a fix for the root cause of the bug. The chain of causation (in reverse) looks like this:

  • The relationship URL displays all customers instead of the correct list of specific customers.
  • This is because the Ajax request to /main/db/db_customer/store/read sends POST parameters which are missing the rs_method and rs_path parameters, which are required to narrow the result set to the correct results.
  • This is because the Ajax request to the relationship URI returns Javascript code without base parameters containing the rs_method and rs_path parameters.
  • This is because redirect_handle_rest_rel_request() uses the ExtJS configuration data in the RapidApp::Module::DatStor store object to construct the base parameters, and the rs_method and rs_path are also missing from the store object's ExtJS configuration.
  • This is because store_init_onrequest() sets the ExtJS configuration, but it doesn't have the rs_method and rs_path values available when it runs on the store object.
  • This is because store_init_onrequest() runs on the store object before redirect_handle_rest_rel_request() runs, determining the correct rs_method and rs_path values.
  • This is because the store_init_onrequest() method runs as an ONREQUEST handler, which only run once per request. In this case, the ONREQUEST handlers for the store object have already been triggered earlier in the relationship URI request cycle by the time redirect_handle_rest_rel_request() runs.
  • This only seems to be an issue for self-referential relationships. If the relationship connects to different table, redirect_handle_rest_rel_request() ends up running before the ONREQUEST handlers for the other table's store object, which is why the relationship URLs normally work as expected.

The recursive call from redirect_handle_rest_rel_request() to $c->root_module_controller->approot($c,$url) is meant to run as an independent request with fresh object state, so the recursive call should not be skipping ONREQUEST handlers which previously ran for the original request. While it took many hours to trace/debug the code and understand the chain of causation above, the actual code fix is very simple. The value of $c->request_id must be changed to a unique value during the recursive approot() call, which can be accomplished with a single-line code change for each of the two recursive approot() calls. This will cause RapidApp to actually treat the recursive request as an independent request and run all ONREQUEST handlers as it should, regardless of what ONREQUEST handlers may have already run during the original request.

With this fix, the self-referential relationship URL works correctly, returning the intended subset of customers in the results.

This is the same technique used by RapidApp::RapidApp::cleanupAfterRequest() for any post-processing tasks that it runs. My patch adds 0.01 to the $c->request_id value to prevent overlap with the values generated for post-processing tasks.

deven added a commit to deven/RapidApp that referenced this issue Nov 18, 2021
Use "local" to bypass the attribute accessor and dynamically override
the internal $c->{request_id} field during recursive approot() calls,
adding 0.01 to the value to avoid overlap with similar values generated
by RapidApp::RapidApp::cleanupAfterRequest() for post-processing tasks.
deven added a commit to deven/RapidApp that referenced this issue Nov 18, 2021
Use "local" to bypass the attribute accessor and dynamically override
the internal $c->{request_id} field during recursive approot() calls,
adding 0.01 to the value to avoid overlap with similar values generated
by RapidApp::RapidApp::cleanupAfterRequest() for post-processing tasks.
vanstyn added a commit that referenced this issue Nov 22, 2021
Change $c->request_id when recursing to approot(). (#201)
@vanstyn
Copy link
Owner

vanstyn commented Nov 22, 2021

Fixed in PR #202

@vanstyn vanstyn closed this as completed Nov 22, 2021
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

2 participants