Re: [Spacewalk-devel] devel repo: sync channels from dump
On 26/10/2015 15:30, Tomas Lestach wrote: Would you, please, add it to the main PR, please? We would tag and build it and Pavel may check, whether it helps. Done, thanks! -- Silvio Moioli SUSE Manager Development Team ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] devel repo: sync channels from dump
On 15/10/2015 11:45, Pavel Studeník wrote: Hello, I think that syncing channels from devel repo is broken. [...] satellite-sync -m dumps -c rhel-i386-server-5 -c rhel-i386-server-vt-5 I am attaching a patch which should help - unfortunately we are not able to access the channels you mention so it's difficult for us to test that. Please give the patch a try and report back if it works. Thanks! -- Silvio Moioli SUSE Manager Development Team -- Silvio Moioli SUSE Manager Development Team >From 76694b63d50f7c79d9cf5a6a3de88675976f1120 Mon Sep 17 00:00:00 2001 From: Michael Calmer <m...@suse.de> Date: Thu, 22 Oct 2015 11:54:22 +0200 Subject: [PATCH] ignore all not any longer supported entitlements --- backend/server/importlib/archImport.py | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/backend/server/importlib/archImport.py b/backend/server/importlib/archImport.py index d5199fa..21105b7 100644 --- a/backend/server/importlib/archImport.py +++ b/backend/server/importlib/archImport.py @@ -144,9 +144,12 @@ class ServerGroupServerArchCompatImport(BaseArchCompatImport): arches2_field_name = 'server_group_type' submit_method_name = 'processServerGroupServerArchCompatMap' -# monitoring is no longer supported, ignore any monitoring info for +# some entitlements are no longer supported, ignore any of them for # backwards compatibility def _postprocess(self): -self.batch[:] = [entry for entry in self.batch if not - entry[self.arches2_name] == 'monitoring_entitled'] +self.batch[:] = [entry for entry in self.batch if + entry[self.arches2_name] not in [ + 'monitoring_entitled', 'sw_mgr_entitled', + 'provisioning_entitled', 'nonlinux_entitled', + 'virtualization_host_platform']] BaseArchCompatImport._postprocess(self) -- 2.1.4 ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Travis CI for Spacewalk git
On 05/06/2015 16:32, Jan Dobes wrote: as you may already noticed, we enabled Travis CI [1] for Spacewalk Github account several weeks ago. Since then, few bugs were fixed and now it works with every package in our git except some thirdparty packages in 'spec-tree' directory (if they are explicitly requiring Oracle RPMs or are not buildable on Fedora 21 for any reason). Thank you Jan, great work, this really helps! If you want to integrate Java or Python unit tests I think we can help - we have a homegrown Docker infrastructure to run them through Jenkins at every commit. I guess that needs to be adapted to be used in Travis, but it should be doable and it would be a further step in the same direction. Regards, -- Silvio Moioli SUSE Manager Development Team -- Silvio Moioli SUSE LLC Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Introducing tetra
On 16/04/2015 14:31, Tomas Lestach wrote: Feel free to enhance the docs, that you need to install: [...] Documentation updated, thanks! Actually compilation of commons-collections-3.2.1 failed on my machine. (Errors below.) Ouch, looks like you found a known incompatibility between Commons Collections 3.x and JDK 8: https://issues.apache.org/jira/browse/COLLECTIONS-527 Solution would be to either use JDK 7 or patch sources. I updated the example to Commons Collections 4 not to confuse first-time users. Thanks for reporting. I know that building of java packages with all their dependencies and dependencies of their dependencies and ... is pain. However, I do not think we would adopt tetra for building Spacewalk java packages. The most java packages we use from the old jpackage repository [1] as they are and honestly - we're happy we do not need to (re)build, fix and maintain them. Okay, will keep that into account. Thanks! -- Silvio Moioli SUSE LLC Maxfeldstraße 5, 90409 Nürnberg Germany -- Silvio Moioli SUSE LLC Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] Introducing tetra
Dear Spacewalk developers, in the SUSE Manager team we have been looking for ways to simplify the packaging of Java software, in particular Maven based projects, since some time. Now we have finally settled on a solution, so I am glad to present it here today and I would like to gather some feedback from you. It boils down to this: 1) we allow prebuilt (binary JAR) build dependencies. This means that all software we ship is still completely built from source, but its build chain, which we don't ship, is mostly copied in binary form from the Maven Central repository instead of being rebuilt from scratch; 2) we make use of a tool I wrote, tetra, to greatly simplify the process given the above assumption. https://github.com/moio/tetra As an example, I could get basically all Spacewalk Java dependencies to build for openSUSE in two weeks - without it would have easily taken months. My main question now now is: if we update versions of libraries such as commons-lang, commons-collections or Hibernate and refactor Spacewalk's code accordingly, would you accept contributions in the form of specfiles/tarballs generated with tetra, or anyway assuming 1)? Comments, thoughts and further discussion are of course very welcome. Silvio PS: currently tetra outputs SUSE-centric specfiles, but they are so simple that tweaking templates for Fedora or Red Hat distros would be trivial. -- Silvio Moioli SUSE LLC Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Spacewalk test suite/SQL validation?
On 19 February, 2015, at 21:28, Avi Miller avi.mil...@oracle.com wrote: I found your spacewalk-test-suite here: https://github.com/SUSE/spacewalk-testsuite-base I was able to get this working on an OpenSUSE 13.2 VM to test a Spacewalk 2.2 instance with its repository on Oracle RDBMS 12c. Obviously that test suite is designed for SUSE Manager, so a fair amount of tweaking is required to get it to pass on vanilla Spacewalk, but at least a lot of test cases are already in place, so it's just a case of tweaking the features to look for the right text/response. That is actually our Cucumber _integration_ test suite, which is separate from the jUnit and Python unit test suites which you find in the main Spacewalk code tree. I was actually referring to those unit tests in my previous message. Pros: it actually tests the product “end to end” by driving a real browser. Cons: it is much more work to set up and to adapt to Spacewalk wrt the unit tests which basically work out of the box. As you mentioned our project targets SUSE Manager currently. I suppose a more general question is: what level of testing would be required for you guys to be happy if I submit a pull request allowing installation on a 12.1 database? From the SUSE Manager team's perspective if your patches result in all unit tests to run correctly and some minimal manual testing is done (setup - register one system - patch it) then it would probably be enough, you can count on my vote at least. We can always run the Cucumber integration test suite on SUSE Manager nightly once the PR is approved and report any bugs afterwards. Hth, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Outstanding pull requests
On 02/02/2015 15:06, Cliff Perry wrote: Hi folks, the amount of outstanding pull requests has gone down a lot. We only have 8 remaining. Yes! Thanks a lot for your efforts. Is there any remaining which would be good to get done before SW 2.3 was branched? - Any of high priority that are being advocated above the rest. Assuming we want to run all unit tests at least once before release, this one should be useful to speed up work: https://github.com/spacewalkproject/spacewalk/pull/216 Among others, JSP fixes are particularly welcome to minimize downstream conflicts so I would also recommend this: https://github.com/spacewalkproject/spacewalk/pull/200 Thanks, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] RFC: Action Chaining mockups and ideas
On 03/03/2014 06:44 PM, Lamont Peterson wrote: Would it not make the most sense for Action Chains to be defined without any systems, then to schedule an action chain to run against either a single system or SSM? It could, but there are two problems with that approach: 1) it requires to basically duplicate a lot of the user interface just for Action Chaining. To install a package, for example, you have a page that allows you to select a channel - such logic should be replicated in the Action Chaining UI. Same goes for all Actions (select a configuration channel to select a configuration file to deploy, select an installed package to remove it, etc). From a development perspective, this is easily some extra months, which was considered too expensive; 2) it requires to either rewrite or ignore all checks that are currently done before scheduling Actions - for example with current UI you cannot install a package on a system if it is not subscribed to the appropriate channel, and you cannot run a remote command if the system is not configured to do so. Defining an abstract Action Chain and applying it later to a set of systems would require to apply all those checks in a different point in time and with a different logic, which basically boilsdown to rewriting them or refactoring them heavily. Current implementation has the advantage of reusing current workflow thus inheriting those checks for free. Again your proposal is not impossible, but it is much harder. Volunteers welcome :-) Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] On picking a standard on HTML5 void tags (was: Date/Time picker)
On 02/17/2014 04:43 PM, Matej Kollar wrote: * standard allows closing of tags, Let me rephrase this: standard allows adding an optional trailing slash to void elements, that by definition do not have end tags, as syntax sugar[0]. Non-void elements must have a start and end tag - closing and self-closing are not defined by the standard. So standard allows both adding and not adding, and IMO it's a matter of choosing a coding convention. * HTML is generated on some places explicitly... are you sure it would not lead to state where we would be inconsistent (=bad), I think that the goal of a coding convention is mostly patch consistency - picking one style to avoid unneeded git conflicts because of different programmers having different habits. Goal here is not being nice on browsers' lexers IMO. Actually I would say that this case is very similar to the problem of having different whitespace conventions in JSPs (2 spaces, 4 spaces, tabs) - since browsers accept anything the main concern should be setting a rule for any future code contribution to minimize conflicts, not necessarily fixing all inconsistencies right now. * in case we switch (and do it properly and completely) it would make it much harder to switch to say XHTML if desired, We are focusing on Bootstrap and HTML5 only for the foreseeable future. * without closing tags it would be like those evil old times with wild HTML... I cannot see your point here - HTML5 does not have those slashes by default, whether that looks modern or not really disputed. (not speaking about omitting closing tags...) We are still committed to valid code, of course. Omitting end tags when they are required by the standard is a whole different problem - code that does not validate is wrong and should be fixed, as always. * code that Michael commented on was wrong anyway as it only deleted symbol and immediately pended it. Thread renamed to keep the two discussions separated. On the other hand, I would argue that: * those slashes are really superfluous; * a good portion of Web projects outside Spacewalk are following the same convention, among others Bootstrap itself[1] and HTML5 Boilerplate[2], which is also used in Initializr[3] which in turn are used in lots of projects. I do acknowledge that debate is ongoing[4] and there is no ultimate agreement in the wider Web development community, so I do not want to start and endless discussion here trying to solve that. So all I have is one last question: Is this your own personal opinion or the whole Spacewalk team official position? In particular, will future patches be rejected if they have those trailing slashes? Thanks [0] http://dev.w3.org/html5/spec-author-view/syntax.html#syntax-start-tag [1] Bootstrap itself. See br, link, etc in official examples: http://getbootstrap.com/examples/theme/ [2] http://html5boilerplate.com/ [3] http://www.initializr.com/ [4] http://www.reddit.com/r/web_design/comments/uvvxg/html5_developers_do_you_self_close_your_tags_or/ -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] On picking a standard on HTML5 void tags
On 02/18/2014 09:46 AM, Silvio Moioli wrote: So all I have is one last question: Is this your own personal opinion or the whole Spacewalk team official position? In particular, will future patches be rejected if they have those trailing slashes? Oops, typo there. I really meant: In particular, will future patches be rejected unless they have those trailing slashes? Sorry! -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Query question regarding system.listLatestUpgradeablePackages
On 02/13/2014 05:43 AM, Jeremy D Davis wrote: My question is why use this really big query when you could use a select * from rhnserverneededcache where serverid=x and pull all the packages that way. Granted you would need to do some joins to get the correct names based on the package id but it just seem like rhnserverneededcache would be a better place to obtain the information than this long query is doing. I have not investigated all minor aspects at the moment, but a big difference I see is that rhnserverneededcache only tells you about the outdated package and not the new one. Determining the to_version, to_release, to_epoch and to_package_id outputs of listLatestUpgradablePackages is actually non trivial and I suspect it adds up quite a lot of computation. Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] Date/Time picker
On 02/14/2014 03:16 PM, Michael Mraka wrote: should likely be / otherwise it produce opening tags not self closed tags. Actually in HTML5 void tags do not need the slash (it is optional). http://www.w3.org/TR/html-markup/syntax.html#void-element self-closing as a definition does not exist anymore in HTML5, except for SVG and MathML elements! We also recently started a discussion about this same topic and decided to stick to a coding standard rule to avoid confusion - namely not using those optional slashes in new code. I hope you are also fine with that. I was supposed to start a discussion with you in the next days, but this was faster :-) Thanks, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] RFC: Action Chaining mockups and ideas
On 02/14/2014 01:05 PM, Cliff Perry wrote: http://turing.suse.de/~smoioli/action-chaining-mockups/list.html I suspect this page really wants a column for when is the event scheduled to be executed. Not really - those Action Chains are not scheduled for execution yet. When they actually get scheduled, those become regular actions in Schedule - Pending Actions (and later Completed Actions, Failed Actions, etc.). Merge/use either only created or modified, vs both - I don't think we need them both. Okay, agreed. Task scheduled for next sprint. http://turing.suse.de/~smoioli/action-chaining-mockups/editor.html This is a bit worrying, it looks like it would make it hard/complex to render information meaningfully. Because they are chained and you have different systems for each portion of the event - does it mean, you don't reboot the 10 systems, if one of the two config deployment fails? No, the chaining is done per-system, so if an action fails on a particular system then only subsequent actions in the same Chain for the same system will fail. See: http://wiki.novell.com/index.php/SUSE_Manager/ActionChaining#Definitions Rendering basically works at the moment, I still have to fix a couple of bugs but it looks very similar to that mockup. I already planned to prepare a screencast for you during the next sprint so that you can also see it. While the power to add/remove systems for each portion sounds nice, it makes my brain hurt, and I'd have to assume it would for those managing the systems - selecting X systems and being stuck to work with them only, sounds more sane approach. We do expect that users will mostly use the SSM with a fixed set of systems to chain actions, and this will be the recommended use case anyway. Please note that using the SSM automatically implies having chains with actions that are executed on different sets of systems. This is because, in general, you do not get one action per system that you have in the set as some of them might not meet the prerequisites (entitlements, channels, etc.). So, from a UI perspective, we could not really do much more than reflect that fact, so we came up with that tree-like visualization scheme. From there, adding the capability of deleting a system from an action was just a low-hanging fruit, it could also be disabled easily if you feel that it would hurt usability. Adding a system to an existing action in a chain is not possible at the moment. I don't see anything bad in getting this into Spacewalk, or that would go against Spacewalk's future desires. Great news! I will be back with a screencast soon. Thanks a lot, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Things that needs to be dobne before next release.
On 01/09/2014 01:26 PM, Matej Kollar wrote: Major things that we currently consider release blockers for 2.1 from UI point of view are: [...] * Bare-metal systems management May I ask what's the status of that? Is it still planned for 2.1? More importantly, is there something that prevents merging? We can help in that case. From our side, we just added custom icons and I am planning to display some more relevant data in the Bare-metal System List page in the next weeks. Thanks, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] Date/Time picker
On 02/06/2014 11:12 AM, Duncan Mac-Vicar P. wrote: See attached patches. [...] On a slightly unrelated note, I have just noticed the current date picker allows incorrect dates to be picked, such as February 31. In the SSM errata scheduling page, among other things, such values cause a NPE. Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Action chaining - table names
I've come across commit 8844fcb10460d7d9838e89e376e09e7b1539b3fe which adds new tables for action chaining feature. I noticed all table/trigger/constraint names have 'suse' prefix. I guess I understand why you named it that way :). Anyway majority of tables have rhn prefix but it isn't because they were created by Red Hat engineers rather then just a legacy prefix from old days when Satellite was born as Red Hat Network (and many years later opensourced as Spacewalk project). So we continue to use rhn prefix even if the project is called differently nowadays because renaming of all tables is just not worth of it. To sum it up - I'd like new tables to continue with old rhn prefix naming. Hi Michael, first of all thanks a lot for reviewing that branch during development - that was exactly what I hoped for! It was my understanding that using the suse prefix was standard practice for schema parts that were contributed by us, as there are other tables with that prefix - evidently I was wrong. No problem though, I fixed those commits already. Thanks again, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Action chaining - new js files
On 01/23/2014 01:56 PM, Michael Mraka wrote: Last week I've finaly cleaned up branding and web packages from third party stuff (bootstrap, font-awesome, roboto) and put it into separate packages. I'd like to ask you also put it into independent packages. It helps us to keep our code sparated from others work, track where it comes from, what's its license etc. Sure. At the moment I just kept them in separate commits, as they will also be packaged in SUSE Manager differently. It is just handy to have them in the web/ directory while developing and reviewing; they will be removed when code is ready to merge. By the way I am not familiar with the Spacewalk RPM build system and conventions so I might need some help - when everything else is finished I will ask you again. Thanks, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] Single-system reboot page ported to Java
On 01/21/2014 01:26 PM, Matej Kollar wrote:
* $Rev$ is legacy of our old versioning system (cvs) and has no use
now that we use git (and we would not like to see it in new code).
Just to have everyone informed: I used those tags because they were
required by checkstyle_eclipse.xml. Matej will fix that configuration
file, meanwhile I fixed the patch.
By the way I also fixed my review-pending branches with these commits:
e9ac621 - master-pxe-default-image-no-powermanagement
9e04ed5 - master-power-management-ssm
* Preferred space indentation for `.jsp` files is 2 spaces.
As discussed, since there is no current standard in Spacewalk and the
Oracle code conventions for JSP is four spaces[1], we will stick with
four. Of course we can revert the decision later if a decision is taken.
* Use of `property=dispatch` for submit button isn't needed
when the only thing you actually check is `isSubmitted(form)`. [...]
Agreed, fixed.
* Confirm button -- I would prefer Reboot system that was used on
`.pxt` page..
Fixed.
* Using sid: [...] one sid is enough.
Okay! :-)
Thanks for your review and thorough explanations.
Regards
[1]
http://www.oracle.com/technetwork/articles/javase/code-convention-138726.html
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 51fc76e6de18e6e93f38652f389edf39976d5c38 Mon Sep 17 00:00:00 2001
From: Michael Calmer m...@suse.de
Date: Thu, 16 Jan 2014 14:08:11 +0100
Subject: [PATCH] port reboot_confirm.pxt from perl to java
---
.../action/systems/sdc/SystemRebootAction.java | 88 ++
.../frontend/strings/jsp/StringResource_en_US.xml | 24 ++
java/code/webapp/WEB-INF/nav/system_detail.xml | 2 +-
.../webapp/WEB-INF/pages/systems/sdc/overview.jsp | 4 +-
.../WEB-INF/pages/systems/sdc/rebootsystem.jsp | 52 +
java/code/webapp/WEB-INF/struts-config.xml | 13
web/include/nav/system_detail.xml | 2 +-
7 files changed, 181 insertions(+), 4 deletions(-)
create mode 100644 java/code/src/com/redhat/rhn/frontend/action/systems/sdc/SystemRebootAction.java
create mode 100644 java/code/webapp/WEB-INF/pages/systems/sdc/rebootsystem.jsp
diff --git a/java/code/src/com/redhat/rhn/frontend/action/systems/sdc/SystemRebootAction.java b/java/code/src/com/redhat/rhn/frontend/action/systems/sdc/SystemRebootAction.java
new file mode 100644
index 000..5e4c998
--- /dev/null
+++ b/java/code/src/com/redhat/rhn/frontend/action/systems/sdc/SystemRebootAction.java
@@ -0,0 +1,88 @@
+/**
+ * Copyright (c) 2014 SUSE
+ *
+ * This software is licensed to you under the GNU General Public License,
+ * version 2 (GPLv2). There is NO WARRANTY for this software, express or
+ * implied, including the implied warranties of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+ * along with this software; if not, see
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+ *
+ * Red Hat trademarks are not licensed under GPLv2. No permission is
+ * granted to use or replicate Red Hat trademarks that are incorporated
+ * in this software or its documentation.
+ */
+package com.redhat.rhn.frontend.action.systems.sdc;
+
+import com.redhat.rhn.common.util.DatePicker;
+import com.redhat.rhn.domain.action.Action;
+import com.redhat.rhn.domain.action.ActionFactory;
+import com.redhat.rhn.domain.server.Server;
+import com.redhat.rhn.domain.user.User;
+import com.redhat.rhn.frontend.struts.RequestContext;
+import com.redhat.rhn.frontend.struts.RhnAction;
+import com.redhat.rhn.frontend.struts.RhnHelper;
+import com.redhat.rhn.manager.action.ActionManager;
+import com.redhat.rhn.manager.system.SystemManager;
+
+import org.apache.struts.action.ActionForm;
+import org.apache.struts.action.ActionForward;
+import org.apache.struts.action.ActionMapping;
+import org.apache.struts.action.DynaActionForm;
+
+import java.util.Date;
+import java.util.Map;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+/**
+ * SystemRebootAction handles the interaction of the system reboot.
+ */
+public class SystemRebootAction extends RhnAction {
+/** Success forward name. */
+private static final String CONFIRM_FORWARD = confirm;
+
+/** {@inheritDoc} */
+@Override
+@SuppressWarnings(unchecked)
+public ActionForward execute(ActionMapping mapping, ActionForm formIn,
+HttpServletRequest request, HttpServletResponse response) {
+
+DynaActionForm form = (DynaActionForm) formIn;
+RequestContext context = new RequestContext(request);
+User user = context.getLoggedInUser();
+MapString, Object params = (MapString, Object) makeParamMap(request);
+String forward = RhnHelper.DEFAULT_FORWARD;
+
+Long sid = context.getRequiredParam(RequestContext.SID);
+Server server = SystemManager.lookupByIdAndUser(sid, user);
+
+if (isSubmitted(form
[Spacewalk-devel] [PATCH] Single-system reboot page ported to Java
Dear Spacewalkers,
Those two patches are from my colleague Michael Calmer and they contain
ported Java code for the reboot page. Patch 0002 removes stale Perl
code; HTML has been adapted for Bootstrap already.
Bootstrap porting of the Date picker widget, as you know, is being
tackled separately.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From c6c2050412339f1d9fe789dc57f8b170e750ad31 Mon Sep 17 00:00:00 2001
From: Michael Calmer m...@suse.de
Date: Thu, 16 Jan 2014 17:20:53 +0100
Subject: [PATCH 2/2] remove unused perl code after porting reboot_confirm.pxt
to java
---
.../network/systems/details/reboot_confirm.pxt | 39 --
web/modules/rhn/RHN/DB/Scheduler.pm| 22
web/modules/sniglets/Sniglets/Servers.pm | 26 ---
3 files changed, 87 deletions(-)
delete mode 100644 web/html/network/systems/details/reboot_confirm.pxt
diff --git a/web/html/network/systems/details/reboot_confirm.pxt b/web/html/network/systems/details/reboot_confirm.pxt
deleted file mode 100644
index 898ff1f..000
--- a/web/html/network/systems/details/reboot_confirm.pxt
+++ /dev/null
@@ -1,39 +0,0 @@
-?xml version=1.0 encoding=utf8?
-pxt-passthrough
-
-pxt-use class=Grail::Frame /
-pxt-use class=Sniglets::Servers /
-pxt-use class=Sniglets::Sets /
-pxt-use class=Sniglets::HTML /
-pxt-use class=Sniglets::ServerActions /
-
-
-grail-canvas-template base=/templates/c.pxt mode=main_canvas
-pxt-formvar
-pxt-include-late file=/network/components/systems/system_details_toolbar.pxi /
-/pxt-formvar
-
-rhn-navi-nav prefix=system_details file=/nav/system_detail.xml style=contentnav /
-
-pxt-include-late file=/network/components/message_queues/local.pxi /
-
-rhn-up2date-at-least version=2.9.3 release=2.2.1AS
-
-h2System Reboot Confirmation/h2
-
-pThis will schedule a reboot of this system./p
-
-
-pxt-form method=post
-pIf you are strongcertain/strong you wish to do this, you may schedule the reboot to take place as soon as possible, or no earlier than a specified time:/p
-
-rhn-schedule-action-interface action=schedule_errata_updates callback=rhn:reboot_server_cb label=Schedule Reboot /
-
-pxt-hidden name=sid /
-
-/pxt-form
-
-/rhn-up2date-at-least
-
-/grail-canvas-template
-/pxt-passthrough
diff --git a/web/modules/rhn/RHN/DB/Scheduler.pm b/web/modules/rhn/RHN/DB/Scheduler.pm
index a23043f..cf190db 100644
--- a/web/modules/rhn/RHN/DB/Scheduler.pm
+++ b/web/modules/rhn/RHN/DB/Scheduler.pm
@@ -175,28 +175,6 @@ EOQ
}
}
-
-sub schedule_reboot {
- my $class = shift;
- my %params = @_;
-
- my ($org_id, $user_id, $server_set, $server_id, $earliest, $prerequisite, $transaction) =
-map { $params{- . $_} } qw/org_id user_id server_set server_id earliest prerequisite transaction/;
-
- my ($action_id, $stat_id) = $class-make_base_action(-org_id = $org_id,
- -user_id = $user_id,
- -type_label = 'reboot.reboot',
- -earliest = $earliest,
- -prerequisite = $prerequisite,
- -transaction = $transaction);
-
- $class-add_servers_to_action($action_id, $stat_id, $user_id, $server_set, $server_id);
-
- osa_wakeup_tickle();
-
- return $action_id;
-}
-
sub sscd_schedule_reboot {
my $class = shift;
my %params = @_;
diff --git a/web/modules/sniglets/Sniglets/Servers.pm b/web/modules/sniglets/Sniglets/Servers.pm
index 792dd3d..13a03e3 100644
--- a/web/modules/sniglets/Sniglets/Servers.pm
+++ b/web/modules/sniglets/Sniglets/Servers.pm
@@ -71,8 +71,6 @@ sub register_callbacks {
my $class = shift;
my $pxt = shift;
- $pxt-register_callback('rhn:reboot_server_cb' = \reboot_server_cb);
-
$pxt-register_callback('rhn:server_prefs_form_cb' = \server_prefs_form_cb);
$pxt-register_callback('rhn:ssm_change_system_prefs_cb' = \ssm_change_system_prefs_cb);
@@ -313,30 +311,6 @@ sub server_history_event_details {
return $params{__block__};
}
-sub reboot_server_cb {
- my $pxt = shift;
- my $sid = $pxt-param('sid');
- die no server id unless $sid;
-
- my $server = RHN::Server-lookup(-id = $sid);
- die no server obj unless $server;
-
- my $earliest_date = Sniglets::ServerActions-parse_date_pickbox($pxt);
- my $action_id = RHN::Scheduler-schedule_reboot(-org_id = $pxt-user-org_id,
- -user_id = $pxt-user-id,
- -earliest = $earliest_date,
- -server_id = $sid);
-
-
- my $pretty_earliest_date = $pxt-user-convert_time($earliest_date);
-# my $message = sprintf(EOM, $server-name, $server-name, $action_id);
-#Reboot scheduled for system strong%s/strong for $pretty_earliest_date. To cancel the reboot, remove strong%s/strong from a href=/rhn/schedule/InProgressSystems.do?aid=%dstrongthe list of systems to be rebooted/strong/a.
-#EOM
-#
-# $pxt-push_message(site_info = $message);
- $pxt-redirect(/rhn/systems/details/Overview.do?sid=$sidmessage=system.reboot.scheduledmessagep1= . $server-name . messagep2= . $pretty_earliest_date
Re: [Spacewalk-devel] less.js development mode broken
On 01/14/2014 01:56 PM, Duncan Mac-Vicar P. wrote: I know I gave you the tip of the absolute path to you but there is something I did not see: our lesscss-engine does not support search paths, therefore in now looks in /. and spacewalk-branding does not build. Sorry for the confusion, fixed in b7e3621. Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Things that needs to be dobne before next release.
On 01/09/2014 01:26 PM, Matej Kollar wrote: * Bare-metal systems management Some more automatic tests are being worked on in SUSE Manager by my colleague Martin Seidl. I will commit patches for any bug that is discovered if it also affects Spacewalk. Apart from that, coding is considered finished SUSE Manager side, and I also consider the Spacewalk rebase basically finished - please report any defects or issues that block upstreaming so that I can try to find a solution. Thanks, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] RFC: Action Chaining mockups and ideas
Dear Spacewalk Community, here at SUSE we are about starting development on a new feature called Action Chaining, and we would like to try and get you involved from the very beginning this time. This is about scheduling a chain of actions on a server or a group of servers, instead of single operations as it is currently possible. A chain is a sequence of operations like package installs, configuration changes, reboots, etc. to be executed one after another when scheduled. To limit development time, we currently plan on supporting a subset of actions based on users' feedback we collected so far. You can find a complete list, together with a more precise description of this feature, some screenshots and mockups here: http://wiki.novell.com/index.php/SUSE_Manager/ActionChaining Note: these mockups have a SUSE Manager look and feel since they have also been used for internal discussions in SUSE. Please simply ignore the styling at this point. We appreciate your input about this, especially if there is any aspect that you consider blocking for upstreaming. Development on this feature will take place on the master-action-chaining branch on the Spacewalk repo and during in the coming weeks. I plan to regularly push commits there so that you can comment on code at any time. Hopefully this will ease acceptance checks and merging when development ends. Thanks for your cooperation in advance, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] Adding a password strength meter to spacewalk
On 12/20/2013 05:15 PM, R P Herrold wrote: This seems to pull in new dependencies, doesn't it? This merely adds some new Javascript files - that is standard practice in Spacewalk AFAIU, since packaging rules for Web resources in Fedora are still in draft and the Spacewalk community has not decided to adopt them yet: https://fedoraproject.org/wiki/JavaScript_libraries_packaging_guideline_draft Also, how would this interact with non-local authentication interfaces such as LDAP? It won't. This is just a client-side (in-browser, Javascript) clue to the user. All it does is giving basic suggestions on how to improve a password, it does not enforce any rule. Mandatory password complexity checks and the whole authentication code path is left untouched. HTH, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] RFC: manage bare-metal systems from Spacewalk
On 12/19/2013 01:44 PM, Michael Mraka wrote:
So do we need entitle such system at all? Nowadays there can be
unetitled system in spacewalk. Does bootstrap entitlement brings us
something more than current unetitled system?
Short answer: yes. Among other things, a bootstrap-entitled system can
be powered back on and provisioned.
Long answer: we use the bootstrap entitlement to distinguish bare-metal
systems from others in the Web UI: they get a different icon, some forms
are disabled where they do not make sense, we added a bare-metal
specific search page, etc.
Basically we want the sysadmin to have an inventory of bare-metal
systems where the only possible operations are:
- display some hardware details;
- kickstart;
- do power management;
- move between organizations.
Also, we could not find out how to create an activation key without any
entitlement. It seems like both an entitlement and a base channel are
needed in the Web UI, and even with hand-written SQL you need an
entitlement to insert a line in rhnRegTokenEntitlement, otherwise you
get an invalid token.
Server-side error:
rhnServer/server_token.fetch_token('ERROR', Invalid token '1-KEY_NAME')
Client-side error:
Could not find token '1-nokey'
Error Class Code: 60
Error Class Info:
The activation token specified could not be found on the server.
Please retry with a valid key.
[automatic shutdown...] That's a feature of the boot image, right?
Yes.
So I could achieve the same with 'poweroff' in post install kickstart script.
I guess so, even though I do not understand why you would want to do that.
Idea is that the boot image is used by default before any kickstart. We
want any new bare-metal system to automatically appear in Spacewalk as
soon as it is connected to the network and powered on, to have it
inventoried and ease a subsequent kickstart.
Because it might take a long time (even months) between inventory and
kickstart, we automatically shut the system down in between.
Yes, we can't create such boot image in satellite right now. So I'd have
to create external distribution with memory only image and import it to
satellite. Then I'd have to create an activation key which asigns no base
channel and no entitlements to systems. And I'd also have to manage
kickstart based on this distribution + activation key to be the default
pxe image. Is it all I need?
I am not sure to understand who I refers to in the above sentences -
is it you as a Spacewalk developer or user?
With this contribution we propose to add a pre-built bootstrap image to
the files installed by Spacewalk, users should not edit it in any way.
This image gets automatically configured as default in Cobbler as soon
as the feature is enabled in the Web UI. Enabling will also create a
user-invisible activation key used by the bootstrap image on bare-metal
systems. That special key will only assign a user-invisible bootstrap
entitlement that, in turn, will be used later used to distinguish such
bare-metal systems visually.
Just trying to figure out whether we can't achieve the same goal
somehow easier with the current code...
We obviously also thought hard about this, but could not find out a
simpler alternative. Suggestions are welcome, as always.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] RFC: manage bare-metal systems from Spacewalk
However I see the patches are applied on top of the power management feature and we haven't decided yet, whether to accept this feature. Are these patches independent from the power management feature, so I could apply them right now? As I wrote, they are largely independent from a logical point of view, but stacked for convenience reasons. That boils down to the fact that some files were necessarily modified by both features (translation files, configuration files, a handful jsps and the ServerTestUtils class), so git will always complain about some conflicts if patches are applied in the wrong order. Anyway I just cherry-picked the pxe branch on top of master and pushed the result into this new branch: master-pxe-default-image-no-powermanagement You can review those inter-dependent parts in the Conflicts: section of comments in this branch. Of course, if this feature gets merged first, I will rebase power management commits on top of master again. I see a danger when looking at the changes in jsp and jspf files. Now, during the bootstrap integration, when the jsps get changed every day, we'll get at lot of merge conflicts. :( We do actually get conflicts, and AFAIK that is hard to avoid at the moment. Feeling your pain I rebased and re-pushed all branches on top of current master today. Hopefully Twitter Bootstrap patches are going to slow down, so this issue should be less severe in future - of course the sooner those branches get merged, the better. HTH, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Pushing to spacewalk.git
Hi, I've noticed recently that our git repo is a bit weird looking. I'm talking about commit sequences a291087ad5ef010074ad2d4b1679fcc2cdf667c5..0f49b04dc9c6062a2fd6f480449270123ff7f6b7 and 742472c23d67b6d62730c329d09e1bdac63bd22b..0bfcd3c4029fcf886e3ab5001a7dc75860457095. What's wrong with them? As for code changes they are OK. The way they've been forked / merged / re-merged and finally pushed made git repo pretty messy. As human brain is mostly used to think in linear timeline it's easy e.g. to look for bugs in linear sequence of code changes but you will very quickly get lost in the number of parallel branches. That's why our golden rule is - don't merge, rebase - it's an easy way to keep git repo as much linear as possible. First of all, I am sorry for the confusion. The reason for such a strange-looking history depends on the fact that multiple developers from the Spacewalk community and multiple developers at SUSE are concurrently committing a big number of fixes to the user interface due to the recent Twitter Bootstrap changes. Since those changes are many (currently tens of commits a week), we agreed on having a single branch for all fixes from SUSE employees (master-bootstrap-css-fixes). This allows me to push it frequently to the Spacewalk repo instead of sending individual patches via email, thereby greatly simplifying code submission. Naturally, commits from this branch are to be applied to master (either by merging or rebasing/cherry-picking) by others in the Spacewalk community in order to ensure proper reviewing. We also need to keep this branch updated since master changes quite frequently. Among other things, this ensures our fixes can be applied cleanly after being reviewed without extra work. Of course we can do that by rebasing, but it will require force pushing/history rewriting, which is not very convenient since we have multiple developers and designers working on that (currently 6 people). So we prefer merging master into master-bootstrap-css-fixes from time to time, because this does not require force pushing, that is talking to/stopping 6 people every time master changes significantly (and it does, see for example 559ab061). I asked Tomáš Kašpárek if this was acceptable: moio tkasparek: since various people work on master-bootstrap-css-fixes here, we do not really like force pushing. At the moment we are merging master back to the branch from time to time - I hope that you don't mind some extra merge commits tkasparek moio: sure, just do whatever suits you the best moio tkasparek thanks tkasparek np I hope this is not really a difficult issue, as I am happy with the current upstream patch flow that, being quite fast, allows lots of bugs to be solved efficiently. Final note for a future discussion: we would also be very happy if you had an official GitHub account and could accept pull requests, as we already do internally. That would also address this problem cleanly in my opinion! Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Pushing to spacewalk.git
On 12/10/2013 03:00 PM, Michael Mraka wrote: I was complaining about those one-commit sub-branches which immediatelly merge back to the master-bootstrap-css-fixes branch. Ok, no problem, we will avoid those from now on. Thanks, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] RFC: manage bare-metal systems from Spacewalk
, and feel free to bug me (moio in #spacewalk) if you run into any difficulty. Please also note that some design and implementation details were changed based on discussions on this mailing list when the feature was first presented[5]. Regards, Silvio PS. the SUSE Manager team would like to dedicate all efforts in this feature to the memory of Uwe Gansert, who began this as a hackweek project in 2012. We are proud of finally putting the finishing touches to his work. [1] http://tinyurl.com/qxetm2n [2] http://tinyurl.com/oz7bvk5 [3] http://tinyurl.com/px8rgzm [4] http://tinyurl.com/pnkrsm3 [5] http://thread.gmane.org/gmane.linux.redhat.spacewalk.devel/4073/focus=4090 -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] RFC: a Cobbler-based power management feature
Dear Spacewalk Team, I am pushing some code about a couple of features we developed in SUSE Manager[1] and wanted to share with the Spacewalk community, as usual I am glad to hear your comments and hope they will be accepted! First is an implementation of power management - powering on, off or rebooting one or multiple systems directly from Spacewalk. Implementation relies on existing functionality in Cobbler[2], which in current versions ultimately delegates power management protocol implementation to fence agents[3]. The proposed patches can use whatever protocol the installed Cobbler instance supports. Following branch has the single-system version: https://git.fedorahosted.org/cgit/spacewalk.git/log/?h=master-power-management-single Following branch builds on the previous one to add SSM support: https://git.fedorahosted.org/cgit/spacewalk.git/log/?h=master-power-management-ssm Minimal end-user documentation for SUSE Manager, which is mostly relevant for Spacewalk as well, can be found here: http://wiki.novell.com/index.php/SUSE_Manager/NewFeatures2.1#Power_Management Some implementation details you might be interested in: - since there is no way to ask Cobbler which protocols it supports, you should add a java.power_management.types = protocol1, protocol2 ... variable to your configuration files (see rhn_java.conf). Currently supported Cobbler protocols are named after fence agents[3]; - obviously, you also need to install matching fence-agent packages for the feature to work, and have access to hardware that supports those protocols. At the moment we tested IPMI only; - since Cobbler does not allow system records without a profile or an image, we added a little hack to allow power management for systems without a kickstart profile. Basically, when a user tries to do a power management operation on a system, its Cobbler record is checked - if it does not exist, a new one is created, pointing to a dummy image. Possible alternatives we evaluated and ultimately discarded: - restrict the use of this feature to systems with kickstart profiles; - have Cobbler handle system records without profiles nor images (requires patching Cobbler in non-trivial way); - not to use Cobbler at all, reimplementing power management in Spacewalk (having Cobbler and Cobbler integration already in place it didn't sound like a good idea architecturally). - the SSM part uses the existing System Set Manager - Status page to show results power management actions on multiple systems. To have some user-facing feedback in case of errors, we added a note column to rhnOperationServer and the corresponding JSP list - that could be used elsewhere, too. Second is some bits of code to allow bare-metal systems to show up in Spacewalk, a feature which was started long ago by Uwe Gansert. A first implementation was also already discussed on this list[4]. Power management is particularly useful when used together with this feature, but since it also makes sense on its own, I am pushing them separately to ease review. I will follow-up with an email on the bare-metal system management feature soon. Regards, Silvio [1] Yeah, we know this workflow is bad, features should really be implemented in Spacewalk first. Unfortunately development on these ones began long ago on the SUSE Manager codebase and it would have been hard to revert that choice halfway through. Please be patient this time! [2] https://github.com/cobbler/cobbler/wiki/Power-management [3] https://git.fedorahosted.org/cgit/fence-agents.git/tree/fence/agents [4] http://thread.gmane.org/gmane.linux.redhat.spacewalk.devel/4073/focus=4090 -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Using fonts in /rhn/account/LocalePreferences.do
On 11/28/2013 09:09 AM, Duncan Mac-Vicar P. wrote: I am pretty sure that one could generate a subset of the font but I haven't looked into that yet. It should be possible with the build system of those fonts. I am not an expert but it looks like the ttfsubset utility in FontUtils can do that: http://scripts.sil.org/cms/scripts/page.php?item_id=fontutils In SUSE operating systems it is part of the perl-Font-TTF package. HTH, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] Proposal to remove filesystem dependencies from ConfigTest
On 11/06/2013 04:23 PM, Tomas Lestach wrote: I am sorry, it took me so long to reply. No problem. Honestly, we do not really like the proposed patch, but on the other hand we didn't come with anything better. :-) Oh, I see :-) Anyway if you ever come up with a better idea I am always open to hear it and patch the code again! Have a nice day, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Patch proposal: try-wrapping XMLRPC serialization code
On 10/01/2013 10:58 PM, Grant Gainey wrote: I decided there *had to* be a better way to do this - and I found one! Great! More Eyes would be great. Actually, Taskomatic classes were left out (probably overlooked). I added them in commit c29e22. Apart from that, changes look good and also all tests pass tests here as well. Thanks a lot! -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] Proposal to remove filesystem dependencies from ConfigTest
Hi,
ConfigTest at the moment relies on some config files to be in a fixed
filesystem path (/usr/share/rhn/unit-tests/conf). This can be
inconvenient because, among other things, writing to /usr requires root
access.
The attached patch proposes a self-contained solution where those files
are taken from either the classes directory or the jar file, copied in
/tmp and read from there.
As there are other possible solutions I wanted to submit this for
review here before pushing it.
Thanks,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From f027ba5978cb78f786d344d686172fd0756c835b Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Mon, 30 Sep 2013 16:29:29 +0200
Subject: [PATCH] ConfigTest: do not rely on hardcoded paths, preexisting files
---
.../redhat/rhn/common/conf/test/ConfigTest.java| 25 +++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/common/conf/test/ConfigTest.java b/java/code/src/com/redhat/rhn/common/conf/test/ConfigTest.java
index 310964d..ab461a9 100644
--- a/java/code/src/com/redhat/rhn/common/conf/test/ConfigTest.java
+++ b/java/code/src/com/redhat/rhn/common/conf/test/ConfigTest.java
@@ -19,19 +19,38 @@ import com.redhat.rhn.common.conf.Config;
import com.redhat.rhn.testing.RhnBaseTestCase;
import com.redhat.rhn.testing.TestUtils;
+import java.io.File;
import java.util.Iterator;
import java.util.Properties;
+import org.apache.commons.io.FileUtils;
+
public class ConfigTest extends RhnBaseTestCase {
static final String TEST_KEY = user;
static final String TEST_VALUE = newval;
-static final String TEST_CONF_LOCATION = /usr/share/rhn/unit-tests/conf;
private Config c;
public void setUp() throws Exception {
c = new Config();
-c.addPath(TEST_CONF_LOCATION + /default);
-c.addPath(TEST_CONF_LOCATION);
+
+// create test config path
+String confPath = /tmp/ + TestUtils.randomString() + /conf;
+String defaultPath = confPath + /default;
+new File(defaultPath).mkdirs();
+
+// copy test configuration files over
+FileUtils.copyURLToFile(TestUtils.findTestData(conf/rhn.conf), new File(confPath,
+rhn.conf));
+FileUtils.copyURLToFile(TestUtils.findTestData(conf/default/rhn_web.conf),
+new File(defaultPath, rhn_web.conf));
+FileUtils.copyURLToFile(TestUtils.findTestData(conf/default/rhn_prefix.conf),
+new File(defaultPath, rhn_prefix.conf));
+FileUtils.copyURLToFile(TestUtils
+.findTestData(conf/default/bug154517.conf.rpmsave),
+new File(defaultPath, bug154517.conf.rpmsave));
+
+c.addPath(confPath);
+c.addPath(defaultPath);
c.parseFiles();
}
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] Add support for uploaded files in mocked requests, fix CryptoKeyCreateActionTest
Hi,
In our environment we noticed that CryptoKeyCreateActionTest was
failing. AFAIU, failure was due to the fact that the mocked request did
not have the uploaded file in it, that is, it was not a multipart request.
In order to fix that I added some code to support multipart requests in
RhnPostMockStrutsTestCase (patch 008). Code is a little convoluted
because the API required an extra class, yet I prefer
RhnPostMockStrutsTestCase to be self-contained - other solutions are
possible if you particularly dislike the proposed implementation. This
is of course reusable in other cases where mocking an uploaded file is
needed.
Patch 009 adds code to insert an (empty) uploaded key in
CryptoKeyCreateActionTest and this allowed the action to complete. I
also noticed that for whatever reason the crypto.key.nokey action
error is never issued, since CryptoKeyCreateAction extends
BaseCryptoKeyEditAction with isContentsRequired() always returning true,
so I removed that assertion as well.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From bc726176a9b91dbc51449673e6c845909bf999c2 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Tue, 10 Sep 2013 11:53:36 +0200
Subject: [PATCH 08/22] Add support for uploaded files in mocked requests
---
.../rhn/testing/RhnPostMockStrutsTestCase.java | 126 +
1 file changed, 126 insertions(+)
diff --git a/java/code/src/com/redhat/rhn/testing/RhnPostMockStrutsTestCase.java b/java/code/src/com/redhat/rhn/testing/RhnPostMockStrutsTestCase.java
index 62f9596..5cb22af 100644
--- a/java/code/src/com/redhat/rhn/testing/RhnPostMockStrutsTestCase.java
+++ b/java/code/src/com/redhat/rhn/testing/RhnPostMockStrutsTestCase.java
@@ -14,6 +14,18 @@
*/
package com.redhat.rhn.testing;
+import java.io.ByteArrayInputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Hashtable;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.apache.struts.Globals;
+import org.apache.struts.upload.CommonsMultipartRequestHandler;
+import org.apache.struts.upload.FormFile;
+
import servletunit.HttpServletRequestSimulator;
@@ -30,5 +42,119 @@ public class RhnPostMockStrutsTestCase extends RhnMockStrutsTestCase {
public void setUp() throws Exception {
super.setUp();
request.setMethod(HttpServletRequestSimulator.POST);
+UploadsHandler.clear();
+}
+
+/**
+ * Adds an uploaded text file to the form.
+ *
+ * @param parameterName upload form parameter name
+ * @param fileName the file name
+ * @param contents the file contents
+ */
+public void addUploadedFile(String parameterName, final String fileName,
+final String contents) {
+
+request.setContentType(multipart/form-data);
+request.setAttribute(Globals.MULTIPART_KEY, UploadsHandler.class.getName());
+
+final FormFile ff = new FormFile() {
+public void destroy() {
+}
+
+public String getContentType() {
+return text/plain;
+}
+
+public byte[] getFileData() throws FileNotFoundException, IOException {
+return contents.getBytes();
+}
+
+public String getFileName() {
+return fileName;
+}
+
+public int getFileSize() {
+return contents.length();
+}
+
+public InputStream getInputStream() throws FileNotFoundException, IOException {
+return new ByteArrayInputStream(contents.getBytes());
+}
+
+public void setContentType(String contentType) {
+}
+
+public void setFileName(String fileName) {
+}
+
+public void setFileSize(int fileSize) {
+}
+};
+
+UploadsHandler.addUpload(parameterName, ff);
+}
+
+/**
+ * An utility class to handle multipart requests.
+ */
+public static final class UploadsHandler extends CommonsMultipartRequestHandler {
+/** The request. */
+private HttpServletRequest request = null;
+
+/**
+ * Hashtable of uploaded file elements.
+ */
+private static HashtableString, FormFile uploadedFileElements =
+new HashtableString, FormFile();
+
+/**
+ * Adds an uploaded file.
+ * @param parameterName the parameter name
+ * @param file the file
+ */
+public static void addUpload(String parameterName, FormFile file) {
+uploadedFileElements.put(parameterName, file);
+}
+
+/**
+ * Clears uploaded files.
+ */
+public static void clear() {
+uploadedFileElements.clear();
+}
+
+
+/**
+ * Handles a request.
+ * @param requestIn the request
+ */
+@Override
[Spacewalk-devel] [PATCH] SystemEntitlementsSetupActionTest: do not assume Org has virtualization entitlement
SystemEntitlementsSetupActionTest was failing in our environment, as the
tested Org missed a virtualization entitlement.
The proposed attached patch fixes the problem.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From ae8640bdd4d15e16efdcb2fce7790fc676ab3f8a Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Tue, 10 Sep 2013 13:18:03 +0200
Subject: [PATCH 10/22] SystemEntitlementsSetupActionTest: do not assume Org
has virtualization entitlement
---
.../systems/entitlements/test/SystemEntitlementsSetupActionTest.java | 1 +
1 file changed, 1 insertion(+)
diff --git a/java/code/src/com/redhat/rhn/frontend/action/systems/entitlements/test/SystemEntitlementsSetupActionTest.java b/java/code/src/com/redhat/rhn/frontend/action/systems/entitlements/test/SystemEntitlementsSetupActionTest.java
index 8a8a7df..91b991e 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/systems/entitlements/test/SystemEntitlementsSetupActionTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/systems/entitlements/test/SystemEntitlementsSetupActionTest.java
@@ -52,6 +52,7 @@ public class SystemEntitlementsSetupActionTest extends RhnMockStrutsTestCase {
setRequestPathInfo(/systems/SystemEntitlements);
UserTestUtils.addManagement(user.getOrg());
UserTestUtils.addMonitoring(user.getOrg());
+UserTestUtils.addVirtualization(user.getOrg());
}
/**
*
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] MethodsSetupActionTest: do not assume that method command exists
See attached patch.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From d49e62923f34a64e5c2caac619d751d9e6010ac4 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Mon, 9 Sep 2013 10:56:06 +0200
Subject: [PATCH 05/22] MethodsSetupActionTest: do not assume that methods
exist
---
.../action/monitoring/notification/test/MethodsSetupActionTest.java | 5 +
1 file changed, 5 insertions(+)
diff --git a/java/code/src/com/redhat/rhn/frontend/action/monitoring/notification/test/MethodsSetupActionTest.java b/java/code/src/com/redhat/rhn/frontend/action/monitoring/notification/test/MethodsSetupActionTest.java
index bb06c9e..92bfc22 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/monitoring/notification/test/MethodsSetupActionTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/monitoring/notification/test/MethodsSetupActionTest.java
@@ -15,6 +15,7 @@
package com.redhat.rhn.frontend.action.monitoring.notification.test;
import com.redhat.rhn.common.db.datasource.DataResult;
+import com.redhat.rhn.domain.monitoring.notification.test.MethodTest;
import com.redhat.rhn.frontend.action.monitoring.notification.MethodsSetupAction;
import com.redhat.rhn.frontend.dto.monitoring.MethodDto;
import com.redhat.rhn.frontend.struts.RequestContext;
@@ -37,6 +38,10 @@ public class MethodsSetupActionTest extends RhnBaseTestCase {
sah.getRequest().setupAddParameter(newset, (String)null);
sah.getRequest().setupAddParameter(returnvisit, (String) null);
sah.getRequest().setupAddParameter(submitted, false);
+
+// ensure a Method exists
+MethodTest.createTestMethodCommand(sah.getUser());
+
sah.executeAction();
// Remove if not a List SetupAction
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] junit tests: do not rely on Cobbler
Hi,
We propose some patches to avoid the junit testsuite to depend on
Cobbler. With those, we are able to run all the tests using only a
database instance seeded with the latest schema version. Details:
- patch 17 calls setupTestConfiguration() on KickstartDataTest before
calling other members of the same class which, as far as I understand,
is the correct approach here;
- patch 18 does the same in SystemHandlerTest, which also used other
methods from KickstartDataTest;
- patch 19 adds code to mock the Cobbler connection in SystemManagerTest.
About patch 19: we noticed that most tests where Cobbler was required
were failing because they called SystemManager.deleteServer(). That
method used to remove a system from the database only, but later (in
2011) a patch was added to call Cobbler to delete the system from there
as well. Test code was never updated for Cobbler so it only checks
database and objects, so I think it makes sense to just use a mocked
connection to avoid the Cobbler exception on a test system that will
never be in Cobbler anyway.
Comments welcome!
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 0351959bb8c0398b1243fa045caa2eb580b8ad4a Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Wed, 11 Sep 2013 14:28:05 +0200
Subject: [PATCH 17/22] ActionManagerTest: do not rely on Cobbler, mock the
connection
---
java/code/src/com/redhat/rhn/manager/action/test/ActionManagerTest.java | 1 +
1 file changed, 1 insertion(+)
diff --git a/java/code/src/com/redhat/rhn/manager/action/test/ActionManagerTest.java b/java/code/src/com/redhat/rhn/manager/action/test/ActionManagerTest.java
index c80b5c3..e0094c3 100644
--- a/java/code/src/com/redhat/rhn/manager/action/test/ActionManagerTest.java
+++ b/java/code/src/com/redhat/rhn/manager/action/test/ActionManagerTest.java
@@ -332,6 +332,7 @@ public class ActionManagerTest extends RhnBaseTestCase {
.getServer();
ActionFactory.save(parentAction);
+KickstartDataTest.setupTestConfiguration(user);
KickstartData ksData = KickstartDataTest.createKickstartWithOptions(user.getOrg());
KickstartSession ksSession = KickstartSessionTest.createKickstartSession(server,
ksData, user, parentAction);
--
1.8.1.4
From df48d9e1b93a028c91ac4537ac26518857d630b7 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Wed, 11 Sep 2013 14:39:44 +0200
Subject: [PATCH 18/22] SystemHandlerTest: do not rely on Cobbler, use mock the
connection
---
.../com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java | 2 ++
1 file changed, 2 insertions(+)
diff --git a/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java b/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java
index 69d199a..2bdf677 100644
--- a/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java
@@ -718,6 +718,7 @@ public class SystemHandlerTest extends BaseHandlerTestCase {
Long sid = server.getId();
ClientCertificate cert = SystemManager.createClientCertificate(server);
cert.validate(server.getSecret());
+KickstartDataTest.setupTestConfiguration(admin);
assertEquals(1, handler.deleteSystem(cert.toString()));
assertNull(ServerFactory.lookupById(sid));
}
@@ -753,6 +754,7 @@ public class SystemHandlerTest extends BaseHandlerTestCase {
server.setBaseEntitlement(EntitlementManager.MANAGEMENT);
TestUtils.saveAndFlush(server);
server = (Server) reload(server);
+KickstartDataTest.setupTestConfiguration(admin);
KickstartData k = KickstartDataTest.createKickstartWithProfile(admin);
KickstartDataTest.addCommand(admin, k, url, --url http://cascade.sfbay.redhat.; +
com/rhn/kickstart/ks-rhel-i386-server-5);
--
1.8.1.4
From a2d6170e1d9692b81a9eaba3fa6bbb5e46cb9021 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Wed, 11 Sep 2013 15:12:42 +0200
Subject: [PATCH 19/22] SystemManagerTest: do not rely on Cobbler, mock the
connection
---
.../redhat/rhn/manager/system/test/SystemManagerTest.java| 12
1 file changed, 12 insertions(+)
diff --git a/java/code/src/com/redhat/rhn/manager/system/test/SystemManagerTest.java b/java/code/src/com/redhat/rhn/manager/system/test/SystemManagerTest.java
index 956d77e..ea745ec 100644
--- a/java/code/src/com/redhat/rhn/manager/system/test/SystemManagerTest.java
+++ b/java/code/src/com/redhat/rhn/manager/system/test/SystemManagerTest.java
@@ -14,6 +14,7 @@
*/
package com.redhat.rhn.manager.system.test;
+import com.redhat.rhn.common.conf.Config;
import com.redhat.rhn.common.conf.ConfigDefaults;
import com.redhat.rhn.common.db.datasource.DataResult;
import com.redhat.rhn.common.db.datasource.ModeFactory;
@@ -66,6 +67,8
[Spacewalk-devel] [PATCH] junit tests: some logging cleanups
Hi,
As previously discussed the new logging feature requires calling
LoggingFactory.clearLogId() at the beginning of each transaction, and
there were some places in test code where that was missing.
I confirm that the latest patches from Grant actually covered most
cases, I could only find four more and I am attaching a patch to fix
them as well.
Note: at the moment we haven't looked into main application code yet.
This ends this patch batch, hopefully, as we were finally able to run
all the tests successfully, at least on Postgres :-)
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 38d72cb5bd2cc62534f5b5ad6b0048dcfb999c9a Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Mon, 9 Sep 2013 11:48:44 +0200
Subject: [PATCH 22/22] junit tests: some logging cleanups
---
.../src/com/redhat/rhn/common/messaging/test/MessageQueueTest.java | 2 ++
.../rhn/frontend/action/rhnpackage/profile/test/SyncActionsTest.java | 2 ++
java/code/src/com/redhat/rhn/frontend/xmlrpc/test/SatScrubberTest.java | 2 ++
java/code/src/com/redhat/rhn/testing/BaseTestCaseWithUser.java | 3 +++
4 files changed, 9 insertions(+)
diff --git a/java/code/src/com/redhat/rhn/common/messaging/test/MessageQueueTest.java b/java/code/src/com/redhat/rhn/common/messaging/test/MessageQueueTest.java
index b3ad233..2a1d4c8 100644
--- a/java/code/src/com/redhat/rhn/common/messaging/test/MessageQueueTest.java
+++ b/java/code/src/com/redhat/rhn/common/messaging/test/MessageQueueTest.java
@@ -26,6 +26,7 @@ import com.redhat.rhn.common.db.datasource.ModeFactory;
import com.redhat.rhn.common.db.datasource.WriteMode;
import com.redhat.rhn.common.hibernate.HibernateFactory;
import com.redhat.rhn.common.messaging.MessageQueue;
+import com.redhat.rhn.domain.common.LoggingFactory;
import com.redhat.rhn.domain.monitoring.ServerProbe;
import com.redhat.rhn.domain.monitoring.test.MonitoringFactoryTest;
import com.redhat.rhn.domain.org.OrgFactory;
@@ -60,6 +61,7 @@ public class MessageQueueTest extends RhnBaseTestCase {
// If at some point we created a user and committed the transaction, we need
// clean up our mess
if (committed) {
+ LoggingFactory.clearLogId();
OrgFactory.deleteOrg(user.getOrg().getId(), user);
commitAndCloseSession();
}
diff --git a/java/code/src/com/redhat/rhn/frontend/action/rhnpackage/profile/test/SyncActionsTest.java b/java/code/src/com/redhat/rhn/frontend/action/rhnpackage/profile/test/SyncActionsTest.java
index 2abae4e..3ac9c9d 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/rhnpackage/profile/test/SyncActionsTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/rhnpackage/profile/test/SyncActionsTest.java
@@ -19,6 +19,7 @@ import java.util.Set;
import com.redhat.rhn.domain.channel.Channel;
import com.redhat.rhn.domain.channel.ChannelFactory;
import com.redhat.rhn.domain.channel.test.ChannelFactoryTest;
+import com.redhat.rhn.domain.common.LoggingFactory;
import com.redhat.rhn.domain.org.OrgFactory;
import com.redhat.rhn.domain.rhnpackage.Package;
import com.redhat.rhn.domain.rhnpackage.test.PackageTest;
@@ -108,6 +109,7 @@ public class SyncActionsTest extends RhnMockStrutsTestCase {
protected void tearDown() throws Exception {
super.tearDown();
// We committed stuff - need to remove it all again
+LoggingFactory.clearLogId();
OrgFactory.deleteOrg(user.getOrg().getId(), user);
commitAndCloseSession();
}
diff --git a/java/code/src/com/redhat/rhn/frontend/xmlrpc/test/SatScrubberTest.java b/java/code/src/com/redhat/rhn/frontend/xmlrpc/test/SatScrubberTest.java
index 935b51c..51e697f 100644
--- a/java/code/src/com/redhat/rhn/frontend/xmlrpc/test/SatScrubberTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/xmlrpc/test/SatScrubberTest.java
@@ -18,6 +18,7 @@ import com.redhat.rhn.common.db.datasource.CallableMode;
import com.redhat.rhn.common.db.datasource.DataResult;
import com.redhat.rhn.common.db.datasource.ModeFactory;
import com.redhat.rhn.common.hibernate.HibernateFactory;
+import com.redhat.rhn.domain.common.LoggingFactory;
import com.redhat.rhn.domain.kickstart.KickstartData;
import com.redhat.rhn.domain.kickstart.KickstartFactory;
import com.redhat.rhn.domain.kickstart.KickstartableTree;
@@ -178,6 +179,7 @@ public class SatScrubberTest extends TestCase {
Long id = (Long) row.get(id);
log.debug(Deleting org: + id);
try {
+LoggingFactory.clearLogId();
OrgFactory.deleteOrg(new Long(id.longValue()), UserFactory
.findResponsibleUser(1L, RoleFactory.SAT_ADMIN));
}
diff --git a/java/code/src/com/redhat/rhn/testing/BaseTestCaseWithUser.java b/java/code/src/com/redhat/rhn/testing/BaseTestCaseWithUser.java
index 8c21c45..cf85817 100644
--- a/java/code/src/com/redhat/rhn/testing
[Spacewalk-devel] [PATCH] Frontend monitoring tests: ensure a Monitoring Scout exists
Hi,
In some frontend tests it is assumed that at least one Monitoring Scout
exists, and those tests will fail if there is none.
The proposed attached patch creates a test Monitoring Scout before any
operation that requires one, thus eliminating this precondition.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From f3c39d5f5adb90216f97988c29133e549df291d4 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Fri, 6 Sep 2013 15:10:02 +0200
Subject: [PATCH 02/22] Frontend monitoring tests: ensure a Monitoring Scout
exists
---
.../com/redhat/rhn/domain/monitoring/suite/test/ProbeSuiteTest.java| 2 ++
.../frontend/action/monitoring/notification/test/FilterActionTest.java | 1 +
.../action/monitoring/test/ProbeSuiteProbeCreateActionTest.java| 2 ++
.../action/monitoring/test/ProbeSuiteSystemsEditActionTest.java| 2 ++
.../frontend/action/systems/monitoring/test/ProbeCreateActionTest.java | 2 ++
.../frontend/action/systems/monitoring/test/ProbeCreateTestCase.java | 3 +++
.../frontend/action/systems/monitoring/test/ProbeDeleteActionTest.java | 2 ++
.../action/systems/monitoring/test/ProbeDetailsActionTest.java | 2 ++
.../frontend/action/systems/monitoring/test/ProbeEditActionTest.java | 2 ++
.../frontend/action/systems/monitoring/test/ProbeGraphActionTest.java | 1 +
10 files changed, 19 insertions(+)
diff --git a/java/code/src/com/redhat/rhn/domain/monitoring/suite/test/ProbeSuiteTest.java b/java/code/src/com/redhat/rhn/domain/monitoring/suite/test/ProbeSuiteTest.java
index eaa16b0..f9018ad 100644
--- a/java/code/src/com/redhat/rhn/domain/monitoring/suite/test/ProbeSuiteTest.java
+++ b/java/code/src/com/redhat/rhn/domain/monitoring/suite/test/ProbeSuiteTest.java
@@ -30,6 +30,7 @@ import com.redhat.rhn.domain.server.test.ServerFactoryTest;
import com.redhat.rhn.domain.user.User;
import com.redhat.rhn.testing.BaseTestCaseWithUser;
import com.redhat.rhn.testing.TestUtils;
+import com.redhat.rhn.testing.UserTestUtils;
import org.hibernate.HibernateException;
@@ -191,6 +192,7 @@ public class ProbeSuiteTest extends BaseTestCaseWithUser {
}
// Just grab the 1st one for this test.
+UserTestUtils.addMonitoringScoutOrg(user);
SatCluster sc = (SatCluster)
user.getOrg().getMonitoringScouts().iterator().next();
diff --git a/java/code/src/com/redhat/rhn/frontend/action/monitoring/notification/test/FilterActionTest.java b/java/code/src/com/redhat/rhn/frontend/action/monitoring/notification/test/FilterActionTest.java
index 39ab377..902dab3 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/monitoring/notification/test/FilterActionTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/monitoring/notification/test/FilterActionTest.java
@@ -87,6 +87,7 @@ public class FilterActionTest extends RhnBaseTestCase {
filter = FilterTest.createTestFilter(user,
filter + TestUtils.randomString());
// Create a test probe
+UserTestUtils.addMonitoringScoutOrg(user);
ServerProbe p = (ServerProbe) MonitoringFactoryTest.createTestProbe(user);
Server s = ServerFactoryTest.createTestServer(user, true);
SatCluster sc = (SatCluster) user.getOrg().getMonitoringScouts().iterator().next();
diff --git a/java/code/src/com/redhat/rhn/frontend/action/monitoring/test/ProbeSuiteProbeCreateActionTest.java b/java/code/src/com/redhat/rhn/frontend/action/monitoring/test/ProbeSuiteProbeCreateActionTest.java
index 1cc37ee..648fe51 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/monitoring/test/ProbeSuiteProbeCreateActionTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/monitoring/test/ProbeSuiteProbeCreateActionTest.java
@@ -27,6 +27,7 @@ import com.redhat.rhn.frontend.action.systems.monitoring.test.ProbeCreateTestCas
import com.redhat.rhn.frontend.struts.RequestContext;
import com.redhat.rhn.frontend.struts.RhnAction;
import com.redhat.rhn.testing.ForwardWrapper;
+import com.redhat.rhn.testing.UserTestUtils;
/**
* ProbeCreateActionTest
@@ -51,6 +52,7 @@ public class ProbeSuiteProbeCreateActionTest extends ProbeCreateTestCase {
public void testSubmitExecute() throws Exception {
+UserTestUtils.addMonitoringScoutOrg(user);
ServerProbe orig = (ServerProbe) MonitoringFactoryTest.createTestProbe(user);
modifyActionHelper(success);
diff --git a/java/code/src/com/redhat/rhn/frontend/action/monitoring/test/ProbeSuiteSystemsEditActionTest.java b/java/code/src/com/redhat/rhn/frontend/action/monitoring/test/ProbeSuiteSystemsEditActionTest.java
index 68acee4..69dfd2e 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/monitoring/test/ProbeSuiteSystemsEditActionTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/monitoring/test/ProbeSuiteSystemsEditActionTest.java
@@ -40,6 +40,7 @@ import com.redhat.rhn.manager.rhnset.RhnSetDecl;
import
[Spacewalk-devel] [PATCH] MasterHandlerTest: handle getDefaultMaster exceptions
Hi,
If I understand code correctly MasterHandlerTest is about testing that
some methods can or cannot be called by administrators and regular
users. IssMaster objects are created ad-hoc for testing, and one of them
is flagged and unflagged as default during the tests themselves. The
only problem is the very first call to getDefaultMaster(): since none of
the test IssMaster objects has been flagged as default yet, the call may
or may not throw exceptions depending on the previous state.
The attached patch simply ignores such exception for the very first call.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 463c0da101a290bbee3449d6c77bfce415cad696 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Mon, 9 Sep 2013 13:48:27 +0200
Subject: [PATCH 06/22] MasterHandlerTest: handle getDefaultMaster exceptions
correctly
---
.../rhn/frontend/xmlrpc/sync/master/test/MasterHandlerTest.java | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/java/code/src/com/redhat/rhn/frontend/xmlrpc/sync/master/test/MasterHandlerTest.java b/java/code/src/com/redhat/rhn/frontend/xmlrpc/sync/master/test/MasterHandlerTest.java
index 5bff49b..eb1035b 100644
--- a/java/code/src/com/redhat/rhn/frontend/xmlrpc/sync/master/test/MasterHandlerTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/xmlrpc/sync/master/test/MasterHandlerTest.java
@@ -148,7 +148,13 @@ public class MasterHandlerTest extends BaseHandlerTestCase {
// Make sure satellite-admin can
try {
-IssMaster defaultMaster = handler.getDefaultMaster(adminKey);
+IssMaster defaultMaster = null;
+try {
+defaultMaster = handler.getDefaultMaster(adminKey);
+}
+catch (LookupException e) {
+// master not found, leave null
+}
int rc = handler.makeDefault(adminKey, master1.getId().intValue());
assertEquals(1, rc);
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] TestUtils: use the same temporary filename for the same file
Hi,
The attached patch fixes a previous contribution of mine to
transparently get resources either from the filesystem or from a jar
file. When accessing a file from a jar that files gets decompressed in a
temporary location which name was generated at random for each call.
This however did not work with all the tests - NavTest.testCache(), for
example, expected a file name not to change between calls. The proposed
implementation will stick to the same temporary file name as long as the
requested file name (and path) stays the same, thereby fixing NavTest
when run from a jar.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From cc7e859551be0997f7d45ff6ad3257d91a15d4a6 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Wed, 11 Sep 2013 15:48:57 +0200
Subject: [PATCH 21/22] TestUtils: use the same temporary filename when the
same file is requested
Fixes NavTest.testCache() when running from a jar.
---
java/code/src/com/redhat/rhn/testing/TestUtils.java | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/testing/TestUtils.java b/java/code/src/com/redhat/rhn/testing/TestUtils.java
index 24a8250..0d83f40 100644
--- a/java/code/src/com/redhat/rhn/testing/TestUtils.java
+++ b/java/code/src/com/redhat/rhn/testing/TestUtils.java
@@ -68,6 +68,9 @@ import java.util.Map;
public class TestUtils {
+/** Prefix for temporary file names created by this class. */
+private static String filePrefix = TestUtils.randomString();
+
// static class
private TestUtils() { }
@@ -93,8 +96,7 @@ public class TestUtils {
URL ret = clazz.getResource(path);
if (ret.toString().contains(!)) { // file is from an archive
-String tempPath =
-/tmp/ + System.currentTimeMillis() + TestUtils.randomString();
+String tempPath = /tmp/ + filePrefix + ret.hashCode();
InputStream input = clazz.getResourceAsStream(path);
OutputStream output = new FileOutputStream(tempPath);
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] SystemHandlerTest: do not rely on hardcoded sequence ids
SystemHandlerTest was failing in our environment and, AFAIU, it expects
that the underlying database will always return 1 as the id of a
persisted test object. This is normally not the case, as the value is
taken from a sequence and there is no code to ensure it is reset before
the test, so I simply relaxed that condition accordingly.
See attached patch.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From ec1b4e81c7e82005293bb50623aa6bef8bdcc92c Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Wed, 11 Sep 2013 15:00:56 +0200
Subject: [PATCH 20/22] SystemHandlerTest: do not rely on hardcoded sequence
ids
---
.../com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java b/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java
index 2bdf677..e885711 100644
--- a/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java
@@ -1858,7 +1858,7 @@ public class SystemHandlerTest extends BaseHandlerTestCase {
Long returnInt = handler.scheduleReboot(adminKey,
new Integer(testServer.getId().intValue()), new Date());
-assertEquals(returnInt, new Integer(1));
+assertNotNull(returnInt);
dr = ActionManager.recentlyScheduledActions(admin, null, 30);
assertEquals(1, dr.size() - preScheduleSize);
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] DownloadActionTest: do not assume file to be downloaded exists
See attached patch.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 67eef48cbd7a10d9451501ade7d6df9c01a6f430 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Mon, 9 Sep 2013 08:25:23 +0200
Subject: [PATCH 04/22] DownloadActionTest: do not assume file to be downloaded
exists
---
.../redhat/rhn/frontend/action/common/test/DownloadActionTest.java| 4
1 file changed, 4 insertions(+)
diff --git a/java/code/src/com/redhat/rhn/frontend/action/common/test/DownloadActionTest.java b/java/code/src/com/redhat/rhn/frontend/action/common/test/DownloadActionTest.java
index 8961f24..cdc767d 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/common/test/DownloadActionTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/common/test/DownloadActionTest.java
@@ -28,6 +28,8 @@ import com.redhat.rhn.testing.TestUtils;
import java.io.File;
import java.util.Map;
+import org.apache.commons.io.FileUtils;
+
/**
* TinyUrlActionTest
* @version $Rev$
@@ -121,6 +123,8 @@ public class DownloadActionTest extends RhnMockStrutsTestCase {
fileName);
TestUtils.saveAndFlush(p);
+FileUtils.touch(new File(/tmp/Server/ + fileName));
+
KickstartSession ksession =
KickstartSessionTest.createKickstartSession(ksdata, user);
ksession.setKstree(tree);
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] AuthFilterTest: mocked request object updated
AuthFilterTest was failing in our setup, apparently because the mocked
request object was missing some methods that were added after the test
itself was written. I added those methods and now it passes.
See attached patch.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 50ee6b949de7574f92ed87862ad35f8d6b4c7be5 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Tue, 10 Sep 2013 16:53:47 +0200
Subject: [PATCH 12/22] AuthFilterTest: mocked request object updated
---
.../src/com/redhat/rhn/frontend/servlets/test/AuthFilterTest.java | 6 ++
1 file changed, 6 insertions(+)
diff --git a/java/code/src/com/redhat/rhn/frontend/servlets/test/AuthFilterTest.java b/java/code/src/com/redhat/rhn/frontend/servlets/test/AuthFilterTest.java
index 84e26e6..b33cfd0 100644
--- a/java/code/src/com/redhat/rhn/frontend/servlets/test/AuthFilterTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/servlets/test/AuthFilterTest.java
@@ -88,6 +88,12 @@ public class AuthFilterTest extends MockObjectTestCase {
mockRequest.stubs().method(getMethod)
.will(returnValue(new String(GET)));
+mockRequest.stubs().method(getAttribute).with(eq(session))
+.will(returnValue(null));
+mockRequest.stubs().method(setAttribute).withAnyArguments();
+mockRequest.stubs().method(getCookies)
+.will(returnValue(null));
+
filter.setAuthenticationService((AuthenticationService)mockAuthService.proxy());
}
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] OrgHandlerTest: don't depend on a channel family with free entitlement slots
OrgHandlerTest apparently required an official Red Hat channel family
with free slots:
/**
* Lookup an official Red Hat channel family with free slots.
* Fail the test if none can be found.
[...]
private ChannelFamily lookupRedHatChannelFamily() { [...] }
Actually, this is a precondition not necessarily true in any instance,
and IMO test code should take care of setting up and tearing down such a
channel since it is needed to run tests. The attached patch adds some
setup code to do that.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From df2f9da98e0922307336e2280bec830a027ecc7b Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Wed, 11 Sep 2013 08:58:46 +0200
Subject: [PATCH 14/22] OrgHandlerTest: don't depend on a channel family with
free slots
---
.../frontend/xmlrpc/org/test/OrgHandlerTest.java | 78 --
1 file changed, 28 insertions(+), 50 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/frontend/xmlrpc/org/test/OrgHandlerTest.java b/java/code/src/com/redhat/rhn/frontend/xmlrpc/org/test/OrgHandlerTest.java
index 3a06c07..ecbf176 100644
--- a/java/code/src/com/redhat/rhn/frontend/xmlrpc/org/test/OrgHandlerTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/xmlrpc/org/test/OrgHandlerTest.java
@@ -16,14 +16,13 @@ package com.redhat.rhn.frontend.xmlrpc.org.test;
import com.redhat.rhn.common.conf.ConfigDefaults;
import com.redhat.rhn.domain.channel.ChannelFamily;
-import com.redhat.rhn.domain.channel.ChannelFamilyFactory;
+import com.redhat.rhn.domain.channel.test.ChannelFamilyFactoryTest;
import com.redhat.rhn.domain.org.Org;
import com.redhat.rhn.domain.org.OrgFactory;
import com.redhat.rhn.domain.role.RoleFactory;
import com.redhat.rhn.domain.server.Server;
import com.redhat.rhn.domain.user.User;
import com.redhat.rhn.domain.user.UserFactory;
-import com.redhat.rhn.frontend.dto.ChannelOverview;
import com.redhat.rhn.frontend.dto.MultiOrgEntitlementsDto;
import com.redhat.rhn.frontend.dto.MultiOrgUserOverview;
import com.redhat.rhn.frontend.dto.OrgChannelFamily;
@@ -40,7 +39,6 @@ import com.redhat.rhn.frontend.xmlrpc.ValidationException;
import com.redhat.rhn.frontend.xmlrpc.org.OrgHandler;
import com.redhat.rhn.frontend.xmlrpc.test.BaseHandlerTestCase;
import com.redhat.rhn.frontend.xmlrpc.test.XmlRpcTestUtils;
-import com.redhat.rhn.manager.channel.ChannelManager;
import com.redhat.rhn.manager.entitlement.EntitlementManager;
import com.redhat.rhn.manager.org.OrgManager;
import com.redhat.rhn.testing.ServerTestUtils;
@@ -62,6 +60,7 @@ public class OrgHandlerTest extends BaseHandlerTestCase {
private static final String EMAIL = fakead...@example.com;
private static final String PREFIX = Mr.;
private String[] orgName = {Test Org 1, Test Org 2};
+private ChannelFamily channelFamily = null;
public void setUp() throws Exception {
@@ -71,6 +70,12 @@ public class OrgHandlerTest extends BaseHandlerTestCase {
orgName[i] = Test Org + TestUtils.randomString();
}
TestUtils.saveAndFlush(admin);
+
+channelFamily = ChannelFamilyFactoryTest.createTestChannelFamily(
+admin,
+ChannelFamilyFactoryTest.ENTITLEMENT_ALLOCATION,
+ChannelFamilyFactoryTest.FLEX_ALLOCATION,
+true);
}
public void testCreate() throws Exception {
@@ -225,38 +230,37 @@ public class OrgHandlerTest extends BaseHandlerTestCase {
}
Org testOrg = createOrg();
-ChannelFamily cf = lookupRedHatChannelFamily();
// test the entitlement api before the entitlement has been assigned to the org
ListOrgSoftwareEntitlementDto entitlementCounts = null;
-entitlementCounts = handler.listSoftwareEntitlements(adminKey, cf.getLabel(),
-Boolean.TRUE);
+entitlementCounts = handler.listSoftwareEntitlements(adminKey,
+channelFamily.getLabel(), Boolean.TRUE);
// since includeUnentitled=TRUE, we should find an entry for the org w/ 0 ents
-assertOrgSoftwareEntitlement(testOrg.getId(), cf.getLabel(),
+assertOrgSoftwareEntitlement(testOrg.getId(), channelFamily.getLabel(),
entitlementCounts, 0, true);
-entitlementCounts = handler.listSoftwareEntitlements(adminKey, cf.getLabel(),
-Boolean.FALSE);
+entitlementCounts = handler.listSoftwareEntitlements(adminKey,
+channelFamily.getLabel(), Boolean.FALSE);
// since includeUnentitled=FALSE, we shouldn't be able to locate the org
-assertOrgSoftwareEntitlement(testOrg.getId(), cf.getLabel(),
+assertOrgSoftwareEntitlement(testOrg.getId(), channelFamily.getLabel(),
entitlementCounts, 0, false);
// now give the org some entitlements
int result = handler.setSoftwareEntitlements(adminKey,
- testOrg.getId().intValue(), cf.getLabel
[Spacewalk-devel] [PATCH] ActivationKeyHandlerTest: expect correct exceptions
ActivationKeyHandlerTest expected InvalidEntitlementException where
actually FaultException gets thrown. I updated the testcase.
See attached patch.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 72b582c013959eb7efc45c57b05c76e7ff4bb654 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Tue, 10 Sep 2013 17:09:03 +0200
Subject: [PATCH 13/22] ActivationKeyHandlerTest: expect correct exceptions
---
.../frontend/xmlrpc/activationkey/test/ActivationKeyHandlerTest.java | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/frontend/xmlrpc/activationkey/test/ActivationKeyHandlerTest.java b/java/code/src/com/redhat/rhn/frontend/xmlrpc/activationkey/test/ActivationKeyHandlerTest.java
index 4c72908..d663666 100644
--- a/java/code/src/com/redhat/rhn/frontend/xmlrpc/activationkey/test/ActivationKeyHandlerTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/xmlrpc/activationkey/test/ActivationKeyHandlerTest.java
@@ -41,7 +41,6 @@ import com.redhat.rhn.domain.token.ActivationKey;
import com.redhat.rhn.domain.token.TokenPackage;
import com.redhat.rhn.domain.token.test.ActivationKeyTest;
import com.redhat.rhn.frontend.xmlrpc.InvalidChannelException;
-import com.redhat.rhn.frontend.xmlrpc.InvalidEntitlementException;
import com.redhat.rhn.frontend.xmlrpc.MissingEntitlementException;
import com.redhat.rhn.frontend.xmlrpc.activationkey.ActivationKeyHandler;
import com.redhat.rhn.frontend.xmlrpc.serializer.ActivationKeySerializer;
@@ -174,7 +173,7 @@ public class ActivationKeyHandlerTest extends BaseHandlerTestCase {
new Integer(0), badEntitlements, Boolean.FALSE);
fail();
}
-catch (InvalidEntitlementException iee) {
+catch (FaultException fe) {
// expected
}
}
@@ -187,7 +186,7 @@ public class ActivationKeyHandlerTest extends BaseHandlerTestCase {
new Integer(0), badEntitlements, Boolean.FALSE);
fail();
}
-catch (InvalidEntitlementException iee) {
+catch (FaultException fe) {
// expected
}
}
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] RequestContext.buildPageLink: force parameter ordering
Hi,
I noticed that buildPageLink() uses a HashMap to keep track of query
string parameters. While this is perfectly okay from a web application
point of view, it makes testing a little more difficult and in
particular, RequestContextTest could break because it implicitly relies
on key ordering.
In the attached patch I propose using a TreeMap instead, which produces
a reliable ordering.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 984f3b48a370276bec7d3eedf0f114ed6ce07465 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Fri, 6 Sep 2013 14:23:58 +0200
Subject: [PATCH 01/22] RequestContext.buildPageLink: force parameter ordering
to ease testing
---
java/code/src/com/redhat/rhn/frontend/struts/RequestContext.java | 8 +---
.../com/redhat/rhn/frontend/struts/test/RequestContextTest.java | 5 +
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/frontend/struts/RequestContext.java b/java/code/src/com/redhat/rhn/frontend/struts/RequestContext.java
index 52fc845..20dcd54 100644
--- a/java/code/src/com/redhat/rhn/frontend/struts/RequestContext.java
+++ b/java/code/src/com/redhat/rhn/frontend/struts/RequestContext.java
@@ -47,6 +47,8 @@ import org.apache.log4j.Logger;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
+import java.util.SortedMap;
+import java.util.TreeMap;
import javax.servlet.http.HttpServletRequest;
@@ -589,7 +591,7 @@ public class RequestContext {
// if we already have this param in the query string we have to
// reset it to the new value
if (index = 0) {
-Map parammap = new HashMap();
+SortedMapString, String parammap = new TreeMapString, String();
String[] params = StringUtils.split(request.getQueryString(),
'');
// Convert the parameters into a map so we can
@@ -601,9 +603,9 @@ public class RequestContext {
parammap.remove(name);
parammap.put(name, value);
page.append(?);
-Iterator i = parammap.keySet().iterator();
+IteratorString i = parammap.keySet().iterator();
while (i.hasNext()) {
-String key = (String)i.next();
+String key = i.next();
page.append(key + = + parammap.get(key));
if (i.hasNext()) {
page.append();
diff --git a/java/code/src/com/redhat/rhn/frontend/struts/test/RequestContextTest.java b/java/code/src/com/redhat/rhn/frontend/struts/test/RequestContextTest.java
index 1394204..b34e2c1 100644
--- a/java/code/src/com/redhat/rhn/frontend/struts/test/RequestContextTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/struts/test/RequestContextTest.java
@@ -132,11 +132,8 @@ public class RequestContextTest extends MockObjectTestCase {
request.setupQueryString(otherparam=foobarparam=beersomeparam=value);
url = requestContext.buildPageLink(someparam, z);
-// we really can't guarantee the order of a map, so let's hope this
-// works long term. Thankfully in most cases we don't care about
-// the parameters order.
assertEquals(http://localhost/rhn/somePage.do?; +
-otherparam=foobarparam=beersomeparam=z, url);
+barparam=beerotherparam=foosomeparam=z, url);
}
}
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] pltcl question
On 09/11/2013 02:28 PM, Jan Pazdziora wrote: Which for typical web application request handling is what you want, isn't it? The whole request processing is one transaction and releasing it gives you nice cleanup. It is, except for a very few cases of commit-in-the-middle. Does it happen in our code base? I confirm that in most cases, and in particular all cases we observed until now, this only happens in test code. They need to be fixed to properly initialize the auth info in the logging infrastructure. Indeed. I will send some patches to fix remaining cases I stumbled upon recently. In one case, for example, a ROLLBACK was issued after a test failed. This led to a subsequent code failing because the global Tcl variable would retain the log row id, but the corresponding row in the log table was gone! No to the first part (we could use some other way), I did not find a better way to the second part. Okay, I see the problem now. The fact that the global variables are not initialized means that the user of the database (the applications) broke the contract which states that they should provide the auth logging information before doing anything with the database session because *any* database table can have logging enabled and need that information. That's the critical piece of information I was missing, as the LoggingFactory.clearLogId() method name and documentation were not terribly useful in understanding why it needs to be called after any operation on a new connection, COMMIT or ROLLBACK. Maybe I simply missed some documentation somewhere, I apologize in that case :) Thanks for the detailed explanation, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] pltcl question
Make sure you check latest code - I submitted a number of changes in the last day or so to address *exactly* the need to reset for logging after a commit in the testcases. I'm sure there are still a few places, but it's better now... Thanks! I noticed your latest patches while developing actually. I am reviewing my own patches today, and will be back with news soon. Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] rhn.manager tests: do not assume an Org admin exists
On 09/09/2013 09:34 PM, Grant Gainey wrote: I modified the patch somewhat - [...] Thanks for your contribution! I agree with your changes and confirm it works perfectly throughout our test suite as well. Thanks a lot for all review and commit work so far, we will be back with some more patches as soon as we are done running tests from the frontend package. Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] UserManagerTest: don't rely on hardcoded default time zone
UserManagerTest relied on a default America/New_York time zone, I
removed the hardcoded string replacing it with
UserFactory.getDefaultTimeZone().
See attached patch.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 6b807ef7be31c1e4fc2a08a77cf8944a634234ee Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Thu, 5 Sep 2013 15:16:40 +0200
Subject: [PATCH] UserManagerTest: don't rely on hardcoded default time zone
---
java/code/src/com/redhat/rhn/manager/user/test/UserManagerTest.java | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/manager/user/test/UserManagerTest.java b/java/code/src/com/redhat/rhn/manager/user/test/UserManagerTest.java
index 6fccbca..6b35881 100644
--- a/java/code/src/com/redhat/rhn/manager/user/test/UserManagerTest.java
+++ b/java/code/src/com/redhat/rhn/manager/user/test/UserManagerTest.java
@@ -383,8 +383,8 @@ public class UserManagerTest extends RhnBaseTestCase {
public void testGetTimeZoneDefault() {
RhnTimeZone tz = UserManager.getDefaultTimeZone();
assertNotNull(tz);
-assertTrue(tz.getTimeZone().getRawOffset() ==
-UserFactory.getTimeZone(America/New_York).getTimeZone().getRawOffset());
+assertTrue(tz.getTimeZone().getRawOffset() == UserFactory.getDefaultTimeZone()
+.getTimeZone().getRawOffset());
}
public void testLookupTimeZoneAll() {
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] ConfigureSatelliteCommandTest: do not rely on HashMap key ordering
I noticed that ConfigureSatelliteCommandTest makes an assertion that
depends on a HashMap key ordering, it was changed it to a SortedMap to
guarantee repeatability.
See attached patch.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 005e216a8cba6ace617750dfb09dc170fbd60ac9 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Fri, 6 Sep 2013 08:23:06 +0200
Subject: [PATCH] ConfigureSatelliteCommandTest: do not rely on HashMap key
ordering
---
.../rhn/manager/satellite/test/ConfigureSatelliteCommandTest.java | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/manager/satellite/test/ConfigureSatelliteCommandTest.java b/java/code/src/com/redhat/rhn/manager/satellite/test/ConfigureSatelliteCommandTest.java
index 28904d0..95e5cb9 100644
--- a/java/code/src/com/redhat/rhn/manager/satellite/test/ConfigureSatelliteCommandTest.java
+++ b/java/code/src/com/redhat/rhn/manager/satellite/test/ConfigureSatelliteCommandTest.java
@@ -28,10 +28,10 @@ import com.redhat.rhn.manager.satellite.Executor;
import com.redhat.rhn.testing.BaseTestCaseWithUser;
import com.redhat.rhn.testing.TestUtils;
-import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.TreeMap;
/**
* ConfigureSatelliteCommandTest - test for ConfigureSatelliteCommand
@@ -73,7 +73,7 @@ public class ConfigureSatelliteCommandTest extends BaseTestCaseWithUser {
assertTrue(cmd.getKeysToBeUpdated().contains(TEST_CONFIG_STRING));
assertTrue(cmd.getKeysToBeUpdated().contains(TEST_CONFIG_NULL));
-Map optionMap = new HashMap();
+Map optionMap = new TreeMap();
Iterator i = cmd.getKeysToBeUpdated().iterator();
while (i.hasNext()) {
String key = (String) i.next();
@@ -82,7 +82,7 @@ public class ConfigureSatelliteCommandTest extends BaseTestCaseWithUser {
String[] cmdargs = cmd.getCommandArguments(Config.getDefaultConfigFilePath(),
optionMap);
-assertEquals(--option=test.null_config.config_sat_test=, cmdargs[5]);
+assertEquals(--option=test.null_config.config_sat_test=, cmdargs[4]);
assertEquals(9, cmdargs.length);
assertNull(cmd.storeConfiguration());
assertTrue(cmd.getKeysToBeUpdated().size() == 0);
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] SystemManagerTest: missing super.setUp() call added
See attached patch.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 8377a9ba5f349a661f49a7f0ad9c393f7745f0fc Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Thu, 5 Sep 2013 09:11:37 +0200
Subject: [PATCH] SystemManagerTest: missing super.setUp() call added
---
.../src/com/redhat/rhn/manager/ssm/test/SsmOperationManagerTest.java | 1 +
1 file changed, 1 insertion(+)
diff --git a/java/code/src/com/redhat/rhn/manager/ssm/test/SsmOperationManagerTest.java b/java/code/src/com/redhat/rhn/manager/ssm/test/SsmOperationManagerTest.java
index 014c696..f6509c1 100644
--- a/java/code/src/com/redhat/rhn/manager/ssm/test/SsmOperationManagerTest.java
+++ b/java/code/src/com/redhat/rhn/manager/ssm/test/SsmOperationManagerTest.java
@@ -42,6 +42,7 @@ public class SsmOperationManagerTest extends RhnBaseTestCase {
private String serverSetLabel;
protected void setUp() throws Exception {
+super.setUp();
ssmUser = UserTestUtils.findNewUser(ssmuser, ssmorg);
serverSetLabel = populateRhnSet();
}
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] MonitoringManagerTest: missing super.setUp() call added
See attached patch.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 5865f906d561acb613addd31fa7e6b9e4e05e8af Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Wed, 4 Sep 2013 11:58:10 +0200
Subject: [PATCH] MonitoringManagerTest: missing super.setUp() call added
---
.../com/redhat/rhn/manager/monitoring/test/MonitoringManagerTest.java| 1 +
1 file changed, 1 insertion(+)
diff --git a/java/code/src/com/redhat/rhn/manager/monitoring/test/MonitoringManagerTest.java b/java/code/src/com/redhat/rhn/manager/monitoring/test/MonitoringManagerTest.java
index ec22ce8..4a90e8f 100644
--- a/java/code/src/com/redhat/rhn/manager/monitoring/test/MonitoringManagerTest.java
+++ b/java/code/src/com/redhat/rhn/manager/monitoring/test/MonitoringManagerTest.java
@@ -75,6 +75,7 @@ public class MonitoringManagerTest extends RhnBaseTestCase {
* {@inheritDoc}
*/
public void setUp() throws Exception {
+super.setUp();
user = UserTestUtils.findNewUser(testUser,
testOrg + this.getClass().getSimpleName());
if (ConfigDefaults.get().isMonitoringBackend()) {
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] UpdateErrataCacheCommand: log an error when orgId is incorrect
See attached patch.
Previously a NPE would be generated, but since the stack trace is not
very useful as it can only go back until run(), it only polluted test
outputs. I think that logging the error makes more sense in this case.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 5a061de30a4c26acb841a65cfe8314f712b9f967 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Wed, 4 Sep 2013 13:34:31 +0200
Subject: [PATCH] UpdateErrataCacheCommand: log an error when orgId is
incorrect
---
.../redhat/rhn/manager/errata/cache/UpdateErrataCacheCommand.java| 5 +
1 file changed, 5 insertions(+)
diff --git a/java/code/src/com/redhat/rhn/manager/errata/cache/UpdateErrataCacheCommand.java b/java/code/src/com/redhat/rhn/manager/errata/cache/UpdateErrataCacheCommand.java
index 1697aad..0b2612b 100644
--- a/java/code/src/com/redhat/rhn/manager/errata/cache/UpdateErrataCacheCommand.java
+++ b/java/code/src/com/redhat/rhn/manager/errata/cache/UpdateErrataCacheCommand.java
@@ -68,6 +68,11 @@ public class UpdateErrataCacheCommand extends BaseTransactionCommand {
Org org = OrgFactory.lookupById(orgId);
+if (org == null) {
+log.error(Org with id + orgId + was not found);
+return;
+}
+
int count = ErrataCacheManager.countServersInQueue(org);
if (log.isDebugEnabled()) {
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] VirtualizationEntitlementsManagerTest fixes
I attached two patches: 0001 - avoid failure if virtualization channels/packages are not present; 0002 - use Session.disconnect() instead of session.clear() after an expected ROLLBACK to properly resume Hibernate work in the test method, as clear() was not enough, at least in our environment, to keep Postgres from failing on subsequent statements. Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany From f0b8fd871e09bf4300b2d2a7f5166965b77ea7cf Mon Sep 17 00:00:00 2001 From: Silvio Moioli smoi...@suse.de Date: Thu, 5 Sep 2013 11:58:35 +0200 Subject: [PATCH 2/2] VirtualizationEntitlementsManagerTest: do not fail after rollback Tests on Postgres failed because it did not accept statements after an expected ROLLBACK. Now connection is disposed properly. --- .../rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java b/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java index 6a02efe..031deed 100644 --- a/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java +++ b/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java @@ -162,7 +162,7 @@ public class VirtualizationEntitlementsManagerTest extends BaseTestCaseWithUser } assertEquals(1, VirtualizationEntitlementsManager.getInstance(). convertToFlex(sids, group.getId(), user).size()); -HibernateFactory.getSession().clear(); +HibernateFactory.getSession().disconnect(); l = VirtualizationEntitlementsManager.getInstance().listFlexGuests(user); assertTrue(!l.isEmpty()); assertEquals(1, l.size()); -- 1.8.1.4 From 7b7b2a28ed9bf4181e74d2768d981ce1a34f320d Mon Sep 17 00:00:00 2001 From: Silvio Moioli smoi...@suse.de Date: Thu, 5 Sep 2013 11:52:58 +0200 Subject: [PATCH 1/2] VirtualizationEntitlementsManagerTest: do not assume a virtualization channel exists --- .../rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java b/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java index f12bbde..6a02efe 100644 --- a/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java +++ b/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java @@ -40,6 +40,7 @@ import com.redhat.rhn.manager.system.ServerGroupManager; import com.redhat.rhn.manager.system.SystemManager; import com.redhat.rhn.manager.system.VirtualizationEntitlementsManager; import com.redhat.rhn.testing.BaseTestCaseWithUser; +import com.redhat.rhn.testing.ChannelTestUtils; import com.redhat.rhn.testing.UserTestUtils; import java.util.Collection; @@ -77,6 +78,7 @@ public class VirtualizationEntitlementsManagerTest extends BaseTestCaseWithUser Server host = s.getVirtualInstance().getHostSystem(); Long hostId = host.getId(); assertNotNull(host); +ChannelTestUtils.setupBaseChannelForVirtualization(user, host.getBaseChannel()); SystemManager.entitleServer(host, EntitlementManager.VIRTUALIZATION); l = VirtualizationEntitlementsManager.getInstance(). -- 1.8.1.4 ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] VirtualizationEntitlementsManagerTest fixes
On 09/09/2013 08:29 AM, Silvio Moioli wrote: I attached two patches: Patches updated to apply cleanly after Grant's commits. I apologize for the double post. Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany From 8b9f6060b5db94e49edb9fb804562a89bc188580 Mon Sep 17 00:00:00 2001 From: Silvio Moioli smoi...@suse.de Date: Mon, 9 Sep 2013 08:52:14 +0200 Subject: [PATCH 2/2] VirtualizationEntitlementsManagerTest: do not fail after rollback --- .../rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java b/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java index 7824512..6f65e85 100644 --- a/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java +++ b/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java @@ -161,7 +161,7 @@ public class VirtualizationEntitlementsManagerTest extends BaseTestCaseWithUser } assertEquals(1, VirtualizationEntitlementsManager.getInstance(). convertToFlex(sids, group.getId(), user).size()); -HibernateFactory.getSession().clear(); +HibernateFactory.getSession().disconnect(); l = VirtualizationEntitlementsManager.getInstance().listFlexGuests(user); assertTrue(!l.isEmpty()); assertEquals(1, l.size()); -- 1.8.1.4 From e25b01758da24d955b0c41e87a61d31f8448947c Mon Sep 17 00:00:00 2001 From: Silvio Moioli smoi...@suse.de Date: Thu, 5 Sep 2013 11:52:58 +0200 Subject: [PATCH 1/2] VirtualizationEntitlementsManagerTest: do not assume a virtualization channel exists --- .../rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java b/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java index d8f0845..7824512 100644 --- a/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java +++ b/java/code/src/com/redhat/rhn/manager/system/test/VirtualizationEntitlementsManagerTest.java @@ -40,6 +40,7 @@ import com.redhat.rhn.manager.system.ServerGroupManager; import com.redhat.rhn.manager.system.SystemManager; import com.redhat.rhn.manager.system.VirtualizationEntitlementsManager; import com.redhat.rhn.testing.BaseTestCaseWithUser; +import com.redhat.rhn.testing.ChannelTestUtils; import java.util.Collection; import java.util.LinkedList; @@ -76,6 +77,7 @@ public class VirtualizationEntitlementsManagerTest extends BaseTestCaseWithUser Server host = s.getVirtualInstance().getHostSystem(); Long hostId = host.getId(); assertNotNull(host); +ChannelTestUtils.setupBaseChannelForVirtualization(user, host.getBaseChannel()); SystemManager.entitleServer(host, EntitlementManager.VIRTUALIZATION); l = VirtualizationEntitlementsManager.getInstance(). -- 1.8.1.4 ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] UpdateErrataCacheCommand: log an error when orgId is incorrect
On 09/09/2013 10:03 AM, Tomas Lestach wrote: So, what is the motivation of this change? (The only scenario I can imagine is, that the errata cache event will be queued and its organization gets deleted before the background errata cache action gets executed.) I think that is exactly the case, as we see many such NPEs in the unit tests' standard output. AFAIU this happens since BaseTestCaseWithUser sets up and deletes a new Org for every testsuite, so any pending operation on the errata cache can fail depending on timing. Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] Avoid a possible concurrency issue on RhnSet update
On 09/05/2013 02:26 AM, Jan Pazdziora wrote: Please note that this will not work -- the fact that you check content of some table in your session and your transaction and based on that run the insert operation does not mean that the other session cannot do exactly the same, both will find the record not there, and you will get the ORA-1 anyway. Good point: actually I was thinking about that statement being the only one in its transaction, but that's actually not the case. I think I will do as Thomas suggested, handling exceptions better in order to be as safe as possible when ignoring them, yet accepting failures in this specific case. I will be back with a patch as soon as I have tested it. Thanks for the discussion by the way, as problems in this topic are traditionally difficult to handle! Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] Avoid a possible concurrency issue on RhnSet update
On 09/03/2013 02:33 PM, Tomas Lestach wrote: to be honest, I personally do not like seeing DB index names in the application code. I agree that it is not really an elegant solution, but I could not find another way of ignoring such specific exceptions. Ideas welcome :-) (Btw. does it work on PG, when the index name is stated uppercase?) Actually, we could not reproduce the problem in Postgres at all. If the proposed solution turns out to be acceptable, I will research that. Isn't it possible to use 'SELECT FOR UPDATE' in these cases? I do not really think so, because AFAIU there is actually no SELECT involved. Failing statements are INSERTs that add tuples like (user, label, system, id), all built from HTTP request parameters, so there is no need to fetch data from the database in order to build them. In order not to add duplicates those INSERTs are preceded by DELETEs rather than SELECTs, so I guess write locks are acquired anyway. No, if both transactions start at the same time and one of them commits, the other transaction does not see those changes. Maybe I am missing something here but this is not what I observe. On Postgres, you can easily prove this using pgAdmin III and two Query windows. In the first window input: BEGIN; INSERT INTO rhnset (user_id, label, element, element_two, element_three) VALUES ( (SELECT id FROM web_contact LIMIT 1), 'test', 1, 1, 1 ); F5 (this leaves the transaction pending uncommitted [1]) And in the second one: BEGIN; SELECT * FROM rhnset WHERE label = 'test'; F5 You should not see the above test row. Now switch back to the first window and input: COMMIT; F5 (this commits the pending transaction) Finally switch back to the second window and repeat the query: SELECT * FROM rhnset WHERE label = 'test'; F5 You should see the added row. Note that the second transaction did see the update while not being committed. In fact if you run COMMIT; Postgres will close it (running COMMIT a second time will emit a warning that no transactions were pending). As I wrote, this is the expected behavior since the default READ COMMITTED isolation level is used [2]: [...] two successive SELECT commands can see different data, even though they are within a single transaction, if other transactions commit changes during execution [...] The same proof can be done with Oracle SQL Developer with minor syntax fixes. Note that you will have to open two connections to the same database because, by default, all SQL Worksheets there will refer to the same transaction. On 09/03/2013 04:14 PM, Paul Robert Marino wrote: I'm a little rusty on my Oracle but in PostgreSQL I think this could be resolved using the Isolation level within the transaction That could be done, but since a transaction is opened and closed at the beginning and end of an HTTP request (at least most of the time), that would mean to either change isolation level for all requests or introducing a mechanism to flag certain actions/URLs to get a different isolation level. Also, code using any transaction at an isolation level above READ COMMITTED should be modified to handle transaction serialization errors and be prepared to retry failed transactions, which AFAIK is never done now. possibly snapshots within the transaction That would mean saving snapshot IDs between HTTP requests. Again, doable, but sounds quite difficult at least to me! Yet another possibility would be to INSERT only tuples not already present by set difference, something like this query: INSERT INTO rhnset (user_id, label, element, element_two, element_three) SELECT :user_id, :label, :element, :element_two, :element_three FROM dual WHERE (:user_id, :label, :element, :element_two, :element_three) NOT IN ( SELECT * FROM rhnset ) Is that better in your opinion? [1] http://www.postgresql.org/message-id/937d27e10810020633k164e7f4bsb5213d86d2bfb...@mail.gmail.com [2] http://www.postgresql.org/docs/9.2/static/transaction-iso.html#XACT-READ-COMMITTED Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] pltcl question
Hi, We recently stumbled in some failing tests downstream and we were wondering why it was chosen to use pltcl in the new logging schema (introduced a couple of months ago). Problems arise because: - pltcl global variables are used [1]; - Postgres allocates a new Tcl interpreter for each database connection [2]; - thus, each connection will have its independent set of Tcl global variables; - Hibernate will by default release a database connection after a transaction has ended [3]; - thus, any database operation occurring after, for example, a call to ConnectionManager.commitTransaction(), could get a different connection from the one used before the COMMIT; - possibly, this different connection could be completely new - in that case no global variables are set; - the above could also not happen if, in lucky cases, the same connection is reused but this is (at least) Hibernate, c3p0, context, JVM and load dependent. Actually, there is not a lot of places in the Spacewalk codebase where database operations are carried out after a COMMIT, nevertheless in those cases trouble can happen. For example we found out that DELETE queries on web_contact will fail on a new connection, because web_contact_log_trig will call logging.get_log_id() that will in turn call logging._get_log_id() which would raise an exception due to the global variable the_log_id not being initialized. In our case, this after-commit user deletion happens in BaseTestCaseWithUser.tearDown() - OrgFactory.deleteOrg() - rhn_org.delete_org() - rhn_org.delete_user() which is called because ChannelManagerTest.testListCompatibleBaseChannels() commits in the middle. Questions: - is it really necessary to track variables per _database connection_ in the logging package? Since connections should be handled in a completely transparent way by Hibernate sessions, I believe this should not happen; - is it really necessary to add the pltcl language and use globals? Wouldn't a simple database sequence or even plain table suffice? I see the following possible solutions: - adopting another way to keep the log counter, possibly at transaction or whole database level; - calling LoggingFactory.clearLogId() after COMMITs and ROLLBACKs, or anyway be sure that it is called before any operation that could involve logging; - patching pltcl stored procedures so that they never assume a global has been initialized. Comments welcome! [1] see, for example, logging._get_log_id(). [2] http://wiki.tcl.tk/22422#pagetocea0bdcfb, bottom line: But mind you: one interpreter per db connection [...] [3] http://docs.jboss.org/hibernate/orm/3.2/reference/en/html/transactions.html, 11.5. Connection Release Modes, auto (the default) [...] for JDBCTransactionFactory, this returns ConnectionReleaseMode.AFTER_TRANSACTION Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] Avoid a possible concurrency issue on RhnSet update
Hi,
The attached patch attempts to fix an issue we found while running some
Selenium-based automated UI tests. Basically multiple requests that
involve an RhnSet can overlap if the user is clicking very quickly and
that might result in an ISE because RHNSET.RHN_SET_USER_LABEL_ELEM_UNQ
is violated.
We verified that using a system's Errata page, specifically in code that
gets executed from ErrataList.do to ErrataConfirm.do. In particular, if
you check a box selecting an errata in ErrataList.do then an AJAX call
is fired to DWRItemSelector.select(), which among other things updates
the underlying rhnset by adding a row. Then, if you very quickly click
on Apply Errata, ListRhnSetHelper will also try to add the same row
because DWRItemSelector hasn't finished yet and, in case of particularly
bad luck, the RDBMS will complain that the unique constraint has been
violated.
AFAIU this is possible from an RDBMS point of view since, both in the
Oracle and in Postgres, the READ COMMITTED transaction isolation level
is used. This means that a transaction can see data changing if other
transactions COMMIT during its lifetime [2].
Reproducing this issue is not very easy, since it is RDBMS, network and
server-load dependent. We succeeded in reliably repeat it in SUSE
Manager on Oracle using Selenium IDE for Firefox[1] to automate the
clicks. Of course, this can also happen at more human time scales if,
for example, server is under heavy load.
The attached patch just ignores constraint violations in this specific
case, as I see no point in throwing an exception when the row to INSERT
is simply already there. AFAIU that does not have unwanted side-effects,
but review is nevertheless very welcome.
Regards,
[1] http://docs.seleniumhq.org/download/
[2] http://www.postgresql.org/docs/9.2/static/transaction-iso.html
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From ca96039fb3edc31f05427f0f70ce278174f905b1 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Mon, 26 Aug 2013 18:37:19 +0200
Subject: [PATCH] Avoid issues on concurrent updates to RhnSet
---
.../src/com/redhat/rhn/domain/rhnset/RhnSetFactory.java | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/java/code/src/com/redhat/rhn/domain/rhnset/RhnSetFactory.java b/java/code/src/com/redhat/rhn/domain/rhnset/RhnSetFactory.java
index 0364400..a6b68f5 100644
--- a/java/code/src/com/redhat/rhn/domain/rhnset/RhnSetFactory.java
+++ b/java/code/src/com/redhat/rhn/domain/rhnset/RhnSetFactory.java
@@ -14,6 +14,7 @@
*/
package com.redhat.rhn.domain.rhnset;
+import com.redhat.rhn.common.db.ConstraintViolationException;
import com.redhat.rhn.common.db.datasource.DataResult;
import com.redhat.rhn.common.db.datasource.ModeFactory;
import com.redhat.rhn.common.db.datasource.SelectMode;
@@ -142,7 +143,20 @@ public class RhnSetFactory extends HibernateFactory {
WriteMode insertEl1 = writeMode(add_to_set_el1);
for (Iterator i = added.iterator(); i.hasNext();) {
RhnSetElement current = (RhnSetElement) i.next();
-executeMode(current, insertEl3, insertEl2, insertEl1);
+try {
+executeMode(current, insertEl3, insertEl2, insertEl1);
+}
+catch (ConstraintViolationException e) {
+if (e.getConstraint().endsWith(RHN_SET_USER_LABEL_ELEM_UNQ)) {
+// a concurrent transaction has already inserted this row
+// and COMMITted. This is tolerable and can happen because
+// the default transaction isolation level is READ
+// COMMITTED, thus this exception can be safely ignored
+}
+else {
+throw e;
+}
+}
}
if (!added.isEmpty()) {
simpl.getCleanup().cleanup(simpl);
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] UserFactoryTest: avoid failure if there are no users
On 08/28/2013 07:42 PM, Grant Gainey wrote:
Fixing the tests to be more explicit and less reliant on typical setups is,
at least in my opinion, always a good thing.
I am glad to hear that, as at the moment that's exactly what I am doing!
I attach three patches that go just in that direction.
001 - TestUtils: don't assume that tests are run from a .class directory
(we run them from rhn.jar);
002 - ChannelFactoryTest: don't assume there are any existing channels;
003 - HostBuilder: use ServerFactoryTest methods to build virtualized
hosts/guests to drop assumptions of having virtualization channels with
appropriate packages available. Since this varies some methods semantics
a bit (produced servers now have a base channel and consume
entitlements), patch tests using that class accordingly
(ServerFactoryTest and VirtualizationEntitlementsManagerTest). Also, add
a couple of methods to UserTestUtils and ServerTestUtils to ease
implementation, where appropriate.
Comments are always welcome, I look forward in contributing similar
patches also in the future.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 59f9bf8f5cd2ab708b748eb8224fa6139bcfe9b4 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Wed, 28 Aug 2013 17:30:17 +0200
Subject: [PATCH 1/3] TestUtils.findTestData: do not fail if called from an
archive
---
java/code/src/com/redhat/rhn/testing/TestUtils.java | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/java/code/src/com/redhat/rhn/testing/TestUtils.java b/java/code/src/com/redhat/rhn/testing/TestUtils.java
index 5a8c35c..24a8250 100644
--- a/java/code/src/com/redhat/rhn/testing/TestUtils.java
+++ b/java/code/src/com/redhat/rhn/testing/TestUtils.java
@@ -32,6 +32,7 @@ import com.redhat.rhn.frontend.servlets.PxtSessionDelegate;
import com.redhat.rhn.frontend.servlets.PxtSessionDelegateFactory;
import com.redhat.rhn.frontend.struts.RequestContext;
+import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.RandomStringUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.log4j.Level;
@@ -49,6 +50,7 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
+import java.io.OutputStream;
import java.io.Serializable;
import java.lang.reflect.Field;
import java.net.URL;
@@ -78,8 +80,10 @@ public class TestUtils {
* @return URL a URL referencing the file
* @throws ClassNotFoundException if the calling class can not be found
* (i.e., should not happen)
+ * @throws IOException if the specified file in an archive (eg. jar) and
+ * it cannot be copied to a temporary location
*/
-public static URL findTestData(String path) throws ClassNotFoundException {
+public static URL findTestData(String path) throws ClassNotFoundException, IOException {
Throwable t = new Throwable();
StackTraceElement[] ste = t.getStackTrace();
@@ -87,6 +91,18 @@ public class TestUtils {
Class clazz = Class.forName(className);
URL ret = clazz.getResource(path);
+
+if (ret.toString().contains(!)) { // file is from an archive
+String tempPath =
+/tmp/ + System.currentTimeMillis() + TestUtils.randomString();
+InputStream input = clazz.getResourceAsStream(path);
+
+OutputStream output = new FileOutputStream(tempPath);
+IOUtils.copy(input, output);
+
+return new File(tempPath).toURI().toURL();
+}
+
return ret;
}
--
1.8.1.4
From d189a3ca1236b1848a7a5b0fa1c529b3b383f6a8 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Wed, 21 Aug 2013 16:39:30 +0200
Subject: [PATCH 2/3] ChannelFactoryTest: avoid failure if no channels exist
---
.../src/com/redhat/rhn/domain/channel/test/ChannelFactoryTest.java | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/java/code/src/com/redhat/rhn/domain/channel/test/ChannelFactoryTest.java b/java/code/src/com/redhat/rhn/domain/channel/test/ChannelFactoryTest.java
index f07ba39..6b44f43 100644
--- a/java/code/src/com/redhat/rhn/domain/channel/test/ChannelFactoryTest.java
+++ b/java/code/src/com/redhat/rhn/domain/channel/test/ChannelFactoryTest.java
@@ -335,7 +335,11 @@ public class ChannelFactoryTest extends RhnBaseTestCase {
return pn;
}
-public void testFindChannelArchesSyncdChannels() {
+public void testFindChannelArchesSyncdChannels() throws Exception {
+// ensure at least one channel is present
+User user = UserTestUtils.findNewUser(testuser, testorg);
+ChannelFactoryTest.createTestChannel(user);
+
ListString labels = ChannelFactory.findChannelArchLabelsSyncdChannels();
assertNotNull(labels);
assertNotEmpty(labels);
--
1.8.1.4
From 35888b13580a8045517f7370cefbb0433662892f Mon Sep 17 00:00:00 2001
[Spacewalk-devel] [PATCH] com.redhat.rhn.common.db.datasource tests: get database username from configuration file
Hi,
it seems that some test cases in the
com.redhat.rhn.common.db.datasource.test package have hardcoded database
user names, which does not really work with our setup.
I propose the attached patch to simply get the database name from
rhn.conf. I am not really sure I understand why the original test was
coded that way - maybe there is a reason but at the moment I do not see it.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From fe99b2dfbc388409ce251e5f4fa54b3f21388ac8 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Thu, 29 Aug 2013 10:09:44 +0200
Subject: [PATCH] common.db.datasource tests: get database username from
configuration file
---
.../com/redhat/rhn/common/db/datasource/test/AdvDataSourceTest.java | 4 ++--
.../src/com/redhat/rhn/common/db/datasource/test/DataListTest.java| 4 ++--
.../redhat/rhn/common/db/datasource/test/DataSourceParserTest.java| 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/common/db/datasource/test/AdvDataSourceTest.java b/java/code/src/com/redhat/rhn/common/db/datasource/test/AdvDataSourceTest.java
index 1a04cf7..749819e 100644
--- a/java/code/src/com/redhat/rhn/common/db/datasource/test/AdvDataSourceTest.java
+++ b/java/code/src/com/redhat/rhn/common/db/datasource/test/AdvDataSourceTest.java
@@ -15,6 +15,7 @@
package com.redhat.rhn.common.db.datasource.test;
import com.redhat.rhn.common.ObjectCreateWrapperException;
+import com.redhat.rhn.common.conf.Config;
import com.redhat.rhn.common.conf.ConfigDefaults;
import com.redhat.rhn.common.db.datasource.CallableMode;
import com.redhat.rhn.common.db.datasource.DataResult;
@@ -60,12 +61,11 @@ public class AdvDataSourceTest extends RhnBaseTestCase {
super(name);
if (ConfigDefaults.get().isOracle()) {
db_sufix = _or;
-db_user = SPACEUSER;
}
else {
db_sufix = _pg;
-db_user = spaceuser;
}
+db_user = Config.get().getString(ConfigDefaults.DB_USER);
}
private void lookup(String foobar, int id, int size) {
diff --git a/java/code/src/com/redhat/rhn/common/db/datasource/test/DataListTest.java b/java/code/src/com/redhat/rhn/common/db/datasource/test/DataListTest.java
index 4a135e6..c371b08 100644
--- a/java/code/src/com/redhat/rhn/common/db/datasource/test/DataListTest.java
+++ b/java/code/src/com/redhat/rhn/common/db/datasource/test/DataListTest.java
@@ -19,6 +19,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import com.redhat.rhn.common.conf.Config;
import com.redhat.rhn.common.conf.ConfigDefaults;
import com.redhat.rhn.common.db.datasource.DataList;
import com.redhat.rhn.common.db.datasource.DataResult;
@@ -37,12 +38,11 @@ public class DataListTest extends RhnBaseTestCase {
public void setUp() {
if (ConfigDefaults.get().isOracle()) {
db_sufix = _or;
-db_user = SPACEUSER;
}
else {
db_sufix = _pg;
-db_user = spaceuser;
}
+db_user = Config.get().getString(ConfigDefaults.DB_USER);
hsm = new HookedSelectMode(
ModeFactory.getMode(test_queries, user_tables + db_sufix));
diff --git a/java/code/src/com/redhat/rhn/common/db/datasource/test/DataSourceParserTest.java b/java/code/src/com/redhat/rhn/common/db/datasource/test/DataSourceParserTest.java
index 7df0890..c5814ec 100644
--- a/java/code/src/com/redhat/rhn/common/db/datasource/test/DataSourceParserTest.java
+++ b/java/code/src/com/redhat/rhn/common/db/datasource/test/DataSourceParserTest.java
@@ -14,6 +14,7 @@
*/
package com.redhat.rhn.common.db.datasource.test;
+import com.redhat.rhn.common.conf.Config;
import com.redhat.rhn.common.conf.ConfigDefaults;
import com.redhat.rhn.common.db.datasource.CachedStatement;
import com.redhat.rhn.common.db.datasource.DataResult;
@@ -46,12 +47,11 @@ public class DataSourceParserTest extends RhnBaseTestCase {
public DataSourceParserTest() {
if (ConfigDefaults.get().isOracle()) {
db_sufix = _or;
-db_user = SPACEUSER;
}
else {
db_sufix = _pg;
-db_user = spaceuser;
}
+db_user = Config.get().getString(ConfigDefaults.DB_USER);
}
public void testGetModes() throws Exception {
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] ChannelTest: do not assume a proxy channel family exists
Hi,
Here is another simple patch to avoid having a proxy channel family set
up in order to run ChannelTest.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 5e7a3ada13e76d49e1b2bb9539b1c8eb415dd9d0 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Thu, 29 Aug 2013 11:23:00 +0200
Subject: [PATCH] ChannelTest: do not assume a proxy channel family exists
---
.../rhn/domain/channel/test/ChannelFamilyFactoryTest.java | 14 +-
.../com/redhat/rhn/domain/channel/test/ChannelTest.java| 9 +
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/domain/channel/test/ChannelFamilyFactoryTest.java b/java/code/src/com/redhat/rhn/domain/channel/test/ChannelFamilyFactoryTest.java
index 2e67f7d..a7bb6b3 100644
--- a/java/code/src/com/redhat/rhn/domain/channel/test/ChannelFamilyFactoryTest.java
+++ b/java/code/src/com/redhat/rhn/domain/channel/test/ChannelFamilyFactoryTest.java
@@ -122,10 +122,15 @@ public class ChannelFamilyFactoryTest extends RhnBaseTestCase {
return createTestChannelFamily(user, ents, flexEnts, true);
}
-public static ChannelFamily createTestChannelFamily(User user,
-Long ents, Long flexEnts, boolean nullOrg) throws Exception {
-String label = ChannelFamilyLabel + TestUtils.randomString();
-String name = ChannelFamilyName + TestUtils.randomString();
+public static ChannelFamily createTestChannelFamily(User user, Long ents,
+Long flexEnts, boolean nullOrg) throws Exception {
+return createTestChannelFamily(user, ents, flexEnts, nullOrg, ChannelFamily);
+}
+
+public static ChannelFamily createTestChannelFamily(User user, Long ents,
+Long flexEnts, boolean nullOrg, String prefix) throws Exception {
+String label = prefix + Label + TestUtils.randomString();
+String name = prefix + Name + TestUtils.randomString();
String productUrl = http://www.example.com;;
ChannelFamily cfam = new ChannelFamily();
@@ -146,7 +151,6 @@ public class ChannelFamilyFactoryTest extends RhnBaseTestCase {
pcf.setMaxFlex(flexEnts);
HibernateFactory.getSession().save(pcf);
-
cfam.addPrivateChannelFamily(pcf);
cfam = (ChannelFamily) TestUtils.reload(cfam);
return cfam;
diff --git a/java/code/src/com/redhat/rhn/domain/channel/test/ChannelTest.java b/java/code/src/com/redhat/rhn/domain/channel/test/ChannelTest.java
index 3fbf7f8..f3d1797 100644
--- a/java/code/src/com/redhat/rhn/domain/channel/test/ChannelTest.java
+++ b/java/code/src/com/redhat/rhn/domain/channel/test/ChannelTest.java
@@ -109,10 +109,11 @@ public class ChannelTest extends BaseTestCaseWithUser {
public void testIsProxy() throws Exception {
Channel c = ChannelFactoryTest.createTestChannel(user);
-ChannelFamily cfam = ChannelFamilyFactory
- .lookupByLabel(ChannelFamilyFactory
-.PROXY_CHANNEL_FAMILY_LABEL,
-null);
+ChannelFamily cfam =
+ChannelFamilyFactoryTest.createTestChannelFamily(user,
+ChannelFamilyFactoryTest.ENTITLEMENT_ALLOCATION,
+ChannelFamilyFactoryTest.FLEX_ALLOCATION, false,
+ChannelFamilyFactory.PROXY_CHANNEL_FAMILY_LABEL);
c.setChannelFamily(cfam);
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] AdvDataSourceTest: do not rely on test execution order
Hi,
I noticed that AdvDataSourceTest relies on method ordering, and this can
sometimes break tests. In fact we noticed that method call ordering is
different in our OpenJDK 7 configuration between the commandline Ant
launcher and the Eclipse launcher, resulting in failed tests in the
former case but not in the latter.
More specifically, oneTimeSetup() inserts a row in adv_datasource and
testDelete() will fail if it does not find it. On the other hand,
testStressedElaboration() will fail if it finds it, because
row.getTestColumn() is null for that test line.
The proposed patch simply executes the whole setup and teardown for each
test, since that ensures tests do not depend on each other and does not
hurt performance significantly.
The patch has been tested both on Oracle and Postgres databases.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 895b40c753fe63ee00bf740b778f9ad83157e1e6 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Thu, 29 Aug 2013 14:31:44 +0200
Subject: [PATCH] AdvDataSourceTest: do not rely on test execution order
---
.../db/datasource/test/AdvDataSourceTest.java | 43 +++---
1 file changed, 6 insertions(+), 37 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/common/db/datasource/test/AdvDataSourceTest.java b/java/code/src/com/redhat/rhn/common/db/datasource/test/AdvDataSourceTest.java
index 915c0b5..adbd2ba 100644
--- a/java/code/src/com/redhat/rhn/common/db/datasource/test/AdvDataSourceTest.java
+++ b/java/code/src/com/redhat/rhn/common/db/datasource/test/AdvDataSourceTest.java
@@ -44,10 +44,6 @@ import java.util.Map;
import java.util.Random;
import java.util.Set;
-import junit.extensions.TestSetup;
-import junit.framework.Test;
-import junit.framework.TestSuite;
-
/*
* $Rev$
*/
@@ -185,7 +181,7 @@ public class AdvDataSourceTest extends RhnBaseTestCase {
public void testDelete() throws Exception {
// Take nothing for granted, make sure the data is there.
-lookup(Blarg, 1, 1);
+insert(Blarg, 1);
WriteMode m = ModeFactory.getWriteMode(test_queries, delete_from_table);
Map params = new HashMap();
params.put(foobar, Blarg);
@@ -336,34 +332,12 @@ public class AdvDataSourceTest extends RhnBaseTestCase {
System.out.println(dr);
}
-public static Test suite()
-throws Exception {
-TestSuite suite = new TestSuite(AdvDataSourceTest.class);
-TestSetup wrapper = new TestSetup(suite) {
-protected void setUp() throws Exception {
-oneTimeSetup();
-}
-
-protected void tearDown() throws Exception {
-oneTimeTeardown();
-}
-};
-return wrapper;
-}
-
-protected static void oneTimeSetup() throws Exception {
-Session session = null;
-Connection c = null;
-Statement stmt = null;
+protected void setUp() throws Exception {
+Session session = HibernateFactory.getSession();;
+Connection c = session.connection();
+Statement stmt = c.createStatement();
try {
-session = HibernateFactory.getSession();
-c = session.connection();
-stmt = c.createStatement();
-stmt.executeQuery(select 1 from adv_datasource);
-}
-catch (SQLException e) {
-// Couldn't select 1, so the table didn't exist, create it
if (ConfigDefaults.get().isOracle()) {
stmt.execute(create table adv_datasource +
( +
@@ -373,11 +347,8 @@ public class AdvDataSourceTest extends RhnBaseTestCase {
id number +
constraint adv_datasource_pk primary key +
));
-stmt.execute(insert into adv_datasource(foobar, id) +
-values ('Blarg', 1));
}
else {
-c.rollback();
stmt.execute(create table adv_datasource +
( +
foobar VarChar, +
@@ -386,8 +357,6 @@ public class AdvDataSourceTest extends RhnBaseTestCase {
id numeric +
constraint adv_datasource_pk primary key +
););
-stmt.execute(insert into adv_datasource(foobar, id) +
-values ('Blarg', 1););
}
c.commit();
}
@@ -396,7 +365,7 @@ public class AdvDataSourceTest extends RhnBaseTestCase {
}
}
-protected static void oneTimeTeardown() throws Exception {
+protected void tearDown() throws Exception {
Session session = null;
Connection c = null;
Statement stmt = null;
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel
[Spacewalk-devel] [PATCH] Patch synchronization issue
Hello, We recently stumbled upon an issue with patch synchronization, which turned out to be a bug in Hibernate handling of Package objects, which in turn was caused by a strange definition of hashCode() in that class. Steps to reproduce the problem and a proposed patch follow. Reproducing the problem: - clone a channel (eg. RHEL x86_64 Server 6); - update the original channel so that some of the patches are updated, and in particular, so that at least one patch gets some new packages added. Furthermore, at least one of those added packages should have a (name, arch, EVR) triple that is already present among the original patch's (name, arch, EVR) triples. An example is CL-SA-2013:0696, that adds multiple packages for xulrunner-devel-17.0.5-1.el6_4-x86_64 and xulrunner-devel-17.0.5-1.el6_4-i686; - pick such a patch and attempt to sync it (Channels - Manage Software Channels - your cloned channel name - Patches - Sync - select your patch - Sync Patches - Confirm); - those packages which were added to the patch with a (name, arch, EVR) triple already in the patch will NOT be added to the updated (synced) patch. Cause: Among other things, ErrataFactory.syncErrataDetails() will copy packages from the original patch to the cloned patch with the line Justin Sherrill 2008 a85006389d436719240b49f8eea37f7550edf974 -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] Patch synchronization issue
Hello,
We recently stumbled upon an issue with patch synchronization, which
turned out to be a bug in Hibernate handling of Package objects, which
in turn was caused by a strange definition of hashCode() in that class.
Steps to reproduce the problem and a proposed patch follow.
Reproducing the problem:
- clone a channel (eg. RHEL x86_64 Server 6);
- update the original channel so that some of the patches are updated,
and in particular, so that at least one patch gets some new packages
added. Furthermore, at least one of those added packages should have a
(name, arch, EVR) triple that is already present among the original
patch's (name, arch, EVR) triples. An example is CL-SA-2013:0696, that
adds multiple packages for xulrunner-devel-17.0.5-1.el6_4-x86_64 and
xulrunner-devel-17.0.5-1.el6_4-i686;
- pick such a patch and attempt to sync it (Channels - Manage
Software Channels - your cloned channel name - Patches - Sync
- select your patch - Sync Patches - Confirm);
- those packages which were added to the patch with a (name, arch, EVR)
triple already in the patch will NOT be added to the updated (synced) patch.
Cause:
Among other things, ErrataFactory.syncErrataDetails() will copy packages
from the original patch to the cloned patch with the line:
copy.setPackages(new HashSet(original.getPackages()));
When returning the Set from original.getPackages(), Hibernate will
delete Package objects with identical hash code, which is defined in
turn as a combination of the name, EVR and arch hash codes (see
Package.hashCode() and, similarly, Package.equals()). In this case,
packages do have all this properties set to identical values, but have
different ids and are different packages indeed.
Proposed solution:
Adding id as one of the fields considered by hashCode() and equals(), as
per the attached patch.
Apparently, this was already the case before a commit by Justin Sherrill
in 2008 (a85006389d436719240b49f8eea37f7550edf974) to fix PackageTest.
This seems not to be needed anymore, since I could succesfully run it
both with and without the attached patch. As far as I know this should
not cause any other regression.
Comments welcome!
PS: I apologize for my previous email, it was sent incomplete by mistake.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From edfe0cafeb5a530cd4617303b58e594ff554b17b Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Tue, 20 Aug 2013 17:25:00 +0200
Subject: [PATCH] Allow Hibernate to distinguish packages with identical name,
arch and evr
---
java/code/src/com/redhat/rhn/domain/rhnpackage/Package.java | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/domain/rhnpackage/Package.java b/java/code/src/com/redhat/rhn/domain/rhnpackage/Package.java
index 831b39f..0cbbb79 100644
--- a/java/code/src/com/redhat/rhn/domain/rhnpackage/Package.java
+++ b/java/code/src/com/redhat/rhn/domain/rhnpackage/Package.java
@@ -494,10 +494,10 @@ public class Package extends BaseDomainHelper {
public boolean equals(Object other) {
if (other instanceof Package) {
Package otherPack = (Package) other;
-return new EqualsBuilder().append(this.getPackageName(),
-otherPack.getPackageName()).append(this.getPackageArch(),
-otherPack.getPackageArch()).append(this.getPackageEvr(),
-this.getPackageEvr()).isEquals();
+return new EqualsBuilder().append(this.getId(), otherPack.getId())
+.append(this.getPackageName(), otherPack.getPackageName())
+.append(this.getPackageArch(), otherPack.getPackageArch())
+.append(this.getPackageEvr(), this.getPackageEvr()).isEquals();
}
return false;
}
@@ -508,8 +508,8 @@ public class Package extends BaseDomainHelper {
*/
@Override
public int hashCode() {
-return new HashCodeBuilder().append(this.getPackageName()).append(
-this.getPackageArch()).append(this.getPackageEvr()).toHashCode();
+return new HashCodeBuilder().append(this.getId()).append(this.getPackageName())
+.append(this.getPackageArch()).append(this.getPackageEvr()).toHashCode();
}
/**
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] Patch synchronization issue
On 08/21/2013 01:57 PM, Tomas Lestach wrote: Does this mean the reproducer is wrong? Shall I take two packages of same name, arch and evr? xulrunner-devel-17.0.5-1.el6_4-x86_64 and xulrunner-devel-17.0.5-1.el6_4-x86_64 (let's say signed with another gpg key)? Exactly - the problem arises when both a new package in an updated errata and a package already present in a less recent, cloned errata have identical (name, evr, arch). I apologize for my first explanation and examples not being clear enough. Actually it should not be possible to change the content of RHEL channels, so if you found a way how, please, let me know. In our case, the vendor simply changed a patch, adding packages. Custom channels/patches could also run in the same issue. Thanks, regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Patch: improve MethodUtil determinism
Hello Tomas,
As I wrote directly within the commit message displaying
/rhn/channels/ChannelPackages.do
page with 15 000 packages takes over 1.5 minutes with your patch in
comparison to 3-4secs
without the patch.
Thanks for the feedback. I also confirm the regression - obviously that
patch is unacceptable, I apologize.
Upon further investigation, I found out that:
- invokeStaticMethod isn't actually used anywhere, except tests. It can
removed, if there are no plans to use it (we haven't);
- callMethod is never called on classes that actually have more than
one qualifying (overloaded) method, except in its own test suite. In fact:
- most calls have an empty array as the method arguments, thus there
cannot be any overloaded methods;
- some other calls accept a bean as a parameter in order to call a
setter, and beans' setters cannot be overloaded;
- all BaseSetOperateOnSelectedItemsAction's subclasses only define
one method that gets called via operateOnSelectedSet;
- ActionHelper's executeAction is only used with Struts' standard
execute method on actions, in tests, and that is not overloaded;
- other than that all other calls are from MethodUtilTest.
So I would say it is safe to avoid the overloaded case in the test class
and properly document callMethod so that as long as it is used properly,
no performance-impacting patches are needed. I attached a patch that
implements this idea.
I hope you find my analysis correct and accept the proposed patch!
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 3b4f56a64f76c9650b1d05cb6709c2f272c4255d Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Fri, 16 Aug 2013 10:47:56 +0200
Subject: [PATCH] Avoid testing callMethod on multiple qualifying methods
---
java/code/src/com/redhat/rhn/common/util/MethodUtil.java | 10 +++---
.../src/com/redhat/rhn/common/util/test/MethodUtilTest.java| 4 ++--
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/common/util/MethodUtil.java b/java/code/src/com/redhat/rhn/common/util/MethodUtil.java
index 424af91..d432eaf 100644
--- a/java/code/src/com/redhat/rhn/common/util/MethodUtil.java
+++ b/java/code/src/com/redhat/rhn/common/util/MethodUtil.java
@@ -63,7 +63,9 @@ public class MethodUtil {
/**
- * Invoke a static method from a class.
+ * Invoke a static method from a class. Note that if more than one method
+ * qualifies for calling, eg. more than one method is found with compatible
+ * parameters, the actual invocation target is undefined.
* @param clazz The Class to search for the specified method
* @param method The method to execute.
* @param args the Arguments to the method.
@@ -93,8 +95,10 @@ public class MethodUtil {
}
/**
- * Call the specified method with the specified arguments, converting
- * the argument type if necessary.
+ * Call the specified method with the specified arguments, converting the
+ * argument type if necessary. Note that if more than one method qualifies
+ * for calling, eg. more than one method is found with compatible
+ * parameters, the actual invocation target is undefined.
* @param o The object from which to call the method
* @param methodCalled The method to call
* @param params a Collection of the parameters to methodCalled
diff --git a/java/code/src/com/redhat/rhn/common/util/test/MethodUtilTest.java b/java/code/src/com/redhat/rhn/common/util/test/MethodUtilTest.java
index 593311c..a4272a6 100644
--- a/java/code/src/com/redhat/rhn/common/util/test/MethodUtilTest.java
+++ b/java/code/src/com/redhat/rhn/common/util/test/MethodUtilTest.java
@@ -45,7 +45,7 @@ public class MethodUtilTest extends RhnBaseTestCase {
// This test assumes that we have a translation to go from String to
// boolean. The translator should convert 'Y' to true, and everything else
// to false.
-public String nonStaticMethod(boolean b) {
+public String nonStaticMethodWithTranslatedParameter(boolean b) {
return TEST_STRING + b;
}
@@ -98,7 +98,7 @@ public class MethodUtilTest extends RhnBaseTestCase {
}
public void testCallMethodWithTranslate() throws Exception {
-String teststr = (String)MethodUtil.callMethod(this, nonStaticMethod,
+String teststr = (String)MethodUtil.callMethod(this, nonStaticMethodWithTranslatedParameter,
new Object[] {Y});
assertEquals(TEST_STRING + true, teststr);
}
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] Patch: improve MethodUtil determinism
Hi,
Some downstream unit-testing highlighted a cause of non-determinism in
the MethodUtil class that made tests fail during some runs but not all
of them, without apparent changes in environment conditions.
An example is MethodUtilTest.testCallMethod(), which sometimes failed
with actual output Test true instead of the expected Test 1. The
root cause seems to be the implementation of MethodUtil.callMethod(),
which can in fact call any of the the following methods, since they are
all compatible:
public String nonStaticMethod(Integer number);
public String nonStaticMethod(boolean b);
MethodUtil will pick the first compatible method it finds, then since
getMethods() does not guarantee any ordering[1] behavior is
non-deterministic and the test fails (at least in some JVM/environment
combinations).
Please find the attached patch to force method sorting and thus let
callMethod() predictably run the same method when more than one is
compatible.
[1] getMethods' Javadoc explicitly warns against relying on ordering:
[...] The elements in the array returned are not sorted and are not in
any particular order. [...]
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 512281755d750b8188302917db905998e58e7b64 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Wed, 14 Aug 2013 14:49:26 +0200
Subject: [PATCH] Make callMethod() and invokeStaticMethod() deterministic when
multiple methods qualify for calling
---
.../src/com/redhat/rhn/common/util/MethodUtil.java | 23 --
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/common/util/MethodUtil.java b/java/code/src/com/redhat/rhn/common/util/MethodUtil.java
index 424af91..35486bc 100644
--- a/java/code/src/com/redhat/rhn/common/util/MethodUtil.java
+++ b/java/code/src/com/redhat/rhn/common/util/MethodUtil.java
@@ -26,6 +26,8 @@ import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
+import java.util.Arrays;
+import java.util.Comparator;
/**
* A simple class that assists with method invocation. We should just use
@@ -76,7 +78,7 @@ public class MethodUtil {
Object[] args)
throws NoSuchMethodException, IllegalAccessException,
InvocationTargetException {
-Method[] meths = clazz.getMethods();
+Method[] meths = getSortedMethods(clazz);
for (int i = 0; i meths.length; i++) {
if (!meths[i].getName().equals(method)) {
@@ -111,7 +113,7 @@ public class MethodUtil {
Class myClass = o.getClass();
Method[] methods;
try {
-methods = myClass.getMethods();
+methods = getSortedMethods(myClass);
}
catch (SecurityException e) {
// This should _never_ happen, because the Handler classes must
@@ -208,8 +210,25 @@ public class MethodUtil {
}
}
+/**
+ * Returned methods in a class, sorted by signature.
+ *
+ * @see java.lang.reflect.Method#toGenericString
+ * @param clazz the class
+ * @return clazz methods, sorted by signature
+ */
+public static Method[] getSortedMethods(Class? clazz) {
+Method[] methods = clazz.getMethods();
+Arrays.sort(methods, new ComparatorMethod() {
+@Override
+public int compare(Method m1, Method m2) {
+return m2.toGenericString().compareTo(m1.toGenericString());
+}
+});
+return methods;
+}
/**
* Get an instance of a class that can have its classname overridden in our config
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] Patch: adapt LocalizationServiceTest to run on all JDKs
Hi,
after bug 6609737[1] was fixed LocalizationServiceTest was updated to
expect MEZ instead of CET as timezone for the German-locale
timestamp format assertion. This is correct in those JDKs that contain a
fix, but the test actually fails when run on some older JDKs.
As we use IBM j9 downstream, we propose the attached patch to let the
test to pass in any case.
[1] http://bugs.sun.com/view_bug.do?bug_id=6609737
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From 77ebde681a357f44499d90e015c5cb94e2172a4d Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Wed, 14 Aug 2013 16:14:30 +0200
Subject: [PATCH] LocalizationServiceTest: run on JDKs without fix for bug
6609737
---
.../redhat/rhn/common/localization/test/LocalizationServiceTest.java | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/common/localization/test/LocalizationServiceTest.java b/java/code/src/com/redhat/rhn/common/localization/test/LocalizationServiceTest.java
index a4c2631..2c91228 100644
--- a/java/code/src/com/redhat/rhn/common/localization/test/LocalizationServiceTest.java
+++ b/java/code/src/com/redhat/rhn/common/localization/test/LocalizationServiceTest.java
@@ -206,8 +206,8 @@ public class LocalizationServiceTest extends RhnBaseTestCase {
// Now test formatting it to German format in a DE TimeZone.
ctx.setTimezone(TimeZone.getTimeZone(Europe/Paris));
String deDate = ls.formatDate(dt, Locale.GERMAN);
-expected = 10.12.04 22:20:00 MEZ;
-assertEquals(expected, deDate);
+expected = 10\\.12\\.04 22:20:00 (CET|MEZ);
+assertTrue(deDate.matches(expected));
String shortDeDate = ls.formatShortDate(dt, Locale.GERMAN);
expected = 10.12.04;
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] Sharing state between Java and Perl stacks
On 07/08/2013 10:38 AM, Tomas Lestach wrote: I migrated the page from Perl to Java. (Still need to remove the old perl page.) Yes, it's more work, but migrating perl pages to java is our long-term effort. We do not want to introduce any extra parameters, but rather migrate. OK, thanks very much, will keep that in mind in the future. Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] avoid relying on types returned by Hibernate
On 07/01/2013 05:14 PM, Tomas Lestach wrote: All right, so in case the bug does not evince in current Spacewalk, will it be visible when we upgrade to newer hibernate (which version) or any other component? As far as I know, patched code is fragile regardless of Hibernate version (we encountered the bug with 3.2.4.1). See, as an example, the following method: public static ListErrata publishToChannel(ListErrata errataList, Channel chan, User user, boolean inheritPackages) This will fail if chan is not a real instance of ClonedChannel but rather a proxy loaded from Hibernate. This is not easily reproducible, because Hibernate can return either a true ClonedChannel or a proxy depending on the type of lookup, state of the cache, object graph, etc. and indeed, in our case, channel was indirectly loaded from ChannelFactory.getAccessibleChildChannels(). Unfortunatelu, despite several efforts, I am still not able reproduce a bug with upstream Spacewalk code so far. Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] avoid relying on types returned by Hibernate
On 07/01/2013 01:12 PM, Tomas Lestach wrote: Could you point me to the appropriate bugzilla? I'd like to check the reproducer. Unfortunately our Bugzilla component is closed for outside view at the moment, and the bug only affected SUSE-specific code. If you need any further details, just let me know. Regards, -- Silvio Moioli SUSE LINUX Products GmbH Maxfeldstraße 5, 90409 Nürnberg Germany ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] avoid relying on types returned by Hibernate
Hi,
we had a downstream bug that boiled down to a failing Channel to
ClonedChannel cast. The cast was failing because Hibernate proxy objects
for subclasses are guaranteed to implement all properties and methods,
so that isClone() correctly returned true, but do not necessarily
respect the class hierarchy: in that case a CGLIB generated class object
was returned and it did not extend ClonedChannel.
This has been documented as a pitfall 2 in the following article:
http://java-success.blogspot.it/2012/09/understanding-hibernate-proxy-objects.html
While fixing the bug we found some other places in the code where the
same problem could happen, since the implementation assumes some object
can be cast to ClonedChannel. We then purpose this patch to avoid
relying on that mechanism, using a separate method that will work
regardless if the Channel is actually a Hibernate proxy or not.
Any comment is welcome!
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
From e5d4e4a2b5e19524ae3c6b8e8e16a9825d8d28e6 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Fri, 7 Jun 2013 16:02:00 +0200
Subject: [PATCH] Avoid relying on types returned by Hibernate
Conflicts:
java/code/src/com/redhat/rhn/domain/channel/Channel.java
---
java/code/src/com/redhat/rhn/domain/channel/Channel.java | 7 +++
java/code/src/com/redhat/rhn/domain/channel/ClonedChannel.java | 1 +
java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java | 2 +-
.../frontend/action/channel/manage/ChannelPackagesAddAction.java | 2 +-
java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java | 6 ++
java/code/src/com/redhat/rhn/manager/system/SystemManager.java | 3 +--
6 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/domain/channel/Channel.java b/java/code/src/com/redhat/rhn/domain/channel/Channel.java
index f091476..bcb06fa 100644
--- a/java/code/src/com/redhat/rhn/domain/channel/Channel.java
+++ b/java/code/src/com/redhat/rhn/domain/channel/Channel.java
@@ -836,4 +836,11 @@ public class Channel extends BaseDomainHelper implements Comparable {
}
return checksumType.getLabel();
}
+
+/**
+ * @return the original Channel the channel was cloned from
+ */
+public Channel getOriginal() {
+throw new UnsupportedOperationException();
+}
}
diff --git a/java/code/src/com/redhat/rhn/domain/channel/ClonedChannel.java b/java/code/src/com/redhat/rhn/domain/channel/ClonedChannel.java
index 22b7e85..c130dd9 100644
--- a/java/code/src/com/redhat/rhn/domain/channel/ClonedChannel.java
+++ b/java/code/src/com/redhat/rhn/domain/channel/ClonedChannel.java
@@ -27,6 +27,7 @@ public class ClonedChannel extends Channel {
/**
* @return the original Channel the channel was cloned from
*/
+@Override
public Channel getOriginal() {
return original;
}
diff --git a/java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java b/java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java
index d4ce27d..78db9fe 100644
--- a/java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java
+++ b/java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java
@@ -263,7 +263,7 @@ public class ErrataFactory extends HibernateFactory {
throw new InvalidChannelException(Cloned channel expected: +
chan.getLabel());
}
-Channel original = ((ClonedChannel) chan).getOriginal();
+Channel original = chan.getOriginal();
// see BZ 805714, if we are a clone of a clone the 1st clone
// may not have the errata we want
while (original.isCloned()
diff --git a/java/code/src/com/redhat/rhn/frontend/action/channel/manage/ChannelPackagesAddAction.java b/java/code/src/com/redhat/rhn/frontend/action/channel/manage/ChannelPackagesAddAction.java
index b074956..8dcbd5e 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/channel/manage/ChannelPackagesAddAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/channel/manage/ChannelPackagesAddAction.java
@@ -116,7 +116,7 @@ public class ChannelPackagesAddAction extends RhnAction {
//If a channel isn't selected, select one smartly
if (selectedChan == null) {
if (chan.isCloned()) {
-selectedChan = ((ClonedChannel) chan).getOriginal().getId().toString();
+selectedChan = chan.getOriginal().getId().toString();
}
else {
selectedChan = ORPHAN_PACKAGES;
diff --git a/java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java b/java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java
index 3d8fbef..5c255e7 100644
--- a/java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java
+++ b/java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java
@@ -2568,8
[Spacewalk-devel] Sharing state between Java and Perl stacks
Hello everyone,
I am trying to fix a minor bug in the Manage Software Channels section, and
I would like to know your opinion about the possible solutions.
Steps to reproduce the bug:
- click on the Channels tab,
- click on Manage Software Channels,
- click on any one of the available channels (the Details sub-tab gets
opened),
- click on the Managers sub-tab,
- click on delete software channel
Expected output: the Managers sub-tab stays selected
Actual output: the Details sub-tab gets selected
Basically the navigation bar will remember whatever sub-tab you had opened
before Managers and will select it after you click on delete software
channel. Note that this does not happen in any of the other sub-tabs
(Details, Patches, Packages and Repositories).
As far as I understand that happens because NavTreeIndex tries to determine the
current sub-tab in order to select it, and since none of the URLs match the
current one it will pick the most recently used:
// If we have a lastActive URL we
// will add it to the end of the list of URLs to
// use it as a last resort.
if (lastActive != null) {
…
}
This does not work for the Managers sub-tab because it is actually
implemented in Perl, thus it has no visibility over Java session state
variables and lastActive will remain untouched.
So:
- as far as I understand there is no standard mechanism to share session data
between the Perl and Java stacks;
- as far as I understand this is a general problem whenever Perl and Java
pages need to share temporary state information; anyway I am not aware of other
cases in which that is necessary;
- the specific problem could be worked around by adding an extra query string
parameter to the delete software channel link and having NavTreeIndex (or
some other controller class) interpret it. Similar solutions could be
implemented if analogous bugs are found;
- the specific problem could also be resolved by porting the page form Perl to
Java, even if it seems to be quite a lot of effort for this bug alone.
What are your thoughts about this? Do you think that adding a parameter would
be acceptable?
Thanks in advance,
Silvio
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] avoid link-jars task when executing make-eclipse-project
Hi,
I noticed that the make-eclipse-project Ant task in build.xml depends
on link-jars, which as far as I understand should not be needed. This
could result in errors when a developer wants to produce the Eclipse
project files even if he/she doesn't have all dependencies set up
correctly (eg. because they are available, but in other locations).
I attached a simple patch that removes the dependency.
Regards,
Silvio Moioli
From 541498264598781c2c8656bf03abe63a8bc04277 Mon Sep 17 00:00:00 2001
From: Silvio Moioli smoi...@suse.de
Date: Tue, 7 May 2013 09:50:52 +0200
Subject: [PATCH] Removed an unnecessary task dependency
Avoids an error in make-eclipse-project when the developer does not
have JPackage dependencies installed.
---
java/build.xml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/java/build.xml b/java/build.xml
index 1431394..948d243 100644
--- a/java/build.xml
+++ b/java/build.xml
@@ -62,8 +62,8 @@
/target
!-- Generates Eclipse classpath --
- target name=gen-eclipse depends=link-jars
- description = Generates eclipse .classpath file
+ target name=gen-eclipse depends=resolve
+ description = Generates eclipse .classpath file
exec executable=${basedir}/scripts/gen-eclipse.py
output=${basedir}/.classpath
arg value=${eclipse.lib.dirs}:${rhn-home}/buildconf/tempjars:${env.JAVA_HOME}/lib/tools.jar /
--
1.8.1.4
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
