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

ArtifactStoreFileSystem timestamp deserialization must be consistent #17

Closed
or-shachar opened this issue Sep 13, 2017 · 4 comments
Closed
Milestone

Comments

@or-shachar
Copy link
Contributor

While maven serialize timestamp to seconds granularity, the Artifact object holds timestamp with type "long" (epoch milliseconds).

To deserialize timestamp ArtifactStoreFileSystem uses GregorianCalendar (maybe cosnider moving to java8 time?), and sets it to the values it finds in the maven serialization.

Calendar cal = new GregorianCalendar();
cal.setTimeZone( TimeZone.getTimeZone( "GMT" ) );
cal.set( Calendar.YEAR, Integer.parseInt( matcher.group( 2 ) ) );
cal.set( Calendar.MONTH, Integer.parseInt( matcher.group( 3 ) ) - 1 );
cal.set( Calendar.DAY_OF_MONTH, Integer.parseInt( matcher.group( 4 ) ) );
cal.set( Calendar.HOUR_OF_DAY, Integer.parseInt( matcher.group( 5 ) ) );
cal.set( Calendar.MINUTE, Integer.parseInt( matcher.group( 6 ) ) );
cal.set( Calendar.SECOND, Integer.parseInt( matcher.group( 7 ) ) );
long timestamp = cal.getTimeInMillis();
int buildNumber = Integer.parseInt( matcher.group( 8 ) );

Problem with this is that if we don't control the milliseconds as well we will get different Long value with each run of this code.

This might not affect most artifact stores but MemoryArtifactStore do expect timestamp match. It causes tests that uses MemoryArtifactStore to fail.

@or-shachar
Copy link
Contributor Author

closing for new travis run

@or-shachar
Copy link
Contributor Author

opening after #19 was merged

@or-shachar or-shachar reopened this Sep 18, 2017
or-shachar added a commit to or-shachar/mrm that referenced this issue Sep 18, 2017
@rfscholte
Copy link
Member

This one is already fixed, right?

@or-shachar
Copy link
Contributor Author

indeed

@ppalaga ppalaga added this to the 1.2.0 milestone Jun 21, 2022
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

No branches or pull requests

3 participants