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

Add null check prior to using object. #791

Merged
merged 1 commit into from
Feb 4, 2021

Conversation

nigelgbanks
Copy link
Member

What does this Pull Request do?

Checks if an entity is null before de-referencing it.

What's new?

Prevents the following error:

Error: Call to a member function getFileUri() on null in islandora_text_extraction_media_presave() (line 40 of /var/www/drupal/web/modules/contrib/islandora/modules/islandora_text_extraction/islandora_text_extraction.module

How should this be tested?

Small change doesn't really require testing.

Interested parties

@Islandora/8-x-committers

@ajstanley
Copy link

Maybe throw something out to the logger if either $file or $file_id is NULL?

@jordandukart
Copy link
Member

@nigelgbanks you have re-producible conditions for this?

@nigelgbanks
Copy link
Member Author

nigelgbanks commented Aug 19, 2020

Since it takes ages to use the Drupal migrate module, I've got it setup up such that I can run migrations in parallel (Based of this fellows work https://www.drupal.org/project/migrate_tools/issues/3114358). When I create media entities for the first time they may reference files which not have been imported yet. So its a bit of odd use case in that it's not going through the forms to create the entity. That being said there isn't a reason to allow for methods to be called on null values.

@jordandukart
Copy link
Member

You able to address #791 (comment) @nigelgbanks?

@nigelgbanks
Copy link
Member Author

Oh sorry I kinda forgot about this, I've just patched locally. I don't see the point in logging in my use case as it's intentionally empty and whatever I can do to speed up these slow migrations I'm keen on. So I'd like to avoid the database update query for logging if possible, as large migrations are painfully slow. That being said if it's gonna block the pull request I'll add some logs, what do you say @ajstanley?

@dannylamb
Copy link

...and approved before seeing comments above. An info log statement would be appropriate. Other than that, changes are good and straightforward.

@nigelgbanks nigelgbanks force-pushed the 8.x-1.x-prevent-exception branch from 66259d3 to a7e9639 Compare October 19, 2020 16:17
@nigelgbanks nigelgbanks force-pushed the 8.x-1.x-prevent-exception branch from a7e9639 to a610928 Compare January 5, 2021 13:13
@nigelgbanks
Copy link
Member Author

Another rebase to please the Travis god of CI

@dannylamb dannylamb merged commit cfa48a9 into Islandora:8.x-1.x Feb 4, 2021
@nigelgbanks nigelgbanks deleted the 8.x-1.x-prevent-exception branch April 2, 2021 10:03
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

Successfully merging this pull request may close these issues.

4 participants