Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for DateOnly and TimeOnly in MongoDB #4654

Merged
merged 8 commits into from
Jan 22, 2022

Conversation

daviddesmet
Copy link
Contributor

Summary of the changes (Less than 80 chars)

  • Added support for DateOnly and TimeOnly in MongoDB

Closes #4648 (The type of the member Date of the declaring type TransactionSummary is unknown)

Copy link
Member

@PascalSenn PascalSenn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good :)
Could you also add tests like in https://github.com/ChilliCream/hotchocolate/pull/4552/files? :)
The test setup will be slightly different, but there are already a log of tests in the mongo test repo.

@daviddesmet
Copy link
Contributor Author

@PascalSenn Of course, I’ll work on them 😉

@daviddesmet
Copy link
Contributor Author

daviddesmet commented Jan 17, 2022

@PascalSenn When working on the DateOnly tests I've noticed MongoDB C# driver still doesn't support DateOnly and TimeOnly times, there's an open request here. Not sure how long it will take them to include it in the driver so the only way I see for the tests to pass is to create a custom Mongo Serializer until they add support for it, this should be called in the constructor for the tests, like so:

public MongoDbFilterVisitorDateOnlyTests(MongoResource resource)
{
	Init(resource);

	var dateOnlySerializer = new DateOnlySerializer(true);
	if (!BsonClassMap.IsClassMapRegistered(typeof(Foo)))
	{
		BsonClassMap.RegisterClassMap<Foo>(cm =>
		{
			cm.AutoMap();
			cm.MapMember(c => c.Bar).SetSerializer(dateOnlySerializer);
		});
	}
}

This RegisterClassMap will have to be called by developers willing to support those types anyway.

I personally don't like to do this kind of workarounds in the tests. I would rather prefer to leave the tests prepared without such a workaround and when they support it, we can merge this PR. What do you think?

@PascalSenn
Copy link
Member

@daviddesmet hmm i see, i personally do not mind the specific setup in the tests. I don't think we need to specify each property, i think it's enough to register the serializer for the type

From the issue on Jira

BsonSerializer.RegisterSerializer(new DateOnlySerializer());
--
 
internal class DateOnlySerializer : StructSerializerBase<DateOnly>
{
   ....
}

@daviddesmet
Copy link
Contributor Author

@PascalSenn sounds good, I will make the changes and update my PR.

@daviddesmet
Copy link
Contributor Author

@PascalSenn I've added the DateOnly tests, feel free to check them out while I work on the TimeOnly tests.
Btw, would you also want me to add tests for comparisons? Like greater than, less than, etc?

@PascalSenn
Copy link
Member

@DavidSMET this looks very good! Will review ist ASAP!

@daviddesmet
Copy link
Contributor Author

@PascalSenn aye! Let me know if you also want the comparisons tests. Btw, you mentioned another Smet :P

@daviddesmet
Copy link
Contributor Author

daviddesmet commented Jan 18, 2022

@PascalSenn What's the format for passing a TimeSpan in the query? Afaik is ISO8601 but I'm still getting

TimeSpan cannot parse the given literal of type `StringValueNode`.

I've tried with below queries without any luck:

