elimu-ai / webapp

🖥 Web application for hosting Android applications and educational content
https://hin.elimu.ai
MIT License
30 stars 54 forks source link

Rename from `deviceId` to `androidId` #1672

Closed jo-elimu closed 1 week ago

jo-elimu commented 1 month ago

To make the code in the Device entity consistent with the code in the Analytics app, rename from Device#deviceId to Device#androidId.

alexander-kuruvilla commented 2 weeks ago

Hi @jo-elimu I would like to take this up. So Im guessing this would require a database migration script as well to change the deviceid attribute to androidid?

jo-elimu commented 1 week ago

Hi @jo-elimu I would like to take this up. So Im guessing this would require a database migration script as well to change the deviceid attribute to androidid?

Thank you, @alexander-kuruvilla. Yes, that's correct. You can find the current DB table structure in the JPA schems export file.

alexander-kuruvilla commented 1 week ago

Hi @jo-elimu, in the JPA schema export file in addition to the Device table, I also see that a unique key is added to the deviceId.

alter table Device add constraint UK_ktkbd0xm3q2nddw1xxtdaxjy7 unique (deviceId);

Should drop the unique key and re-add it for the androidId in the migration script?

jo-elimu commented 1 week ago

@alexander-kuruvilla That is a good question. I'm not sure if Hibernate will automagically update the constraint after you rename the column.

FYI, here is what the table looks like in the production database:

MariaDB [webapp-HIN]> SHOW CREATE TABLE Device;
+--------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Table  | Create Table

                                    |
+--------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Device | CREATE TABLE `Device` (
  `id` bigint(20) NOT NULL AUTO_INCREMENT,
  `deviceId` varchar(255) NOT NULL,
  `deviceManufacturer` varchar(255) NOT NULL,
  `deviceModel` varchar(255) NOT NULL,
  `deviceSerial` varchar(255) NOT NULL,
  `osVersion` int(11) NOT NULL,
  `remoteAddress` varchar(255) NOT NULL,
  `timeRegistered` datetime DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin |

I think it should be okay if we just rename the column, and don't make any changes to the constraint.

This might also be related to our use of ENGINE=MyISAM (instead of ENGINE=InnoDB).

alexander-kuruvilla commented 1 week ago

Alright @jo-elimu , I'll hold off on altering the constraint. After running on local the unique constraint value in the jpa schema export seems to change as well, so I guess it would add the constraints automagically.

jo-elimu commented 1 week ago

@alexander-kuruvilla The migration script could not be executed:

[HIN] 01:32:29.020 [main] INFO  ai.elimu.util.db.DbMigrationHelper - Looking up file "db/migration/2004020.sql"...
[HIN] 01:32:29.020 [main] INFO  ai.elimu.util.db.DbMigrationHelper - Migration script found for version 2004020
[HIN] 01:32:29.021 [main] INFO  ai.elimu.util.db.DbMigrationHelper - Executing sql: ALTER TABLE `Device` DROP COLUMN `deviceId`;
[HIN] 01:32:29.045 [main] INFO  ai.elimu.util.db.DbMigrationHelper - Executing sql: ALTER TABLE `Device` CHANGE `deviceId` `androidId` VARCHAR(255);
[HIN] 01:32:29.047 [main] WARN  org.hibernate.engine.jdbc.spi.SqlExceptionHelper - SQL Error: 1054, SQLState: 42S22
[HIN] 01:32:29.047 [main] ERROR org.hibernate.engine.jdbc.spi.SqlExceptionHelper - Unknown column 'deviceId' in 'Device'
[HIN] 01:32:29.059 [main] ERROR ai.elimu.web.servlet.CustomDispatcherServlet - Context initialization failed
org.springframework.dao.InvalidDataAccessResourceUsageException: could not execute statement; SQL [n/a]; nested exception is org.hibernate.exception.SQLGrammarException: could not execute statement

So I think we just need to replace DROP COLUMN deviceId with DROP COLUMN androidId and run the migration script again. Sorry, I missed that during the PR review.

alexander-kuruvilla commented 1 week ago

Oh ok, so @jo-elimu is this what you meant?

ALTER TABLE Device DROP COLUMN androidId; ALTER TABLE Device CHANGE deviceId androidId VARCHAR(255);

jo-elimu commented 1 week ago

@alexander-kuruvilla Yes 👍

Would you mind creating a pull request for that change?

alexander-kuruvilla commented 1 week ago

@jo-elimu Thanks for that. Sorry, I was a bit busy and didn't check my mail.

jo-elimu commented 1 week ago

@alexander-kuruvilla No worries. Since the deviceId column was dropped earlier, I manually restored it with

`ALTER TABLE `Device` ADD COLUMN `deviceId` VARCHAR(255);`

before running the updated migration script again.

And we haven't started storing any data in the Device table yet, so it's not a problem that the wrong column was dropped.