Improvement for media diversity management of Recordings

[ghjansen]

With the recent efforts enabling video in Restcomm with XMS, the files created for Recordings have now a new extension apart from .wav, it is the .mp4 extension.

By adapting the correspondent endpoint and interpreters to handle the new mp4 extension, it was noticed that the file extension of a Recording entity is handled through the fileUri by string operations, separating the extension from the absolute path.

Turns out that this practice leads us to multiple occurrences of hard coded operations only to manage this information, like seen in RecordingsEndpoint.java#L273.

So, it may be a good time to think about a new attribute for the entity Recording, that will responsible to keep the media type of the file (as file extension or as MIME type), so there is no need for additional/risky operations to extract this info from another attribute every time.

Pros

• avoid hard coding of file extensions;
• media type is safely managed by persistence layer;
• code gets simpler, safer and straight forward;
• handles a potential media diversity growth with quality.

Cons

• need for a migration process (set value in new database column for all its rows);
• code refactoring.

Of course this is only a idea and we can/must brainstorm on that.

Thoughts? @gvagenas @deruelle @otsakir

[deruelle]

thanks for opening a brainstorm issue on this @ghjansen, I support this idea.

[ghjansen]

After the implementation of video (merged through #2575) we now have the class MediaAttributes that could potentially be injected inside the recordings logic and handle the file format management in a consistent and scalable way. Need to check.

https://github.com/RestComm/Restcomm-Connect/blob/9b7df206b3372e556544e884c4e685b386e58fa8/restcomm/restcomm.dao/src/main/java/org/restcomm/connect/dao/entities/MediaAttributes.java#L32

[otsakir]

Seems right for me. As always, when there is database involved, there is always a trade off with the cost of the migration. I would fallback to at least externalizing the logic of parsing the filename and returning a Video/Audio enum.

[ghjansen]

Right @otsakir, allow me to elaborate a bit. Still in the mentioned hypothesis, i guess MediaAttributes could be the transport of the file format, replacing the hard coded one, so we avoid the exposure of the String in operations like the following ones: https://github.com/RestComm/Restcomm-Connect/blob/10855745f225ac5fc5a2f862df2d59b86e835d7e/restcomm/restcomm.dao/src/main/java/org/restcomm/connect/dao/mybatis/MybatisRecordingsDao.java#L78 https://github.com/RestComm/Restcomm-Connect/blob/10855745f225ac5fc5a2f862df2d59b86e835d7e/restcomm/restcomm.dao/src/main/java/org/restcomm/connect/dao/mybatis/MybatisRecordingsDao.java#L269 https://github.com/RestComm/Restcomm-Connect/blob/10855745f225ac5fc5a2f862df2d59b86e835d7e/restcomm/restcomm.dao/src/main/java/org/restcomm/connect/dao/mybatis/MybatisRecordingsDao.java#L293 Currently, several different classes are using file extension in the hard coded way, so i think the refactor to MediaAttributes.getMediaFileFormat().toString() (or similar) worth it. This information could be obtained once from the RCML in case of Record verb, or also based on MediaAttributes.MediaType of the call.

This modification should not affect the database structure, thus not requiring any kind of migration. I agree though that it might be more interesting to have the file format in a exclusive column, apart from the file path/name, but that makes me think about the real need of it vs. all the intricate consequences like migration, internal refactoring, etc.

@otsakir thoughts?