{ root(where: { bar: { eq: \"06:30:00\" } }){ bar } }
{ root(where: { bar: { eq: \"06:30:00.000\" } }){ bar } }
{ root(where: { bar: { eq: \"6:30:00\" } }){ bar } }
{ root(where: { bar: { eq: \"6:30:00.000\" } }){ bar } }
{ root(where: { bar: { eq: \"6:30\" } }){ bar } }

Those are for the TimeOnly tests

Edit: Nevermind! Found out that I need to specify the TimeSpan type as:

.AddType(new TimeSpanType(TimeSpanFormat.DotNet))

All tests are now passing =)

Copy link
Member

@PascalSenn PascalSenn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great addition! Just made a couple of formatting stuf, apart from that, we'll merge that as is! great job :)
Thanks for the contribution!!

@PascalSenn PascalSenn added the 🎬 ready Ready to merge label Jan 21, 2022
@daviddesmet
Copy link
Contributor Author

It was my pleasure!

@PascalSenn
Copy link
Member

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@sonarcloud
Copy link

sonarcloud bot commented Jan 22, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@PascalSenn PascalSenn merged commit af21d20 into ChilliCream:main Jan 22, 2022
arif-hanif added a commit to arif-hanif/hotchocolate that referenced this pull request Mar 12, 2022
* Update requirement of package (ChilliCream#4461)

* Fixed mutation convention with ID attribute (ChilliCream#4639)

* Updated BCP preview links

* Updated Banana Cake Pop V22 (ChilliCream#4644)

* Fixed mutation convention runtime type determination (ChilliCream#4645)

* Added banners for blog articles

* Handle if errors prop exsits even if we have an response exception (ChilliCream#3928)

Co-authored-by: Pascal Senn <senn.pasc@gmail.com>

* Hot Chocolate 12.5 Release Blog (ChilliCream#4570)

* Fixed Spelling

* Website Cleanup

* Cleanup tabs (Website)

* Fixed Website Layout

* Integrated Apollo Federation Support in Build (ChilliCream#4650)

* Added SkipIntrospectionFields option on MaxDepthAnalyzer (ChilliCream#4649)

* Fixed Website Build Issues

* Added Security Policy

* Update Security Policy

* Fixed mutation convention for ListType payload (ChilliCream#4653)

* Added CodeQL

* Removed JavaScript from CodeQL

* Use Build Scripts with CodeQL

* Reworked CodeQL Build

* Updated CodeQL to install the dotnet SDK

* Fixed CodeQL dotnet build command.

* Fixed Website Layout Bug

* Website: Added Material UI

* Fixed Website Layout Issues

* Added error limits to the document validation. (ChilliCream#4655)

* New Website Search

* Fixed Badges

* Website Launch Button

* Fixed typo in mutations documentation (ChilliCream#4656)

* Fixed problems with benchmarks and adds reference results (ChilliCream#4659)

* Add note for xml docs + custom naming convention (ChilliCream#4661)

* Fixed ApolloFederation Printer (ChilliCream#4663)

* Refined ID attribute to allow use of IDAttribute also without global ID policy (ChilliCream#4667)

* Update issue templates (ChilliCream#4671)

* Update feature_request.yml

* Update bug_report.yml

* Add note about field member to migration document (ChilliCream#4674)

* Added support for DateOnly and TimeOnly in MongoDB (ChilliCream#4654)

Co-authored-by: David De Smet <2607383+idaviddesmet@users.noreply.github.com>
Co-authored-by: Pascal Senn <senn.pasc@gmail.com>

* Reference Resolver Refinement (ChilliCream#4672)

* Fixed Reference Resolvers (ChilliCream#4678)

* Update "Get started" guide (ChilliCream#4451)

* Update MongoDB Connection Documentation (ChilliCream#4681)

* Reworked External Field Handling (ChilliCream#4684)

* Added Apollo Federation Code-First Example (ChilliCream#4687)

* Refined Federation Printer for Custom Schema Directives (ChilliCream#4691)

* Website: Fixed heading color

* Bump Stackexchange.Redis to 2.2.88 (ChilliCream#4693)

* Fixed deprecation in federation schema printer. (ChilliCream#4694)

* Added DataLoader Batching Diagnostics (ChilliCream#4695)

* Adds pure resolver for stitching (ChilliCream#4702)

* Fixing minor typo in mongodb documentation (ChilliCream#4699)

* Added Schema Source Generator (ChilliCream#4708)

* Fixed Snapshots

* Reintroduced PaginationAmount for legacy paging support (ChilliCream#4709)

* Fixed Breaking Change (ChilliCream#4710)

* Fixed Stitching Test

* Fixed concurrency issue with DataLoader (ChilliCream#4718)

* Added support for schema building directives when using schema-first. (ChilliCream#4723)

* Added Helpers for JSON types (ChilliCream#4725)

* Fixed issue were schema-first input runtime types were not correctly inferred. (ChilliCream#4727)

* Fix spelling error (ChilliCream#4729)

* Fixed grammar (ChilliCream#4730)

* Fixed warnings in automapper package (ChilliCream#4728)

* Fixed interface inference issue when using schema-first (ChilliCream#4731)

* Reworked Types Analyzer to auto-register DataLoader (ChilliCream#4732)

* Added Analyzer to Templates (ChilliCream#4733)

* Adds deprecation of input fields and arguments (ChilliCream#4630)

* Added StrawberryShake Code Generator Server (ChilliCream#4742)

* Updated Release Build

* Fixed StrawberryShake Visual Studio issues

* Fixed schema-first input enum binding. (ChilliCream#4750)

* Cleanup StrawberrShake Generator Test Projects (ChilliCream#4751)

* Rename root activity before operation is known (ChilliCream#4749)

* Fixed StrawberryShake MSBuild integration (ChilliCream#4762)

* Added fetch once DataLoader (ChilliCream#4767)

* Remove the need to declare scalars when doing schema stitching (ChilliCream#4769)

* Removed Stitching From Source Generator (ChilliCream#4770)

* Updated Release Build

* Reworked Publish

* Fixed Snapshots

* Fixed build error that published all dlls in the output folder.

* Reworked Source Generator Deployment

* Reworked how we build the code generation server

* Updated Sonar Build Restore

* Changed Build VM

* Exclude Source Generators from Sonar Build

* Fixed Sonar Issue in Mongo Package (ChilliCream#4776)

* Reworked code generation for better compatibility with classic .NET (ChilliCream#4768)

* Abstracted Transport Handling in Schema Stitching (ChilliCream#4777)

* Added support for subscription stitching (ChilliCream#4786)

* Updated BCP to Version 1p23 (ChilliCream#4787)

* Fixed Compile Error

* @defer support for StrawberryShake (ChilliCream#4753)

* Update Neo4JDataRequestBuilderExtensions.cs (ChilliCream#4807)

* Updated BCP to Preview 25

* Use Default Values in UsePagingAttribute (ChilliCream#4815) (ChilliCream#4816)

* Added graphql-ws support (ChilliCream#4793)

* Updated BCP to Preview 26

* GraphQL over WebSocke fixes. (ChilliCream#4819)

* Add default arguments to IEndpointRouteBuilder Map methods (ChilliCream#4792)

* Add default arguments to Map methods

* Add ArgumentNullExceptions

Co-authored-by: Michael Staib <michael@chillicream.com>

* Document dependency injection / Entity Framework Core (ChilliCream#4817)

* Fixed Typos. (ChilliCream#4830)

* Error message grammar (ChilliCream#4829)

Co-authored-by: Michael Staib <michael@chillicream.com>

* Added a utility to communicate with graphql-ws (ChilliCream#4820)

* Add support for custom NodaTime patterns for serialization and deserialization (ChilliCream#4801)

* Reorganized Transport Tests (ChilliCream#4834)

* Fix Website Release Pipeline (ChilliCream#4836)

Co-authored-by: Jari Pennanen <ciantic@oksidi.com>
Co-authored-by: PascalSenn <senn.pasc@gmail.com>
Co-authored-by: Rafael Staib <rafael@chillicream.com>
Co-authored-by: Michael Staib <michael@chillicream.com>
Co-authored-by: Kim Svensson <61824112+allmightyse@users.noreply.github.com>
Co-authored-by: Aleksey Bogomolov <horse315@gmail.com>
Co-authored-by: Daniel Granerud <86605963+nullableexception@users.noreply.github.com>
Co-authored-by: martetassyns <78635303+martetassyns@users.noreply.github.com>
Co-authored-by: Ben McCallum <ben.mccallum@live.com.au>
Co-authored-by: David De Smet <2607383+daviddesmet@users.noreply.github.com>
Co-authored-by: David De Smet <2607383+idaviddesmet@users.noreply.github.com>
Co-authored-by: Tobias Tengler <45513122+tobias-tengler@users.noreply.github.com>
Co-authored-by: itmuckel <michael.bachner@outlook.de>
Co-authored-by: stxwright <14978758+stxwright@users.noreply.github.com>
Co-authored-by: Mikkel Hansen <92272397+mhnisportal@users.noreply.github.com>
Co-authored-by: Gabriel Lucaci <gabriel.lucaci.dev@gmail.com>
Co-authored-by: avisra <chris@avisra.com>
Co-authored-by: Trey Mack <tmack10@gmail.com>
Co-authored-by: grounzero <16921017+grounzero@users.noreply.github.com>
Co-authored-by: David Driscoll <david.driscoll@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The type of the member Date of the declaring type TransactionSummary is unknown
3 participants