dd4t / dd4t.core Goto Github PK

Class should contain the defaults that will be set by on of the DD4T.DI.*

Example:

var mappings = new Dictionary<Type, Type>();
mappings.Add(typeof(IMessageProvider), typeof(JMSMessageProvider));

return mappings;

The ViewModelFactory.BuildViewModel(IModel) method throws a NRE if called for a page and one of the component presentations on the page has a CT without any metadata.

Stack trace:

[NullReferenceException: Object reference not set to an instance of an object.]
DD4T.ViewModels.ViewModelFactory.ProcessViewModel(IViewModel viewModel, Type type, IContextModel contextModel) in C:\projects\dd4t-core\source\DD4T.ViewModels\ViewModelFactory.cs:237
DD4T.ViewModels.ViewModelFactory.BuildViewModel(Type type, IModel modelData, IContextModel contextModel) in C:\projects\dd4t-core\source\DD4T.ViewModels\ViewModelFactory.cs:175
DD4T.ViewModels.ViewModelFactory.BuildViewModelByAttribute(IModel modelData, IContextModel contextModel) in C:\projects\dd4t-core\source\DD4T.ViewModels\ViewModelFactory.cs:163

The DD4TConfiguration class is not entirely thread safe. This leads to errors like this:

10:47:35.24 Error Global Error Handling (http://backstage.flydubai.com/en/ by 172.31.9.79)
... System.ArgumentException (mscorlib)
... An item with the same key has already been added.
... at System.Collections.Generic.Dictionary2.Insert(TKey key, TValue value, Boolean add) ... at DD4T.Utils.DD4TConfiguration.GetExpirationForCacheRegion(String region) ... at DD4T.Utils.Caching.DefaultCacheAgent.FindCacheItemPolicy(String key, Object item, String region) ... at DD4T.Utils.Caching.DefaultCacheAgent.Store(String key, String region, Object item, List1 dependOnTcmUris)
... at DD4T.Factories.PageFactory.TryFindPage(String url, IPage& page)
... at DD4T.Mvc.Controllers.DD4TControllerBase.GetPage(String url)
... at Indivirtual.Flydubai.Web.Controllers.PageController.PageModel(String url)

Solution: add locking and a double-if construction to make the code thread-safe.

If a rich text field contains a link to a binary (e.g. a PDF) the default rich text resolver does not handle it correctly.
The template writes out 2 attributes:

• xhtml:href containing the URL of the published binary
• xlink:href containing the URI of the multimedia component

This is on purpose: you could write your own rich text resolver which handles these links differently. However, the default resolver should simply remove the xlink:href and keep the xhtml:href (which becomes a normal hyperlink).
Right now, it tries to resolve the xlink:href (the URI), and if that fails, removes all attributes including the xhtml:href (which should be kept!).

public IComponent GetIComponentObject(string componentStringContent)
{
throw new NotImplementedException();
}

public override DateTime GetLastPublishedDateCallBack(string key, object cachedItem)
{
throw new NotImplementedException();
}

ViewModels are pretty complex, we really need proper unit testing to be confident about releasing them!

We should have attributes PageId, ComponentId, KeywordId in case the implementer needs to access the TCM uri of the objects.

For example:

public class ComponentIdAttribute : ComponentAttributeBase
{
   public override IEnumerable GetPropertyValues(IComponent component, IModelProperty property, 
                                                 ITemplate template, IViewModelFactory factory)
   {
       return component == null ? null : new string[] { component.Id };
   }
   public override Type ExpectedReturnType
   {
       get { return typeof(String); }
   }
}

Currently, if the property BinaryProvider.LoadBinariesAsStream is set to true (it defaults to false), the provider connects to the broker database directly. This goes against one of our principle to always use the Tridion API.
I suggest to mark this property as obsolete in 2.0 so we can remove it some time later. Is anyone using this feature (to load binaries as a stream rather than an array of bytes) at all?

Improve the way we work with Tridion providers + MVC DLLs.

Various improvements to enable continuous integrations.

If you don't call LoadViewModels on the ViewModelFactory, the default should be to load them from the assembly which calls the ViewModelFactory. This is normally the web application.

The linkfactory must be enabled to resolve page links, by implementing the following method:
public string ResolvePageLink(string tcmUri)

Note that the results must be cached just like component links.

Requests that result in a 404 are not cached in the page factory. This can lead to excess load on the broker DB and/or the REST service.
The null results must be cached, similarly to the links.

In DXA, it is possible to bind a view model class to multiple schemas (= multiple root element names). This makes it possible to define common models that map to multiple schemas, for example a 'Article' model which can be mapped to News Articles, Press Releases, General Articles, etc. This common model will only contain fields which are shared by all these schemas.
Should be added after 2.0beta.

We need to refactor the caching logic thoroughly in order to support cache invalidation for binaries.

Debugging a dd4t application can be made easier with a Symbols server. Investigate what it takes to host such a Symbols server or that we create pdb files on release.

DD4T.Utils\Caching\JMSListener.cs is an old class (which is excluded from the project but still present in Github). Please remove.
Low prio..

As a starting point, we might consider annotating:
https://github.com/dd4t/DD4T.Core/blob/develop/source/DD4T.Utils/ConfigurationKeys.cs

It would also be helpful to have this in a wiki page that's easily findable from how-to articles etc.

If time permits (otherwise we can use the Java version)

The KeywordFieldAttribute should return the raw values (a list of IKeywords) if the property type is set to IKeyword (or an implementation thereof).

• Support multiple DI-libs
• Change factories / providers / agents to support constructor injection

There should be two cache agents:

• a 'piggy-back' agent, which checks the Tridion broker to find the last published date. It should do so EVERY TIME, and the monitors (like the current DefaultCacheAgent uses) must GO!!!
• an agent which is invalidated through JMS

Both agents should store in the same cache store. Preferably one which supports async invalidation (so when the item is to be invalidated, this is done in the background and the most recent item is returned until the new item is put in the cache store).

Currently a Tridion field which starts with an upper case letter is not automatically mapped to a property (even if the property ALSO starts with an upper case letter!).
It should work regardless of the case.

ComponentPresentationController and ComponentPresentationAction are marked as [Obsolete], But they shouldn't. this values are used by DD4T.Mvc.ViewModels.Attributes.RenderDataAttribute

[Obsolete] annotation should be removed!

To be discussed with Jason K.

Right now the view model attributes we use for rich text resolving and link resolving (RichTextField and ResolveUrlField) have a hard-coded dependency on the MVC helper classes which perform the actual resolving of links and rich text. This makes it hard to override the built-in logic.
There is a work around: you can create your own attributes (MyRichTextField, MyResolveUrlField) but that forces you to duplicate lots of code.

I suggest the following approach instead:

• Add interfaces ILinkResolver and IRichTextResolver
• Add default implementations for each (IRichTextResolver must go in DD4T.Mvc, the other one in DD4T.Core)
• Add a property 'IViewModelFactory ViewModelFactory {get; set; }' to the interfaces IModelAttribute and IPropertyAttribute
• Add properties ILinkResolver and IRichTextResolver to the interface IViewModelFactory
• Add parameters for ILinkResolver and IRichTextResolver to the constructor of the ViewModelFactory
• Pass the ViewModelFactory to the IModelAttributes in the method ViewModelFactory.FindViewModelByAttribute
• Pass the ViewModelFactory to the IPropertyAttributes in the method ViewModelFactory.ProcessViewModel
• The attributes will now always have access to the ViewModelFactory
• The ResolveUrlAttribute uses ViewModelFactory.LinkResolver to resolve the links
• The RichTextFieldAttribute uses ViewModelFactory.RichTextResolver to resolve the links
• The DI containers must set defaults for the ILinkResolver and IRichTextResolver
• The default IRichTextResolver can only be set by finding the implementation dynamically, same as the Tridion providers (otherwise the DI projects would need a dependency on the Mvc project)
• Finally, the ResolveUrlAttribute can now be moved to the Core where it belongs

Remove the dependency on Log4Net nuget packages.

When a binary component is added as a component link to a RTF field in Tridion. The DefaultRichTextResolver used by the ViewModelAttribute does not resolve binary links.

Possible fix:

Inject BinaryFactory into DefaultRichTextResolver and resolve the binary link.

When compiling DD4T.Core I get the following warning.

Some NuGet packages were installed using a target framework different from the current target framework and may need to be reinstalled. Visit http://docs.nuget.org/docs/workflows/reinstalling-packages for more information. Packages affected: Newtonsoft.Json DD4T.ViewModels

In Field:
[XmlIgnore]
public List Keywords
{
get
{
return KeywordValues;
}
set
{
KeywordValues = value;
}
}

Careful: there may be more examples of this in the ContentModel.cs!

There is a line in the JMSMessageProvider that causes a log4net error because it contains unescaped curly brackets (which are then treated as placeholders by the logging service).

Run broker queries asynchronously to free up threads and enhance performance.

In the native Tridion pageproviders, it is possible to set the publication id to 0. This means: get the page by URL without looking at the publication id. This works fine in many implementations, especially if the publicationUrl property is unique per publication.

In the REST provider an error is thrown if the publication id is 0. Please make the behaviour consistent (by allowing 0 for the REST provider).

Currently, the view model class must be annotated with 'viewkeys'. The component templates in Tridion must have a metadata containing this viewkey. This creates interdependencies which are really not necessary.
In DXA, the viewmodels are tied to the views by simply looking at the @model directive in the views. This is done using reflection on the compiled Razor view classes. See DXA 1.1 source code.
This removes the need to configure anything on the view model classes OR the templates.
I think we should copy this approach soon after 2.0beta comes out.

Tridion supports multiple JMS strategies (like AsyncJMS11MDB). All these should be supported.

See https://github.com/TcmExtensions/TcmCDService for a working example (only it uses XML as data format and we will use JSON).

The DefaultPublicationResolver now only uses the Web.Config setting to resolve the publication Id, but for older version we can use the dynamic conf to resolve the publicationId and now with CIL we can use the topman services to resolve the mappings.

The getting started documentation does not currently cover creating schemas to allow for metadata on component templates and page templates in order to specify the view.

This information is available as part of the article at http://remcorakers.nl/dd4t-development-environment/, but it should be part of the DD4T documentation

Currently, all assemblies that are loaded in memory are searched looking for view models (i.e. classes implementing IViewModel). This is highly inefficient! For example, your app will go over all classes in assemblies like System, System.Web, DD4T., Tridion., etc..

We need to find a way to limit this to assemblies which are likely to contain the customer's view models. Possibilities are:

• Add 'ViewModelAssemblies' key to the configuration and have the ViewModelFactory search that
• Add a public property 'ViewModelAssemblies' to the ViewModelFactory interface so you can set it manually in the global.asax
• Hard-code the factory to NOT search in anything starting with System, DD4T or Tridion (on the plus side this requires no configuration at all, but it still leaves countless others like Newtonsoft, Autofac, .... )

The BinaryDistributionModule cannot deal with binary URLs that do not end with an extension. E.g.:

http://mysite/media/blablabla

This will create an endless series of nested 'BinaryData' folders (until the maximum length of a path on the file system is reached).

Although Tridion does not support binaries without extensions, it is conceivable that someone has created template code that supplies a custom file name. So we should try to fix this.

The caching logic is commented out in the LinkFactory. This must be put back in.

I got this comment from Rafeeq (a good MVC developer);

[17-1-2016 8:59:01] Rafeeq: Is it good too have [HandleError] attribute on framework level..
I believe it should be removed..
[17-1-2016 8:59:13] Rafeeq: https://github.com/dd4t/DD4T.MVC/blob/develop/source/DD4T.Mvc/Controllers/ModelControllerBase.cs#L65
https://github.com/dd4t/DD4T.MVC/blob/develop/source/DD4T.Mvc/Controllers/TridionControllerBase.cs#L27
[17-1-2016 8:59:41] Rafeeq: It should be handled at application level right??
[17-1-2016 8:59:50] Rafeeq: rather than framework level..

I think he has a point. Any objections if we remove the HandleError attributes?

Remove the dependency on Apache.NMS nuget packages.

Move it DD4T.Essentials Repo.