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

Index MediaAssets in OpenSearch #1014

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Conversation

naknomum
Copy link
Member

@naknomum naknomum commented Feb 6, 2025

Index MediaAssets in OpenSearch

PR fixes #912

  • best guess at fields needed: id, acmId, uuid, filename(s), detectionStatus, basic info about Encounters and Annotations
  • only "root parent" MediaAssets index - no derived children (thumbnails, etc)
  • no api for search added (will be added when need arises)

⚠️ Substantial change ⚠️

Due to the fact that the Base.java class (which search is built on) requires getId() of the implementing class to be overridden and return a String -- and thanks to a questionable decision to have the primary key id in MediaAsset to be an int -- I had to do a little slight-of-hand. int getIdInt() now has replaced the original getId() in MediaAsset.java; getId() returns the String (of the int id).

All code which used to call getId() but requires an int, has been modified. A surprisingly huge amount of the code was just fine with getId() being left as-is and returning a String: most of these were just being used inside other Strings, jsp output, debugging statements, and the like.

Best attempt was made to find all the substitutions in jsp files (as there would be no compilation warnings there). Spot checks on running jsp pages could not find any missed. However, this branch should be tested thoroughly on QA before merging. For that reason, it will be left a DRAFT until further testing and code review.

@codecov-commenter
Copy link

codecov-commenter commented Feb 6, 2025

Codecov Report

Attention: Patch coverage is 2.24719% with 87 lines in your changes missing coverage. Please review.

Project coverage is 0.14%. Comparing base (5b1695e) to head (a6368af).

Files with missing lines Patch % Lines
src/main/java/org/ecocean/media/MediaAsset.java 4.08% 47 Missing ⚠️
src/main/java/org/ecocean/Shepherd.java 0.00% 12 Missing ⚠️
src/main/java/org/ecocean/Encounter.java 0.00% 7 Missing ⚠️
src/main/java/org/ecocean/identity/IBEISIA.java 0.00% 4 Missing ⚠️
...main/java/org/ecocean/media/YouTubeAssetStore.java 0.00% 4 Missing ⚠️
src/main/java/org/ecocean/media/AssetStore.java 0.00% 2 Missing ⚠️
...ain/java/org/ecocean/servlet/MediaAssetAttach.java 0.00% 2 Missing ⚠️
...ain/java/org/ecocean/servlet/MediaAssetCreate.java 0.00% 2 Missing ⚠️
src/main/java/org/ecocean/Annotation.java 0.00% 1 Missing ⚠️
src/main/java/org/ecocean/OpenSearch.java 0.00% 1 Missing ⚠️
... and 5 more
Additional details and impacted files
@@           Coverage Diff           @@
##              main   #1014   +/-   ##
=======================================
  Coverage     0.13%   0.14%           
- Complexity      29      30    +1     
=======================================
  Files          580     580           
  Lines        64015   64075   +60     
  Branches     11090   11094    +4     
=======================================
+ Hits            89      91    +2     
- Misses       63907   63965   +58     
  Partials        19      19           
Flag Coverage Δ
backend 0.14% <2.24%> (+<0.01%) ⬆️
frontend 0.14% <2.24%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@naknomum naknomum changed the title 912 mediaasset in opensearch Index MediaAssets in OpenSearch Feb 7, 2025
@vkirkl vkirkl added this to the 10.7.0 milestone Feb 11, 2025
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.

mediaAssets indexed by OpenSearch
3 participants