fix (web): Prevent shared yearMonth reference corruption in asset upserts (updateObject)

[ghost]

Description

A typo?

Fixes # (issue)

How Has This Been Tested?

• Test A
• Test B

Screenshots (if appropriate)

Checklist:

• I have carefully read CONTRIBUTING.md
• I have performed a self-review of my own code
• I have made corresponding changes to the documentation if applicable
• I have no unrelated changes in the PR.
• I have confirmed that any new dependencies are strictly necessary.
• I have written tests for new code (if applicable)
• I have followed naming conventions/patterns in the surrounding code
• All code in src/services/ uses repositories implementations for database calls, filesystem operations, etc.
• All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic (that belongs in src/services/)

Please describe to which degree, if any, an LLM was used in creating this pull request.

...

Test Files  52 passed (52)
      Tests  474 passed | 1 skipped (475)
   Start at  22:04:26
   Duration  43.52s (transform 310.74s, setup 97.42s, import 561.06s, tests 25.56s, environment 57.85s)

[immich-push-o-matic]

Label error. Requires exactly 1 of: changelog:.*. Found: 🖥️web. A maintainer will add the required label.

[ghost]

not sure whether its expected behavior or not but it was causing tests failure for my changes and according to tyoes in constructors its indeed been using incorrect values

[bwees]

The tests are passing on main.

[ghost]

@danieldietzler
@alextran1502

[ghost]

the issue is its somehow causing an ub, it doesnt even contain neither year nor month so right now it works simply because nothing triggers the error

[ghost]

p.s i just only realized that it unironically got no such fields as year/month at all (guy who close the pr got me confused in whether fix was correct or not rofl), but while testing #26877 it causes the test to fail due to an ub

any better suggestions how to mitigate it?

[alextran1502]

What is your dev environment look like? How does it trigger for you?

[ghost]

updated comment above, but its not about dev env, more like it being unreliable somehow

[alextran1502]

Yeah there were some flaky tests in the past

[ghost]

right now we're at least passing wrong type to the constructor

which for some reason triggers ub in certain point,

can one of you pull the correct fix because i simply assumed it had year and month while destructing but right now realizing theres no such fields at all and i got no idea what to pass lol nevermind the fix was correct

[ghost]

nevermind my fix was correct, they do exist on the type but while passing it down as localDateTime its UB, so can someone re-review and accept the pr?

[ghost]

seems like the existence of a $state in ViewerAsset was covering this bug and it went unnoticeable for a while

[ghost]

Fixed some stuff in code but since pr is closed it wont catch up

[midzelis]

the issue is its a ub, it doesnt even contain neither year nor month so right now it works simply because nothing triggers the error

asset.localDateTime is of type TimelineDateTime

export type TimelineDateTime = TimelineDate & {
  hour: number;
  minute: number;
  second: number;
  millisecond: number;
};
export type TimelineDate = TimelineYearMonth & {
  day: number;
};
export type TimelineYearMonth = {
  year: number;
  month: number;
};

TimelineDateTime is a TimelineYearMonth
It effectively contains all of the following properties:

{
  year: number;
  month: number;
  day: number;
  hour: number;
  minute: number;
  second: number;
  millisecond: number;
}

namely, month and year which is the only thing the constructor for MonthGroup requires.

AFIACT, this change is exactly equivalent to the current behavior, but you create two extra objects.

[ghost]

yeah as you can see i did strike those parts

AFIACT

wouldnt have created pr if it was that