Re: svn commit: r882210 [1/2] - in /ofbiz/trunk/applications/acco
Locked
Re: svn commit: r882210 [1/2] - in /ofbiz/trunk/applications/acco
 
|
Re: svn commit: r882210 [1/2] - in /ofbiz/trunk/applications/acco
|
|
On 20/11/2009, at 6:50 AM, Adam Heath wrote:
> [hidden email] wrote: >> Author: jleroux >> Date: Thu Nov 19 17:26:03 2009 >> New Revision: 882210 >> >> URL: http://svn.apache.org/viewvc?rev=882210&view=rev >> Log: >> A patch from Marc Morin "Resolve java warnings exposed in >> Eclipse : application - accounting" (https://issues.apache.org/jira/browse/OFBIZ-3157 >> ) - OFBIZ-3157¨ >> Patch for generic's warnings for accounting application >> >> I will soon commit InvoiceWorker.java (got a conflict) and some >> other changes I want to do, mostly enhanced for loops >> >> Modified: >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> GlEvents.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> invoice/InvoiceServices.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> payment/BillingAccountWorker.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> payment/GiftCertificateServices.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> payment/PaymentGatewayServices.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> payment/PaymentMethodServices.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> period/PeriodServices.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/tax/ >> TaxAuthorityServices.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> test/FinAccountTests.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> thirdparty/clearcommerce/CCServicesTest.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> thirdparty/gosoftware/PcChargeServices.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> thirdparty/gosoftware/RitaApi.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> thirdparty/gosoftware/RitaServices.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> thirdparty/valuelink/ValueLinkApi.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> thirdparty/valuelink/ValueLinkServices.java >> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> util/UtilAccounting.java >> >> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/ >> accounting/GlEvents.java >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/GlEvents.java?rev=882210&r1=882209&r2=882210&view=diff >> = >> = >> = >> = >> = >> = >> = >> = >> = >> ===================================================================== >> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> GlEvents.java (original) >> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> GlEvents.java Thu Nov 19 17:26:03 2009 >> @@ -19,7 +19,6 @@ >> package org.ofbiz.accounting; >> >> import java.math.BigDecimal; >> -import java.util.List; >> import java.util.Map; >> >> import javax.servlet.http.HttpServletRequest; >> >> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/ >> accounting/invoice/InvoiceServices.java >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/invoice/InvoiceServices.java?rev=882210&r1=882209&r2=882210&view=diff >> = >> = >> = >> = >> = >> = >> = >> = >> = >> ===================================================================== >> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> invoice/InvoiceServices.java (original) >> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >> invoice/InvoiceServices.java Thu Nov 19 17:26:03 2009 >> @@ -1158,7 +1158,7 @@ >> return serviceResult; >> } >> >> - public static Map<String, Object> >> createInvoicesFromShipments(DispatchContext dctx, Map context) { >> + public static Map<String, Object> >> createInvoicesFromShipments(DispatchContext dctx, Map<String, >> Object> context) { >> Delegator delegator = dctx.getDelegator(); >> LocalDispatcher dispatcher = dctx.getDispatcher(); >> List<String> shipmentIds = >> UtilGenerics.checkList(context.get("shipmentIds")); >> > > Nope, this is wrong. > > Map<String, ? extends Object> context. > > THIS IS A BUG. PLEASE REVERT UNTIL IT IS FIXED. > > The service engine *owns* the incoming context map. Called services > are not allowed to change it *at all*. > > There are already plenty of existing examples. > > When I did similiar things to framework, I discovered several services > that modified the incoming context. I then had to change their code > to deal with that problem. This patch needs to have the same thing > done to it. Could you explain a bit more? When I look at the StandardJavaEngine class the serviceInvoker method is specifically passing a Map<String, Object> for the context. Thanks Scott |
smime.p7s (4K)
Re: svn commit: r882210 [1/2] - in /ofbiz/trunk/applications/acco
|
|
Re: svn commit: r882210 [1/2] - in /ofbiz/trunk/applications/acco
|
|
On 20/11/2009, at 9:16 AM, Adam Heath wrote:
>>>> = >>>> = >>>> = >>>> = >>>> = >>>> = >>>> = >>>> = >>>> = >>>> = >>>> = >>>> =================================================================== >>>> >>>> --- >>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >>>> invoice/InvoiceServices.java >>>> (original) >>>> +++ >>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >>>> invoice/InvoiceServices.java >>>> Thu Nov 19 17:26:03 2009 >>>> @@ -1158,7 +1158,7 @@ >>>> return serviceResult; >>>> } >>>> >>>> - public static Map<String, Object> >>>> createInvoicesFromShipments(DispatchContext dctx, Map context) { >>>> + public static Map<String, Object> >>>> createInvoicesFromShipments(DispatchContext dctx, Map<String, >>>> Object> >>>> context) { >>>> Delegator delegator = dctx.getDelegator(); >>>> LocalDispatcher dispatcher = dctx.getDispatcher(); >>>> List<String> shipmentIds = >>>> UtilGenerics.checkList(context.get("shipmentIds")); >>>> >>> >>> Nope, this is wrong. >>> >>> Map<String, ? extends Object> context. >>> >>> THIS IS A BUG. PLEASE REVERT UNTIL IT IS FIXED. >>> >>> The service engine *owns* the incoming context map. Called services >>> are not allowed to change it *at all*. >>> >>> There are already plenty of existing examples. >>> >>> When I did similiar things to framework, I discovered several >>> services >>> that modified the incoming context. I then had to change their code >>> to deal with that problem. This patch needs to have the same thing >>> done to it. >> >> Hi Adam, >> >> Could you explain a bit more? When I look at the StandardJavaEngine >> class the serviceInvoker method is specifically passing a Map<String, >> Object> for the context. > > serviceInvoker adds/removes things to this context, yes, so it has to > have that generics markup. However, the things it calls *must* keep > it read-only, so that when several services in a chain are > called(think secas, service groups), once service can't affect another > in the set. > using the same markup as the invoking method modifies the context though :-) I'll just flag this piece of generics as something I need to study up on. Regards Scott |
Re: svn commit: r882210 [1/2] - in /ofbiz/trunk/applications/acco
|
|
Re: svn commit: r882210 [1/2] - in /ofbiz/trunk/applications/acco
|
|
On 20/11/2009, at 9:36 AM, Adam Heath wrote:
> Scott Gray wrote: >> On 20/11/2009, at 9:16 AM, Adam Heath wrote: >> >>>>>> = >>>>>> = >>>>>> = >>>>>> = >>>>>> = >>>>>> = >>>>>> = >>>>>> = >>>>>> = >>>>>> = >>>>>> = >>>>>> = >>>>>> = >>>>>> ================================================================= >>>>>> >>>>>> --- >>>>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >>>>>> invoice/InvoiceServices.java >>>>>> >>>>>> (original) >>>>>> +++ >>>>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ >>>>>> invoice/InvoiceServices.java >>>>>> >>>>>> Thu Nov 19 17:26:03 2009 >>>>>> @@ -1158,7 +1158,7 @@ >>>>>> return serviceResult; >>>>>> } >>>>>> >>>>>> - public static Map<String, Object> >>>>>> createInvoicesFromShipments(DispatchContext dctx, Map context) { >>>>>> + public static Map<String, Object> >>>>>> createInvoicesFromShipments(DispatchContext dctx, Map<String, >>>>>> Object> >>>>>> context) { >>>>>> Delegator delegator = dctx.getDelegator(); >>>>>> LocalDispatcher dispatcher = dctx.getDispatcher(); >>>>>> List<String> shipmentIds = >>>>>> UtilGenerics.checkList(context.get("shipmentIds")); >>>>>> >>>>> >>>>> Nope, this is wrong. >>>>> >>>>> Map<String, ? extends Object> context. >>>>> >>>>> THIS IS A BUG. PLEASE REVERT UNTIL IT IS FIXED. >>>>> >>>>> The service engine *owns* the incoming context map. Called >>>>> services >>>>> are not allowed to change it *at all*. >>>>> >>>>> There are already plenty of existing examples. >>>>> >>>>> When I did similiar things to framework, I discovered several >>>>> services >>>>> that modified the incoming context. I then had to change their >>>>> code >>>>> to deal with that problem. This patch needs to have the same >>>>> thing >>>>> done to it. >>>> >>>> Hi Adam, >>>> >>>> Could you explain a bit more? When I look at the >>>> StandardJavaEngine >>>> class the serviceInvoker method is specifically passing a >>>> Map<String, >>>> Object> for the context. >>> >>> serviceInvoker adds/removes things to this context, yes, so it has >>> to >>> have that generics markup. However, the things it calls *must* keep >>> it read-only, so that when several services in a chain are >>> called(think secas, service groups), once service can't affect >>> another >>> in the set. >>> >> >> Thanks for the explanation, I won't pretend that I understand how >> using >> the same markup as the invoking method modifies the context >> though :-) >> I'll just flag this piece of generics as something I need to study >> up on. > > == > Map<String, Object> context: > > This means that the type of the values stored in the context need to > be of type Object. Since all objects are this way, anything can be > stored into the context. > > Map<String, ? extends Object> context: > > This means that the *concrete* type of the value is *some unknown* > class, that extends Object. Anyone can pull anything out of this > context, as it *will* extend Object. However, since the actual type > of the values is not known, no one can put anything in, as it might > cause a ClassCastException somewhere. > > == > > The former can be cast to the latter, but not vice versa. > > Plus, it's rather good form, to consider *all* incoming parameters on > *any* method(not just service implementations) to be read-only, and > only return results thru the method's return value. This is called > having no side-effects. > > Also look at this: > http://www.angelikalanger.com/GenericsFAQ/JavaGenericsFAQ.html allow the context to be modified whereas ? will block any attempts to use put. BTW, <String, ? extends Object> is the same as using <String, ?> isn't it? Thanks Scott |
Re: svn commit: r882210 [1/2] - in /ofbiz/trunk/applications/acco
|
|
Re: svn commit: r882210 [1/2] - in /ofbiz/trunk/applications/acco
|
|
On 20/11/2009, at 9:54 AM, Adam Heath wrote:
>>> >>> Also look at this: >>> http://www.angelikalanger.com/GenericsFAQ/JavaGenericsFAQ.html >> >> Ah okay I see, so using Object doesn't modify the context but it does >> allow the context to be modified whereas ? will block any attempts to >> use put. >> >> BTW, <String, ? extends Object> is the same as using <String, ?> >> isn't it? > > No. ? means you have no idea what kind of value is in the map. None > at all. All you can do with such a map is take it's size, work with > the keyset, or remove values. You can't store values, can't do > anything with values *returned* from methods. > > I't s a pure side-effect that all classes in java extend Object, but > the generics markup above doesn't consider that. > reference. Regards Scott |
Re: svn commit: r882210 [1/2] - in /ofbiz/trunk/applications/acco
Administrator
|
| Free forum by Nabble | Edit this page |