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

Refactor the discovery.xml fetching #56

Open
donquixote opened this issue Nov 12, 2024 · 1 comment
Open

Refactor the discovery.xml fetching #56

donquixote opened this issue Nov 12, 2024 · 1 comment

Comments

@donquixote
Copy link
Collaborator

donquixote commented Nov 12, 2024

We have the CoolRequest class which fetches the discovery.xml and extracts the WOPI client url from the xml.
I would like to refactor this.

Refactoring:

  • Replace global functions with services.
  • Avoid object state where it is not needed.

Functional changes:

  • Rethink error handling.
  • Don't concatenate t() strings, and rethink error message passing.
  • Use Guzzle instead of file_get_contents to fetch the remote content.
  • Cache the result, to avoid repeated calls. This also means we need to flush that cache at some point.
  • Allow getting different values from the xml, e.g. for different mime types.

@hfiguiere some questions.
In CoolRequest we have these error codes 101, 201 etc.
Do these numbers have special meanings? Is there any external system that understands these number codes?

donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
…get discovery.

It can happen that file_get_contents() hangs at the end of a stream, waiting for more data.
With curl it seems this does not happen.

Even better would be to use e.g. Guzzle http client, but that's too big a change for now.
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Nov 12, 2024
donquixote added a commit that referenced this issue Dec 3, 2024
donquixote added a commit that referenced this issue Dec 3, 2024
donquixote added a commit that referenced this issue Dec 3, 2024
donquixote added a commit that referenced this issue Dec 3, 2024
…t in ViewerController.

Also make the private property protected.
donquixote added a commit that referenced this issue Dec 3, 2024
donquixote added a commit that referenced this issue Dec 3, 2024
donquixote added a commit that referenced this issue Dec 3, 2024
donquixote added a commit that referenced this issue Dec 3, 2024
donquixote added a commit that referenced this issue Dec 3, 2024
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