[2.5] NXCM-5153: NXCM-5161: Core changes for upload resource #799

Merged
merged 8 commits into from Apr 10, 2013

3 participants

@cstamas
Sonatype member

Refactoring and cleanup.

Issues solved (well, indirectly solves):
https://issues.sonatype.org/browse/NXCM-5153
https://issues.sonatype.org/browse/NXCM-5161

Changes:

  • doc'd how upload actually works
  • cleanup: variable and their scope cleanup, formatting cleanup
  • minor logging cleanup
  • support for form field processing extension (in subclasses of abstract upload resource)
  • and the "key change", introduction of new methods (one called once when all the coordinates of upload payload is available), to make subclasses introduce new parameters easier.

The key change is introduction of new method
uploadGavParametersAvailable() that is in this same
class implemented as nop. This method is called by
upload method only once, at the moment when
upload GAVP coordinates are available.

None of these changes modify functionality of upload (nor the Artifact Upload resource is changed in any way). Same thing on UI side. The new changes are used to implement NXCM-5153 and NXCM-5161 in staging plugin.

cstamas added some commits Apr 4, 2013
@cstamas cstamas NXCM-5153: Reformat, no code change eb61783
@cstamas cstamas NXCM-5153: Renamed internal state const
As it had nothing to do with pom generating,
it simply meant "pom parsed".

Just for clarity sake.
70f8a58
@cstamas cstamas NXCM-5153: Core changes needed
This abstract REST resource is used by Staging
Upload resource too. These are the changes needed
to implement what we need for staging.

Changes:
* reverse engineered and doc'd how upload actually works
* variable and their scope cleanup
* minor logging cleanup

The key change is introduction of new method
uploadGavParametersAvailable that is in this same
class implemented as nop. This method is called by
upload method only once, at the moment when
upload GAVP coordinates are available.
09da78a
@cstamas cstamas NXCM-5153: NXCM-5161: More refactoring
Made the upload resources extending this abstract class
introduce and process more form fields, as needed.

Original design made it impossible to extend the set of
processed form fields. This change continues previous
changes and makes this possible from subclasses.
71a75d6
@cstamas cstamas commented on an outdated diff Apr 4, 2013
...xus/rest/artifact/AbstractArtifactPlexusResource.java
- String groupId = null;
-
- String artifactId = null;
-
- String version = null;
-
- String classifier = null;
-
- String packaging = null;
-
- String extension = null;
-
- ArtifactCoordinate coords = null;
-
- PomArtifactManager pomManager =
+ // Every file selected for upload will generate a SEPARATE upload HTTP POST request
@cstamas
Sonatype member
cstamas added a note Apr 4, 2013

This spec should be moved to javadoc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nabcos nabcos commented on the diff Apr 10, 2013
...xus/rest/artifact/AbstractArtifactPlexusResource.java
}
else
{
- // a file
- isPom = fi.getName().endsWith( ".pom" ) || fi.getName().endsWith( "pom.xml" );
+ // a file, this means NO parameters will income anymore
@nabcos
nabcos added a note Apr 10, 2013

FTR, I don't like that this may break when you pass in parameters in the wrong order (files first). Maybe move the upload out of the for-loop?
But it's been like that forever IIUC, so we can leave it like this.

@cstamas
Sonatype member
cstamas added a note Apr 10, 2013

Yes if you read the original (those three lines talking about "nibbles" that are removed with this change) or the new (in javadoc) "specs" how this resource works (and did work, as no change has been introduced in this area), it is a requirement that params must arrive first, then the content(s). And if you think about it, it does make sense: if a huge JAR would arrive before parameters (like "r" for repository ID), where would Nexus put it?

This has been like this forever and was designed like this.

@nabcos
nabcos added a note Apr 10, 2013

My point was that I think if you send the parameter order "r,g,a,v,file,c" the artifact will be deployed without classifier, because it will be deployed inside the loop. If I send "file,r,g,a,v" it will fail altogether. The jar file is placed in a temp storage by fileupload-commons anyway - we can easily hold on to it until we've seen all parameters.

Minor point, as you said it's been like this forever. I still don't like it. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nabcos nabcos and 1 other commented on an outdated diff Apr 10, 2013
...xus/rest/artifact/AbstractArtifactPlexusResource.java
+
+ private String version = null;
+
+ private String packaging = null;
+
+ public String getRepositoryId()
+ {
+ return repositoryId;
+ }
+
+ public void setRepositoryId( String repositoryId )
+ {
+ this.repositoryId = repositoryId;
+ }
+
+ public boolean isHasPom()
@nabcos
nabcos added a note Apr 10, 2013

isHasPom? better rename that field to 'pomAvailable' ;)

@cstamas
Sonatype member
cstamas added a note Apr 10, 2013

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nabcos

+1

@cstamas cstamas merged commit 6469f7c into master Apr 10, 2013
@cstamas cstamas deleted the nxcm-5153-staging-upload-v2 branch Apr 10, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment