Skip to content

Commit

Permalink
Fixes after another review round.
Browse files Browse the repository at this point in the history
  • Loading branch information
jahav committed Jun 4, 2023
1 parent 54a44d9 commit 0d8fe87
Show file tree
Hide file tree
Showing 11 changed files with 81 additions and 55 deletions.
14 changes: 14 additions & 0 deletions ClosedXML/Excel/IXLWorkbook.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,33 @@ public interface IXLWorkbook : IXLProtectable<IXLWorkbookProtection, XLWorkbookP
Double ColumnWidth { get; set; }

IXLCustomProperties CustomProperties { get; }

Boolean DefaultRightToLeft { get; }

Boolean DefaultShowFormulas { get; }

Boolean DefaultShowGridLines { get; }

Boolean DefaultShowOutlineSymbols { get; }

Boolean DefaultShowRowColHeaders { get; }

Boolean DefaultShowRuler { get; }

Boolean DefaultShowWhiteSpace { get; }

Boolean DefaultShowZeros { get; }

IXLFileSharing FileSharing { get; }

Boolean ForceFullCalculation { get; set; }

Boolean FullCalculationOnLoad { get; set; }

Boolean FullPrecision { get; set; }

Boolean LockStructure { get; set; }

Boolean LockWindows { get; set; }

/// <summary>
Expand Down
3 changes: 2 additions & 1 deletion ClosedXML/Excel/PivotTables/IXLPivotCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public interface IXLPivotCache
/// Get names of all fields in the source, in left to right order. Every field name is unique.
/// </summary>
/// <remarks>
/// The field names are case sensitive. The field names of the cached
/// The field names are case insensitive. The field names of the cached
/// source might differ from actual names of the columns
/// in the data cells.
/// </remarks>
Expand All @@ -28,6 +28,7 @@ public interface IXLPivotCache
/// Shared items are distinct values of a source field values. Updating them can be expensive
/// and this controls, when should the cache be updated. Application-dependent attribute.
/// </remarks>
/// <value>Default value is <see cref="XLItemsToRetain.Automatic"/>.</value>
XLItemsToRetain ItemsToRetainPerField { get; set; }

/// <summary>
Expand Down
5 changes: 5 additions & 0 deletions ClosedXML/Excel/PivotTables/IXLPivotTables.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ public interface IXLPivotTables : IEnumerable<IXLPivotTable>

void DeleteAll();

/// <summary>
/// Get pivot table with the specified name (case insensitive).
/// </summary>
/// <param name="name">Name of a pivot table to return.</param>
/// <exception cref="KeyNotFoundException">No such pivot table found.</exception>
IXLPivotTable PivotTable(String name);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ namespace ClosedXML.Excel
{
/// <summary>
/// An interface for fluent configuration of how to show <see cref="IXLPivotValue"/>,
/// when the value should be displayed not a value itself, but in relation to another
/// when the value should be displayed not as a value itself, but in relation to another
/// value (e.g. percentage difference in relation to different value).
/// </summary>
public interface IXLPivotValueCombination
Expand All @@ -24,7 +24,7 @@ public interface IXLPivotValueCombination
/// Example:
/// We have a table of sales and a pivot table, where sales are summed per month.
/// The months are sorted from Jan to Dec. To display a percentage increase of
/// sales per quarter (the base value is previous month):
/// sales per month (the base value is previous month):
/// <c>
/// IXLPivotValue sales;
/// sales.SetSummaryFormula(XLPivotSummary.Sum).ShowAsPercentageDifferenceFrom("Month").AndPrevious();
Expand Down
13 changes: 8 additions & 5 deletions ClosedXML/Excel/PivotTables/XLPivotCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace ClosedXML.Excel
{
internal class XLPivotCache : IXLPivotCache
{
private readonly Dictionary<String, Int32> _fieldIndexes = new();
private readonly Dictionary<String, Int32> _fieldIndexes = new(StringComparer.OrdinalIgnoreCase);
private readonly List<String> _fieldNames = new();
private readonly List<List<XLCellValue>> _fieldValues = new();

Expand Down Expand Up @@ -71,15 +71,18 @@ internal IList<String> SourceRangeFields
{
get
{
// TODO: Once pivot cache is filled with values, replace with fields of a cache.
return PivotSourceReference.SourceRange
.FirstRow()
.Cells()
.Select(c => c.GetString())
.ToList()
.AsReadOnly();
.ToList();
}
}

/// <summary>
/// Pivot cache definition id from the file.
/// </summary>
internal uint? CacheId { get; set; }

internal Guid Guid { get; }
Expand All @@ -90,7 +93,7 @@ internal IList<String> SourceRangeFields

internal XLPivotCache AddCachedField(String fieldName, List<XLCellValue> items)
{
if (_fieldNames.Contains(fieldName))
if (_fieldNames.Contains(fieldName, StringComparer.OrdinalIgnoreCase))
{
throw new ArgumentException($"Source already contains field {fieldName}.");
}
Expand Down Expand Up @@ -127,7 +130,7 @@ private String AdjustedFieldName(String header)
{
var modifiedHeader = header;
var i = 1;
while (_fieldNames.Contains(modifiedHeader))
while (_fieldNames.Contains(modifiedHeader, StringComparer.OrdinalIgnoreCase))
{
i++;
modifiedHeader = header + i.ToInvariantString();
Expand Down
5 changes: 2 additions & 3 deletions ClosedXML/Excel/PivotTables/XLPivotField.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;

namespace ClosedXML.Excel
{
Expand All @@ -13,7 +12,7 @@ internal class XLPivotField : IXLPivotField

public XLPivotField(XLPivotTable pivotTable, string sourceName)
{
this._pivotTable = pivotTable;
_pivotTable = pivotTable;
SourceName = sourceName;
Subtotals = new List<XLSubtotalFunction>();
SortType = XLPivotSortType.Default;
Expand Down Expand Up @@ -131,6 +130,6 @@ private void SetExcelDefaults()

public Boolean IsInFilterList => _pivotTable.ReportFilters.Contains(this.SourceName);

public Int32 Offset => _pivotTable.PivotCache.SourceRangeFields.ToList().IndexOf(this.SourceName);
public Int32 Offset => _pivotTable.PivotCache.SourceRangeFields.IndexOf(this.SourceName);
}
}
23 changes: 17 additions & 6 deletions ClosedXML/Excel/PivotTables/XLPivotSourceReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,27 @@ public XLPivotSourceReference(IXLTable table)

public XLPivotTableSourceType SourceType { get; }

#region IEquatable interface
public override bool Equals(object obj)
{
var other = obj as XLPivotSourceReference;
return Equals(other);
}

public override int GetHashCode()
{
unchecked
{
return (SourceRange.GetHashCode() * 397) ^ (int)SourceType;
}
}

public bool Equals(XLPivotSourceReference other)
{
if (other is null || SourceType != other.SourceType)
{
return false;
}

if (ReferenceEquals(this, other))
return true;

switch (SourceType)
{
Expand All @@ -41,10 +54,8 @@ public bool Equals(XLPivotSourceReference other)
return XLRangeAddressComparer.IgnoreFixed.Equals(SourceRange.RangeAddress, other.SourceRange.RangeAddress);

default:
throw new NotImplementedException();
throw new NotSupportedException();
}
}

#endregion IEquatable interface
}
}
7 changes: 3 additions & 4 deletions ClosedXML/Excel/PivotTables/XLPivotTables.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,17 @@ internal class XLPivotTables : IXLPivotTables
{
private readonly Dictionary<String, XLPivotTable> _pivotTables = new(StringComparer.OrdinalIgnoreCase);

public XLPivotTables(IXLWorksheet worksheet)
public XLPivotTables(XLWorksheet worksheet)
{
Worksheet = worksheet ?? throw new ArgumentNullException(nameof(worksheet));
}

internal IXLWorksheet Worksheet { get; }
internal XLWorksheet Worksheet { get; }

public IXLPivotTable Add(string name, IXLCell targetCell, IXLPivotCache pivotCache)
{
if (!pivotCache.FieldNames.Any())
{
pivotCache.Refresh();
}

var pivotTable = new XLPivotTable(Worksheet)
{
Expand Down Expand Up @@ -92,6 +90,7 @@ internal void Add(String name, IXLPivotTable pivotTable)
_pivotTables.Add(name, (XLPivotTable)pivotTable);
}

/// <inheritdoc cref="IXLPivotTables.PivotTable"/>
internal XLPivotTable PivotTable(String name)
{
return _pivotTables[name];
Expand Down
15 changes: 5 additions & 10 deletions ClosedXML/Excel/XLWorkbook_Load.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,7 @@ private void LoadSheetsFromTemplate(String fileName)
private void ResetAllRelIds()
{
foreach (var pc in PivotCachesInternal)
{
pc.WorkbookCacheRelId = null;
}

foreach (var ws in Worksheets.Cast<XLWorksheet>())
{
Expand Down Expand Up @@ -861,17 +859,15 @@ private void LoadSpreadsheetDocument(SpreadsheetDocument dSpreadsheet)
if (!(pivotTableDefinition.PivotFields.ElementAt(pageField.Field.Value) is PivotField pf))
continue;

// TODO: we chould check that pf.Axis == PivotTableAxisValues.AxisPage ?

var cacheFieldValues = pivotSource.GetFieldValues(pageField.Field.Value);
var cacheFieldValues = pivotSource.GetFieldValues(pageField.Field.Value);

var filterName = pf.Name?.Value ?? pivotSource.FieldNames[pageField.Field.Value];

IXLPivotField xlPivotField;
IXLPivotField rf;
if (pageField.Name?.Value != null)
xlPivotField = pt.ReportFilters.Add(filterName, pageField.Name.Value);
rf = pt.ReportFilters.Add(filterName, pageField.Name.Value);
else
xlPivotField = pt.ReportFilters.Add(filterName);
rf = pt.ReportFilters.Add(filterName);

var openXmlItems = new List<Item>();

Expand All @@ -895,7 +891,7 @@ private void LoadSpreadsheetDocument(SpreadsheetDocument dSpreadsheet)
&& (item.Index?.HasValue ?? false))
{
var value = cacheFieldValues[(int)item.Index.Value];
xlPivotField.AddSelectedValue(value);
rf.AddSelectedValue(value);
}
}
}
Expand Down Expand Up @@ -1134,7 +1130,6 @@ private void LoadPivotStyleFormats(XLPivotTable pt, PivotTableDefinition ptd, Di
var values = pt.PivotCache.GetFieldValues(index);
if (fieldItemValue < values.Count)
{
// TODO Implement ClosedXMLValueComparer here later
predicate = o => o.ToString() == values[fieldItemValue].ToString();
}
}
Expand Down
45 changes: 22 additions & 23 deletions ClosedXML/Excel/XLWorkbook_Save.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ private void CreateParts(SpreadsheetDocument document, SaveOptions options)
pivotTableCacheDefinitionPart.PivotCacheDefinition.CacheFields.RemoveAllChildren();
}

var allPivotTables = WorksheetsInternal.Cast<XLWorksheet>().SelectMany(ws => ws.PivotTables).ToList();
var allPivotTables = WorksheetsInternal.SelectMany<XLWorksheet, IXLPivotTable>(ws => ws.PivotTables).ToList();

// Phase 1 - Synchronize all pivot cache parts in the document, so each
// source that will be saved has all required parts created and relationship
Expand Down Expand Up @@ -1835,7 +1835,7 @@ private static void SynchronizePivotTableParts(WorkbookPart workbookPart, IReadO
var xlUsedCaches = allPivotTables.Select(pt => pt.PivotCache).Distinct().Cast<XLPivotCache>().ToList();
if (xlUsedCaches.Any())
{
// Recreate the workbook pivot cache references to removed the previous gunk
// Recreate the workbook pivot cache references to remove previous gunk
var pivotCaches = new PivotCaches();
workbookPart.Workbook.PivotCaches = pivotCaches;

Expand Down Expand Up @@ -1889,7 +1889,7 @@ static void RemoveUnusedPivotCacheDefinitionParts(WorkbookPart workbookPart, IRe
static void AddUsedPivotCacheDefinitionParts(WorkbookPart workbookPart, IReadOnlyList<IXLPivotTable> allPivotTables, SaveContext context)
{
// Add ids and part for the caches to workbooks
// We might get a XLPivotSource with an id of apart that isn't in the file (e.g. loaded form a file and saved to a different one).
// We might get a XLPivotSource with an id of apart that isn't in the file (e.g. loaded from a file and saved to a different one).
var newPivotSources = allPivotTables
.Select(pt => pt.PivotCache.CastTo<XLPivotCache>())
.Where(ps => string.IsNullOrEmpty(ps.WorkbookCacheRelId) || !workbookPart.HasPartWithId(ps.WorkbookCacheRelId))
Expand Down Expand Up @@ -1927,10 +1927,9 @@ private void GeneratePivotCaches(WorkbookPart workbookPart, SaveContext context)
{
foreach (var pt in xlWorksheet.PivotTables.Cast<XLPivotTable>())
{
//context.PivotTables.Clear();

PivotTablePart pivotTablePart;
if (String.IsNullOrWhiteSpace(pt.RelId))
var createNewPivotTablePart = String.IsNullOrWhiteSpace(pt.RelId);
if (createNewPivotTablePart)
{
var relId = context.RelIdGenerator.GetNext(RelType.Workbook);
pt.RelId = relId;
Expand Down Expand Up @@ -2324,16 +2323,16 @@ private void GeneratePivotCaches(WorkbookPart workbookPart, SaveContext context)
}

// TODO: improve performance as per https://github.com/ClosedXML/ClosedXML/pull/984#discussion_r217266491
var sourceFieldNames = pt.PivotCache.FieldNames;
for (var sourceFieldIndex = 0; sourceFieldIndex < sourceFieldNames.Count; ++sourceFieldIndex)
var fieldNames = pt.PivotCache.FieldNames;
for (var fieldIndex = 0; fieldIndex < fieldNames.Count; ++fieldIndex)
{
var sourceFieldName = sourceFieldNames[sourceFieldIndex];
var ptfi = pti.Fields[sourceFieldName];
var fieldName = fieldNames[fieldIndex];
var ptfi = pti.Fields[fieldName];

if (pt.RowLabels.Contains(sourceFieldName))
if (pt.RowLabels.Contains(fieldName))
{
var rowLabelIndex = pt.RowLabels.IndexOf(sourceFieldName);
var f = new Field { Index = sourceFieldIndex };
var rowLabelIndex = pt.RowLabels.IndexOf(fieldName);
var f = new Field { Index = fieldIndex };
orderedRowLabels.Add(rowLabelIndex, f);

if (ptfi.IsTotallyBlankField)
Expand All @@ -2352,10 +2351,10 @@ private void GeneratePivotCaches(WorkbookPart workbookPart, SaveContext context)
rowItemTotal.AppendChild(new MemberPropertyIndex());
rowItems.AppendChild(rowItemTotal);
}
else if (pt.ColumnLabels.Contains(sourceFieldName))
else if (pt.ColumnLabels.Contains(fieldName))
{
var columnLabelIndex = pt.ColumnLabels.IndexOf(sourceFieldName);
var f = new Field { Index = sourceFieldIndex };
var columnLabelIndex = pt.ColumnLabels.IndexOf(fieldName);
var f = new Field { Index = fieldIndex };
orderedColumnLabels.Add(columnLabelIndex, f);

if (ptfi.IsTotallyBlankField)
Expand All @@ -2376,21 +2375,21 @@ private void GeneratePivotCaches(WorkbookPart workbookPart, SaveContext context)
}
}

for (var sourceFieldIndex = 0; sourceFieldIndex < sourceFieldNames.Count; ++sourceFieldIndex)
for (var fieldIndex = 0; fieldIndex < fieldNames.Count; ++fieldIndex)
{
var sourceFieldName = sourceFieldNames[sourceFieldIndex];
var xlpf = pt.ImplementedFields.FirstOrDefault(pf1 => pf1.SourceName.Equals(sourceFieldName, StringComparison.OrdinalIgnoreCase));
var fieldName = fieldNames[fieldIndex];
var xlpf = pt.ImplementedFields.FirstOrDefault(pf => pf.SourceName.Equals(fieldName, StringComparison.OrdinalIgnoreCase));

if (xlpf == null)
{
xlpf = new XLPivotField(pt, sourceFieldName)
xlpf = new XLPivotField(pt, fieldName)
{
CustomName = sourceFieldName,
CustomName = fieldName,
ShowBlankItems = true,
};
}

var ptfi = pti.Fields[sourceFieldName];
var ptfi = pti.Fields[fieldName];

IXLPivotField labelOrFilterField = null;
var pf = new PivotField
Expand Down Expand Up @@ -2465,7 +2464,7 @@ private void GeneratePivotCaches(WorkbookPart workbookPart, SaveContext context)
var pageField = new PageField
{
Hierarchy = -1,
Field = sourceFieldIndex
Field = fieldIndex
};

if (labelOrFilterField.SelectedValues.Count == 1)
Expand Down
2 changes: 1 addition & 1 deletion docs/features/pivot-tables.rst
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ Field names
-----------

Names of fields in a pivot cache can be retrieved from `IXLPivotCache.FieldNames`
property. Every field name is unique string and field names are case-sensitive.
property. Every field name is unique string and field names are case-insensitive.

Field names are taken from top row of the source range. If value isn't
a string, it is converted to a string and used as a field name (e.g. number 12
Expand Down

0 comments on commit 0d8fe87

Please sign in to comment.