[$500] Chat - The video is reloaded every time you open it

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Found when validating PR : #37036

Version Number: v1.4.43-18
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): applausetester+ihchat4_2302_133c@applause.expensifail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal Team
Slack conversation:

Action Performed:

1. Open the app
2. Login with any account
3. Go to any chat
4. Plus button -> Add attachment -> Send a video
5. Open the video that you just sent
6. Tap the back button
7. Open the video again

Expected Result:

Loading spinner is displayed only when you open the video for the first time

Actual Result:

The video is reloaded every time you open it

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6390764_1708771155473.Record_2024-02-23-23-25-53_4f9154176b47c00da84e32064abf1c48.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018db53b84762ad824
• Upwork Job ID: 1761342164490567680
• Last Price Increase: 2024-03-30

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~018db53b84762ad824

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @DylanDylann (External)

[melvin-bot]

Triggered auto assignment to @greg-schroeder (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @Gonals (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[izarutskaya]

We think that this bug might be related to #vip-vsb
CC @quinthar

[webbdays]

This shows no caching in the app.

[melvin-bot]

📣 @webbdays! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[brandonhenry]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The problem we are trying to solve is that the video attachment in the chat is reloaded every time it is opened, instead of being cached after the first load. This results in unnecessary network requests and a suboptimal user experience. This only happens on the mobile version of the app.

What is the root cause of that problem?

The root cause of the problem is that the video is not being cached on the device after the first load in the AttachmentViewVideo component. Every time the video is opened, it is fetched from the server again, causing the reload. This is a common issue for react native and caching videos.

App/src/components/Attachments/AttachmentView/AttachmentViewVideo/index.js

Line 24 in 699e302

function AttachmentViewVideo({source, isHovered, shouldUseSharedVideoElement, videoDuration}) {

What changes do you think we should make in order to solve the problem?

To solve the problem, we should implement a caching mechanism for the video files. When a video is opened for the first time, it should be downloaded and cached on the device. Subsequent openings of the video should use the cached version instead of fetching it from the server again.

Thankfully, since we are already using expo, we can just make use of expo's file system.

1. Import the necessary functions from expo-file-system:

import * as FileSystem from 'expo-file-system';

2. Create a utility function to check if the video is cached and fetch/cache it if necessary:

const MAX_CACHE_SIZE = 500 * 1024 * 1024; // 500MB
const CACHE_DIR = `${FileSystem.cacheDirectory}videos/`;

const fetchAndCacheVideo = async (videoUri) => {  
  const fileName = videoUri.split('/').pop();
  const videoPath = `${CACHE_DIR}${fileName}`;

  // Check if the video is already cached
  const isCached = await FileSystem.getInfoAsync(videoPath).then(
    (fileInfo) => fileInfo.exists
  );

  if (isCached) {
    // Load the cached video
    return videoPath;
  } else {
    // Ensure the cache directory exists
    await FileSystem.makeDirectoryAsync(CACHE_DIR, { intermediates: true });
    
    // Check available space and clean cache if needed
    await cleanCacheIfNeeded();

    // Fetch and cache the video
    const downloadResult = await FileSystem.downloadAsync(
      videoUri,
      videoPath
    );

    if (downloadResult.status === 200) {
      return videoPath;
    } else {
      throw new Error('Failed to fetch video');
    }
  }
};

const getCacheSize = async () => {
  const files = await FileSystem.readDirectoryAsync(CACHE_DIR);
  const sizes = await Promise.all(
    files.map(file => FileSystem.getInfoAsync(`${CACHE_DIR}${file}`))
  );
  
  return sizes.reduce((total, {size}) => total + size, 0); 
};

const cleanCacheIfNeeded = async () => {
  const cacheSize = await getCacheSize();
  
  if (cacheSize > MAX_CACHE_SIZE) {
    const files = await FileSystem.readDirectoryAsync(CACHE_DIR);
    const fileInfos = await Promise.all(
      files.map(file => FileSystem.getInfoAsync(`${CACHE_DIR}${file}`))  
    );
    
    // Sort by modificationTime ascending to remove oldest first
    fileInfos.sort((a,b) => a.modificationTime - b.modificationTime);
    
    let sizeToClear = cacheSize - MAX_CACHE_SIZE;
  
    for (const {uri, size} of fileInfos) {
      await FileSystem.deleteAsync(uri);
      sizeToClear -= size;
      
      if (sizeToClear <= 0) {
        break;
      }
    }
  }
};

The fetchAndCacheVideo function checks if the video is already cached in the app's cache directory (FileSystem.cacheDirectory). If the video is cached, it returns the cached file path. If not, it creates a videos/ directory in the cache directory (if it doesn't exist), fetches the video from the provided videoUri, and caches it in the videos/ directory.

UPDATE:

Two new utility functions are added:

• getCacheSize recursively gets the total size of all files in the video cache directory
• cleanCacheIfNeeded checks if the cache exceeds MAX_CACHE_SIZE (set to 500MB here) and if so, deletes the oldest videos until the size is below the limit. The check is performed before caching a new video.

This approach limits the video cache to a maximum size and automatically removes the oldest cached videos when that limit is reached. The specific size limit and cache location can be adjusted as needed.

[webbdays]

Note:
Streaming the video is the best thing to do than download whole video at a time.
Its best to achieve this by doing things like when video plays for the first time we get the stream segments. As we get more and more segments we join them to form a video. This helps us to get the video and users sees the video without waiting long time instead of waiting for full long video to download at first.

Second when we add caching, cache invalidation is very important. Adding functionality like ask the server "is the video/file modified?" before asking for video. (This needs some server side changes(very minor)).

[Skalakid]

@izarutskaya @DylanDylann @greg-schroeder I'm not sure if it is a bug, I think it is more like a new feature for the video player. The presented behavior is correct according to the video player design doc. Video caching wasn't planned during the predesign. I think it might be a great improvement! What do you think about it @francoisl?

@brandonhenry video player work differently depending on the screen size. On small screens, they are only usable inside the attachment modal. On large screens video players are available inside chat. When extending the video we are sharing the component to the attachment carousel to optimize video loadings (currently it is broken on main but I created a fix for it). Because of that, I think we should cache only videos that have shouldUseSharedVideoElement set to false.
Also, videos may have bigger sizes, so I think it would be nice to limit the amount of videos that are currently cached so a user won't run out of memory. What's your thoughts on that?

[webbdays]

Cache mechanism is as usual -> use cache if exists.

Details

Here is the detail overview what and how the storage API is used for caching and its compatibility.

Storage API in react-native-onyx repo:

1. It was written to be compatible for all the three platforms (web, Android, IOS). Its just wrapper around below two libraries.

   • idb-keyval
   • react-native-quick-sqlite

2. It uses "idb-keyval" to connect with indexedDB for web platforms. Store key value pairs data. Storage Capacity: some GB's

3. It uses "react-native-quick-sqlite" to store data in native sqlite DB in both android and ios platforms. Storage Capacity: Some GB's.

4. These libraries help us to store video data.

5. When the video is played first, we download as blob and convert that blob to base64 string, and build a json.
   {base64:base64ofVideoblob}
   store this in DB using storage API,
   for web(Indexed DB) - key-value pairs are stored. It don't care what inside it as far as the key - url is a string and value is in correct json format. This way of storage is compatible with mweb safari as well, no problem.
   for native - here json is converted to string. and store in a keyvaluepairs table along with the key.

Mainly these methods are used.

// example usage
import cacheStorage from "@libs/storageAPI";
cacheStorage.setItem(url, value_json)
cacheStorage.getItem(url)
// to know if any keys exist.
cacheStorage.getAllKeys()

// we use this way in our cache Implementation as usual.

for web

this code is used by storage API

to get key value pair from db
to set key value pair
to get all keys in DB

for native

this code is used by storage API
to get key value pair from db
to set key value pair
to get all keys in DB

References:

1. Storage API
2. idb-keyval
3. react-native-quick-sqlite
4. IndexedDB

[brandonhenry]

@DylanDylann

1. It seems expo-file-system doesn't support mWeb
2. Why do you think we should remove the video from the cache in modificationTime order
3. The data in the cache could be outdated. How we can validate data on the cache?

1. Ahh good catch. In that case, we can just rely on the Cache-Control header of the fetch call to manage that. This is supported in all browsers and will handle this edge case for mWeb
2. This is essentially the creation time. To me, it makes sense to remove the oldest videos first and so we use this variable to determine that
3. Sure, that makes sense. I have a validation strategy, but its a bit long. The general idea is to create a metadata object and store that along with the video when we call fetchAndCacheVideo. We can check if the video has already been cached and if so, validate it based on the metadata we have stored. The metadata could be the video converted to blob or a simple timestamp that we can validate against an expiration:

const isVideoCachedValid = (timestamp) => {
  const currentTimestamp = Date.now();
  const expirationTime = 24 * 60 * 60 * 1000; // 24 hours in milliseconds

  return currentTimestamp - timestamp < expirationTime;
};

Example:

import * as FileSystem from 'expo-file-system';

const CACHE_DIR = `${FileSystem.cacheDirectory}videos/`;

const fetchAndCacheVideo = async (videoUri) => {
  const fileName = videoUri.split('/').pop();
  const videoPath = `${CACHE_DIR}${fileName}`;
  const metadataPath = `${CACHE_DIR}${fileName}.metadata`;

  // Check if the video is already cached
  const isCached = await FileSystem.getInfoAsync(videoPath).then(
    (fileInfo) => fileInfo.exists
  );

  if (isCached) {
    // Read the cached metadata
    const metadata = await FileSystem.readAsStringAsync(metadataPath);
    const { timestamp } = JSON.parse(metadata);

    // Check if the cached video is still valid
    const isValid = isVideoCachedValid(timestamp);

    if (isValid) {
      return videoPath;
    } else {
      // If the cached video is outdated, delete it and its metadata
      await FileSystem.deleteAsync(videoPath);
      await FileSystem.deleteAsync(metadataPath);
    }
  }

  // Fetch the video from the server and turn on caching for mWeb
   const response = await fetch(videoUri, {
    headers: {
      'Cache-Control': 'max-age=31536000'
    },
   });

  if (response.ok) {
    // Create the cache directory if it doesn't exist
    // Download and cache the video
};

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@Gonals @greg-schroeder @DylanDylann this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[brandonhenry]

i can put together a working POC this weekend. Trying to get my iOS build / mWeb localhost working but having issues with websocket... trying to get that resolved quickly so i can get a PR merged but will get back to this (getting 200+ lottie errors when building ios but everything else builds fine)

Edit: Fixed the below issues. I just needed to update my Mac to latest vesrsion and swap to XCode 15.1

[greg-schroeder]

conversation ongoing

[DylanDylann]

Discussing on Slack

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[webbdays]

@DylanDylann
@brandonhenry
any outcome?
Thank you.

[DylanDylann]

Discussing on Slack

[greg-schroeder]

This might be moved Internal per discussion - at any rate it's definitely chat-related (but not expense related) so I'm going to put this into #vip-vsb

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[DylanDylann]

@francoisl We decided to handle this issue internally. Should we close this one?

[greg-schroeder]

I'm going to close it per that discussion ... please reopen if you disagree.

[lanitochka17]

Issue is still reproducible on the latest build 1.4.74-0

vide.mp4

[greg-schroeder]

Thanks @lanitochka17 - we're just handling it in the linked issue above, so we can leave this closed.