This repository has been archived by the owner on Apr 8, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 15
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
GTNPC-100: PortalContext.VERSION needs to be updated for each release
- Loading branch information
Showing
8 changed files
with
131 additions
and
88 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
103 changes: 103 additions & 0 deletions
103
portlet/src/main/java/org/gatein/pc/portlet/Version.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| /* | ||
| * Copyright (C) 2012 eXo Platform SAS. | ||
| * | ||
| * This is free software; you can redistribute it and/or modify it | ||
| * under the terms of the GNU Lesser General Public License as | ||
| * published by the Free Software Foundation; either version 2.1 of | ||
| * the License, or (at your option) any later version. | ||
| * | ||
| * This software is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU | ||
| * Lesser General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Lesser General Public | ||
| * License along with this software; if not, write to the Free | ||
| * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA | ||
| * 02110-1301 USA, or see the FSF site: http://www.fsf.org. | ||
| */ | ||
|
|
||
| package org.gatein.pc.portlet; | ||
|
|
||
| import java.io.InputStream; | ||
| import java.util.Properties; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
| import java.util.regex.Matcher; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| /** | ||
| * <p>Determine version information from the artifact metadata. When the version cannot be determined, the | ||
| * version <code>0.0.0-GA</code> is used.</p> | ||
| * | ||
| * | ||
| * <p>This class only uses classes provided by the Java Platform in order to be used without any further dependency. | ||
| * Information is logged through {@link java.util.logging.Logger} on purpose instead of any other logger.</p> | ||
| * | ||
| * <p>This class provides a main class that can be executed to check the version manually (hence the reason | ||
| * it does not depend on any other class than the Java Platform):</p> | ||
| * | ||
| * <code>java -cp target/pc-portlet-2.4.1.CR03-SNAPSHOT.jar org.gatein.pc.portlet.Version</code> | ||
| * | ||
| * @author <a href="mailto:julien.viet@exoplatform.com">Julien Viet</a> | ||
| */ | ||
| public class Version | ||
| { | ||
|
|
||
| /** Major and minor extracted. */ | ||
| private static final Pattern MAJOR_MINOR_PATTERN = Pattern.compile("^(\\d)+\\.(\\d)+"); | ||
|
|
||
| /** . */ | ||
| public static final String VALUE; | ||
|
|
||
| /** . */ | ||
| public static final int MAJOR; | ||
|
|
||
| /** . */ | ||
| public static final int MINOR; | ||
|
|
||
| static | ||
| { | ||
| String value = "0.0.0-GA"; | ||
| int major = 0; | ||
| int minor = 0; | ||
| try | ||
| { | ||
| Properties props = new Properties(); | ||
| InputStream in = Version.class.getResourceAsStream("/META-INF/maven/org.gatein.pc/pc-portlet/pom.properties"); | ||
| if (in != null) | ||
| { | ||
| props.load(in); | ||
| String version = props.getProperty("version"); | ||
| if (version != null) | ||
| { | ||
| version = version.trim(); | ||
| if (version.length() > 0) | ||
| { | ||
| value = version.trim(); | ||
| Matcher matcher = MAJOR_MINOR_PATTERN.matcher(value); | ||
| if (matcher.find()) | ||
| { | ||
| major = Integer.parseInt(matcher.group(1)); | ||
| minor = Integer.parseInt(matcher.group(2)); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| Logger.getLogger(Version.class.getName()).log(Level.WARNING, "Could not load version from maven", e); | ||
| } | ||
| VALUE = value; | ||
| MAJOR = major; | ||
| MINOR = minor; | ||
| } | ||
|
|
||
| public static void main(String[] args) | ||
| { | ||
| System.out.println("version=" + VALUE); | ||
| System.out.println("major=" + MAJOR); | ||
| System.out.println("minor=" + MINOR); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43 changes: 0 additions & 43 deletions
43
test/core/src/main/java/org/gatein/pc/test/unit/TestConstants.java
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a backward-compatible change. In particular, this breaks WSRP and might impact other components since it removes publicly available fields. While we could adapt our other components, ideally, this is not a change that should target a minor release.
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GateIn 3.6 (which is not a maintenance of 3.5)
Does it break WSRP in a way you cannot upgrade it trivially ?
If you are not able to upgrade WSRP then we will rollback this change.
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I can still make it and keep the old field that would not be used anymore for keeping this compatiblity. So I will do that before the next release.
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So keeping :
public static final Version VERSION = new Version("GateIn Portlet Container", 2, 4, 1, new Version.Qualifier(Version.Qualifier.Prefix.CR, Version.Qualifier.Suffix.SUFFIX_2), "Community");
Should be sufficient, can you confirm it ?
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking now. Thanks. It shouldn't be too difficult to upgrade the components (I'm only aware of WSRP needing it at the moment) if needed. I just worry about public "API" changes in minor releases.
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes but you may use this in another an earlier wsrp branch and that would be an issue. isn't it the case ?
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main point, I guess, is that the version (whatever form it takes) needs to be available from the API module. Currently, this information is used by WSRP as Consumer Agent identifying GateIn consumers with producers so who knows how this string is used by current users (theoretically, changing this string would amount to changing a browser user agent, though in practice, it most certainly doesn't have the same impact). See: https://github.com/gatein/gatein-wsrp/blob/master/common/src/main/java/org/gatein/wsrp/WSRPConstants.java#L163 to see how it's currently used.
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you care about the portal version or the portlet container version ?
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't care about the specific versions but WSRP cares about the format of the value, it is used to register consumers as defined in the consumerAgent field of the RegistrationData structure: http://docs.oasis-open.org/wsrp/v2/wsrp-2.0-spec-os-01.html#_RegistrationData. Theoretically, producers could adapt content based on that string just as web servers adapt content based on user agent though I don't know that people actually do this with WSRP (especially considering Sharepoint doesn't even respect the consumerAgent format, for example).
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the view to use the portlet container version since I would like our WSRP implementation to be independent of the portal it runs in (as a consumer). Its capabilities are mostly limited by what the portlet container can do, though, granted, a particular implementation in a specific portal might further restrict its capabilities.
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no I mean that there are two versions:
1/ the portal version (i.e the PortalContext but the field gives the version of the PC)
2/ the portlet container version
I would like to know if you care about determining the version of the portal (gatein 3.5) or the version of the portlet container (gatein-pc 2.4)
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok so PC it is!
so this constant should not be on PortalContext at least :-)
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the major / minor sufficient ? can you use the org.gatein.pc.portlet.Version#MAJOR and org.gatein.pc.portlet.Version#MINOR fields ?
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I can use Version since WSRP also depends on pc-portlet, not just pc-api. WSRP also needs a product name but I guess I could hardcode that in WSRP itself.
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, not too sure what the format actually is. Version.Value is the whole String, right (so 2.4.1.CR03-SNAPSHOT at the moment). Version.MAJOR is 2 and Version.MINOR is 4 now, right? Would be nice to be able to get the rest of the version in a separate field as well.
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be moved to the API package if needed (to determine the Version of the API). For the name it will not likely not change soon unless I fork it :-)
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is exactly that. Parsing the patch rest could be easy to do if needed. I tool this approach to make things simpler too. The org.gatein.common Version object was a bit over engineered (by me :-) )
8853db3There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also JBoss changes version format every 2 years, without mentioning the 01 format for numbers introduced by Thomas :-)